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

Disable force update of properties of Konva nodes #271

Closed
lavrton opened this issue Oct 21, 2018 · 11 comments
Closed

Disable force update of properties of Konva nodes #271

lavrton opened this issue Oct 21, 2018 · 11 comments

Comments

@lavrton
Copy link
Member

lavrton commented Oct 21, 2018

Currently, react-konva works VERY strictly in terms of how it updates props of nodes.

When a component is updated react-konva will change all attributes of a node exactly how it is defined in render() function. In many cases, it gives unexpected behavior for new users and it requires additional work to sync state of the app with the props of nodes.

The most common issue is drag&drop:

<Circle
   draggable
   x={10}
   y={10}
   radius={50}
   fill={this.state.fill}
   onDragEnd={() => {
      this.setState({  fill: getRandomColor() })
   }}
/>

At first, the component looks good. But it will not work, as expected. Because on dragend the position will be reseted back to {10,10} (as defined in render). I think most of the users expect no to see that reset.

My proposal is to add useStrictMode function. By default react-konva will NOT use strict mode. I think it will work better for the most of the apps. If strict updates are still required a user can do this:

import { useStrictMode } from 'react-konva';

useStrictMode(true);

In non-strict-mode react-konva wil update properties of the node only if they changed in render() function. That is similar how react-dom updates DOM nodes.

@pleopardi
Copy link

pleopardi commented Oct 21, 2018

At the moment in my use cases I'm not having the issue described, but I agree with your proposal. Current behavior is definitely unexpected.

@elmigranto
Copy link

elmigranto commented Oct 21, 2018

This feels similar to (un)controlled inputs in React itself. Would defaultX prop work here instead of x? I am aslo not that familiar with API, but I guess everything's either draggable out of the box or when you define onDragEnd? I don't feel this being unintuitive at all, how is dragging solved in React?

@lavrton
Copy link
Member Author

lavrton commented Oct 24, 2018

@elmigranto yeah, it is a close concept to (un)controlled inputs. I was thinking to add defaultX (and all other defaultAttr properties). But I decided that it will be too complex and slow.

@lavrton
Copy link
Member Author

lavrton commented Oct 24, 2018

Just made a prerelease 16.6.0-1 with useStrictMode(false) by default. Testing it in large apps, will make a full release later after testing.

@lavrton
Copy link
Member Author

lavrton commented Oct 24, 2018

From the tests all working just fine. Released v16.6.0.

@bmoquist
Copy link

I wish this had been marked in bright red as a breaking change on the release page -- caused me several hours of heartache and confusion around group drag and drop behavior 😩after updating my app's packages. I found that my React-Redux app definitely requires strict mode.

Thanks though for the great answers in #296 -- now, everything seems to be working again. 😃👍

@lavrton
Copy link
Member Author

lavrton commented Jan 14, 2019

@bmoquist sorry for that situation. Will be more careful next time. Also soon I will add more info about strict mode into the documentation.

@lyleunderwood
Copy link

Also struggling with a regression for the last few hours, eventually did a bisect to track it down to a react-konva upgrade and led me here.

@lavrton
Copy link
Member Author

lavrton commented Jan 24, 2019

@bmoquist, @lyleunderwood it will be good if you can share your use cases and show why "non-strict" mode doesn't work for you by default.

@bmoquist
Copy link

I've built an editor that allows people to move and manipulate shapes and photos on a canvas. The positions of the objects are tracked in redux. Objects can be moved individually or selected and moved as a group. When I upgraded where non-strict was the default, the objects were not being properly rendered on drag end and would jump back to prior positions. I'd have to go back and play with it to remember more details about what was going wrong.

@lyleunderwood
Copy link

My application is very similar to that of @bmoquist (redux storing positions, etc.). In my case, however, some items on the canvas when dragged would move twice as quickly as the mouse. I think I actually made a crappy gif of it when i was trying to debug it...

deleted2

If you can make it out, you'll notice that the selected item in the second example is out of sync with its resize handles. The resize handles are actually moving at the correct velocity, and the selected item itself is moving at a higher velocity, as if all position changes were being applied to it twice.

This likely actually exposes some kind of inefficiency in the way I'm passing down these position props, but one I'm not gonna dig into right now. Regardless, switching to strict mode cleared this issue up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants