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

Disallow drag and drop a row inside another uncollapsed row #669

Closed
wants to merge 1 commit into from

Conversation

mdvictor
Copy link
Contributor

@mdvictor mdvictor commented Apr 3, 2024

Screen.Recording.2024-04-02.at.17.37.13.mov

This PR fixes the dashboard crashing when a row is dragged within the area of another uncollapsed row. It's not the most elegant solution, so I'm open to feedback if there is a better way to achieve this, but I think this is the simplest way to fix the bug without adding a lot of complexity, knowing that this will need to be revisited having invalidation in mind.

I think this needs to be further worked on, especially on UX side of things to let users know that they can't have nested rows (e.g.: highlight the uncollapsed row area, similar to how we do to repeated panels and disallow dropping if dragged element is a row). My knowledge of RGL is limited and I couldn't find a proper way to invalidate a drop, which makes me believe that the only way to do it would be to update the layout with the invalid state and then re-update with the old state.

The idea here is to return early if we detect after a drag and drop that we are in a row within a row scenario and force render the grid layout with the old layout. Since there is no actual layout change in the grid props, we need to force the render through the grid layout key prop.

TODO: tests

@mdvictor
Copy link
Contributor Author

mdvictor commented Apr 3, 2024

I'm still working on an alternative where we would store the old children state onDragStart and return early in onDragStop if state is invalid which would call onLayoutChange and if an old state is present it would reset to that, which is still flickering, but I'm still looking into it.

@mdvictor
Copy link
Contributor Author

mdvictor commented Apr 3, 2024

Screen.Recording.2024-04-03.at.12.40.16.mov

Might've found a better way of doing this with what I mentioned above that doesn't flicker 🎉 Will push that to another branch as well

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

Successfully merging this pull request may close these issues.

None yet

1 participant