moveOnStartChange causes unnecessary state updates

[jmuerle]

I'm using react-draggable in my application to scroll an item in a viewport (i.e. dragging the background scrolls the view), and also for custom scrollbars around the viewport. Needless to say, I needed to use the moveOnStartChange attribute to keep these two manners of scrolling the same viewport consistent. I found, however, that there was some really buggy behavior in react-draggable, and it stemmed from the fact that componentWillReceiveProps triggers too often.

If you declare your component like this:

<Draggable start={{x: 0, y: 0}}>...</Draggable>

then every time the render() method is called, the componentWillReceiveProps method is called because, technically, a new object is being passed as the 'start' property.

The solution I have come up with is to change lines 471-473 in the componentWillReceiveProps method to:

if (newProps.moveOnStartChange && newProps.start &&
    (newProps.start.x !== this.props.start.x || newProps.start.y !== this.props.start.y))
{
    this.setState(this.getInitialState(newProps));
}

Thoughts?

[STRML]

Yeah, I really dislike how moveOnStartChange works in general. It is an unfortunate problem in React when you start managing state for the programmer; when he wants to override it, it gets hairy.

See the coreRefactor branch where I've started addressing this concern by splitting the component into <DraggableCore>, which relies on you to update its x and y, and <Draggable>, which builds upon <DraggableCore> to deliver the existing API.

[jmuerle]

Hmmm, I'm not really able to read the new code effectively because it's in .es6 format, but making the translation state for the drag public is a great solution. It briefly crossed my mind while implementing that I wished that that state were accessible.

When is that refactor planned to be shipped?

[STRML]

I believe it's complete, but I'm working it into React-Grid-Layout and
React-Draggable beta versions to ensure the API is useful before tagging
it. Should be a week or so. I believe the README's updated, give it a
shot and see if it works for you.

On 7/14/15 4:30 PM, jmuerle wrote:

Hmmm, I'm not really able to read the new code effectively because
it's in .es6 format, but making the translation state for the drag
public is a great solution. It briefly crossed my mind while
implementing that I wished that that state were accessible.

When is that refactor planned to be shipped?

—
Reply to this email directly or view it on GitHub
#72 (comment).

[jmuerle]

The README in the coreRefactor branch is the same as in the main branch as far as I can tell. Is that the README you were referring to?

[STRML]

You're right. I haven't yet updated it. It's very simple though - <Draggable> API stays the same, <DraggableCore> relies on its start prop to be told where to go. It will call back with drag data for you but you have to update it manually.

[jmuerle]

Do you have an update on the status of the coreRefactor branch?

[henryboldi]

I'm really happy to see that the coreRefactor branch is still being worked on! I'll give <DraggableCore> a try 😄. When do you expect this to get merged into master?

[STRML]

Oh, hopefully soon. I'm trying to get React-Grid-Layout back to its original functionality on coreRefactor branch, which will signal that the new version is usable enough for release.

[jmuerle]

It looks like the coreRefactor branch hasn't been touched for almost a month now. Do you have a target date in mind for when you would like to merge that into master?

[psaia]

I'm working on migrating an existing project that was using moveOnStartChange. Now that it's removed, what is the recommended way of programmatically updating the position?

[STRML]

Use <DraggableCore>, which doesn't keep any state but calls back on drag with its deltas. Use the callback to apply deltas to the position that you keep inside your parent component, and modify that position at will. Apply the position to <DraggableCore> by setting its left/top or by using transform or transform3d.

[psaia]

Got it! Thanks for the quick reply.