Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

callback for setAttributes? #5596

Closed
JimSchofield opened this issue Mar 13, 2018 · 9 comments
Closed

callback for setAttributes? #5596

JimSchofield opened this issue Mar 13, 2018 · 9 comments
Labels
[Status] Not Implemented Issue/PR we will (likely) not implement. [Type] Enhancement A suggestion for improvement. [Type] Help Request Help with setup, implementation, or "How do I?" questions.

Comments

@JimSchofield
Copy link

JimSchofield commented Mar 13, 2018

Issue Overview

There are instances where a callback for setAttributes might make sense

Current Issue

function changeInput(changes) {
    setAttributes({  attribute: changes });
    // something that relies on attributes being updated in the render first, e.g. dom refs
    // I'm doing this to set focus and/or set cursor locations.  As is, refs are undefined here
    // in my attempts, and I'm not sure why
}

Current Workaround

I got it to work sufficiently by doing a setTimeout, but I am not sure why or how reliably it would work:

function changeInput(changes) {
    setAttributes({  attribute: changes });
    setTimeout(
        () => { /*something that relies on attributes being updated in the render first, e.g. dom refs */ },
        0
    );
}

Possible Solution

Might we include a callback like in React's setState method?

function changeInput(changes) {
setAttributes({
    attribute: changes
}, () => { /*something that relies on attributes being updated in the render first, e.g. dom refs */ }
});

Related Issues and/or PRs

I apologize if this has been discussed before, but I couldn't find an instance where this was suggested.

I also am aware this may be solved in other ways, but it seems like including some parallel capability to setState would make sense here if possible.

@aduth
Copy link
Member

aduth commented Mar 13, 2018

There are instances where a callback for setAttributes might make sense

Could you describe such an instance? To have a better understanding of the use-case and whether callback is the most appropriate solution.

@JimSchofield
Copy link
Author

In my case I am creating a block that formats code, and I need to retain my cursor position after tabbing. Below is the only way I could get it to work that I could think of, since the position needed to be placed after the attributes were set:

    function checkKey(event) {
        // checks for a tab keypress, and if present, manually adds spacing
        if(event.keyCode == 9) {
            // escape browser tabbing, will deal with accessibility once it functions
            event.preventDefault();

            // get cursor location
            let location = event.nativeEvent.target.selectionEnd;
            
            // "splice" a tab
            let newCodeString = codeString.slice(0,location) + '    ' + codeString.slice(location);

            let newBeautifulCodeString = Prism.highlight(newCodeString, Prism.languages[language]);

            setAttributes({
                codeString: newCodeString,
                beautifulCode: newBeautifulCodeString
            });

            // setTimout will have to suffice?
            setTimeout(() => {
                nativeElements.inputRef.focus();
                nativeElements.inputRef.selectionEnd = location + 4;
            }, 0);
        }
    }

...
        <pre class="language-javascript">
            <TextareaAutosize
                value={codeString}
                tag="code"
                onChange={(e) => changeCode(e.target.value, e)}
                onKeyDown={checkKey}
                placeholder='Type some code here...'
                innerRef={el => (nativeElements.inputRef = el)} //storing reference to try to set cursor position
            />
        </pre>  

I would be interested to know if there are other strategies people have used for something like this.

I imagine there may be situations for an API call as a callback? You may want to set the state and then call out using the state only after state has updated.

@jeffpaul jeffpaul added the [Type] Enhancement A suggestion for improvement. label Mar 14, 2018
@ajitbohra ajitbohra added the [Type] Help Request Help with setup, implementation, or "How do I?" questions. label Jul 8, 2018
@mtias mtias added the Needs Technical Feedback Needs testing from a developer perspective. label Oct 7, 2018
@aduth
Copy link
Member

aduth commented Oct 10, 2018

At this time, we'll not support a callback for setAttributes.

For your use-case, I'd suggest one of:

  • Use the componentDidUpdate lifecycle method of the Component class to determine whether the component has rendered as the result of an attribute change
  • Maintain the value as state internal to the Component and call setAttributes at known intervals (e.g. blur event).

@aduth aduth closed this as completed Oct 10, 2018
@aduth aduth added [Status] Not Implemented Issue/PR we will (likely) not implement. and removed Needs Technical Feedback Needs testing from a developer perspective. labels Oct 10, 2018
@mintplugins
Copy link

@aduth Since setAttributes is very similar to setState, this would follow the standard set by React itself. Can I ask why this is not being considered?

@aduth
Copy link
Member

aduth commented Feb 21, 2019

Despite the similar name, setAttributes does not behave the same as React's setState. Where the latter is primarily concerned with a component's internal state, setAttributes (and in turn, a block's attributes generally) are global state and thus more akin to props in a React application. Notably, a block's attributes can be modified from anywhere, and its implementation should be tolerant to those changes, and not only under the specific events upon which a callback would depend.

There was a discussion not too long ago about this in Slack, in case it helps provide additional context (link requires registration):

https://wordpress.slack.com/archives/C02QB2JS7/p1545162410122400

@mintplugins
Copy link

@aduth Thanks for responding. Since I posted this I've been pouring through Gutenberg to try and understand why this can't be a thing.

I realized that underneath the actions are called by Redux, and that Redux doesn't have a callback like that. Rather it appears it would require something like thunk. I believe you had some discussion around that here:
#691

Anyway, for the time being I'm just using setTimeout and setInterval to check the value of attributes as mentioned by the OP.

@aduth
Copy link
Member

aduth commented Feb 21, 2019

I'd recommend considering componentDidUpdate, which avoids many issues inherent to timers (i.e. what to do if the block is unmounted before the callback is called) and handles external attributes changes.

A contrived example:

blockSettings.edit = class extends wp.element.Component {
	constructor() {
		super( ...arguments );

		this.setIsClicked = this.setIsClicked.bind( this );
	}

	componentDidUpdate( prevProps ) {
		if ( this.props.attributes.isClicked && ! prevProps.attributes.isClicked ) {
			// After attribute changed.
		}
	}

	setIsClicked() {
		this.props.setAttributes( {
			isClicked: true,	
		} );
	}

	render() {
		return <button type="button" onClick={ this.setIsClicked }>Click</button>
	}
};

See: https://reactjs.org/docs/react-component.html#componentdidupdate

@mintplugins
Copy link

I was hoping to avoid needing to do a ton of prop drilling. If I rely on componentDidUpdate as opposed to a callback function, I have to prop drill every variable from the parent to my child components.

@nickyoung87
Copy link

Thanks for posting your workaround for us @JimSchofield

I just had to do this because I am using a ServerSideRender output for a block. It uses a custom JS to load the data in an iframe. We have it set up to use a preview button, but when the preview is active and the attributes change it doesn't update the preview because the JS needs to be reloaded. So I have set a time to do a refresh. Seems to work for now, but it would have been much nicer if there was a way to just call the function after setAttributes was done saving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Not Implemented Issue/PR we will (likely) not implement. [Type] Enhancement A suggestion for improvement. [Type] Help Request Help with setup, implementation, or "How do I?" questions.
Projects
None yet
Development

No branches or pull requests

7 participants