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

feat: Add drag and drop to Dashboard layouts #1322

Merged
merged 18 commits into from
Feb 1, 2024

Conversation

bprusinowski
Copy link
Collaborator

@bprusinowski bprusinowski commented Jan 30, 2024

This PR adds a drag and drop functionality to Dashboard layouts (Vertical and Tall) in the edit mode.

How to test

  1. Go to this link.
  2. Add two additional charts, so that there are three in total.
  3. Proceed to layout options.
  4. Change layout to Dashboard.
  5. See that there are drag handles next to charts metadata buttons.
  6. See that it's possible to drag and drop the charts to change their order.
  7. Change layout from Vertical to Tall.
  8. See that drag and drop also works for this layout.

Copy link

vercel bot commented Jan 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
visualization-tool ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 31, 2024 7:43am

@ptbrowne
Copy link
Collaborator

Code looks good, I saw no blocker. LGTM 👍 Some remarks after testing:

  • I wonder though about having a drag overlay since the drag overlay does not show exactly what's under it (since it is another re-rendering of the chart) and there is an animation inside the drag overlay.
  • I also wondered if it would not be better to directly move the chart while dragging (not the overlay, but really dispatch re-order events while we drag)
  • The tall layout goes below the fold on my tiny screen. Maybe it would be good to reduce the width of the whole layout so that the first chart does not take the whole screen. Below a screenshot of what I see
image

@bprusinowski
Copy link
Collaborator Author

Hey @ptbrowne, thanks for reviewing and testing 💯

I adjusted the drag and drop behavior a bit to hide the dragged element and disable animations when creating an overlay, I think it is better now.

I initially tried with moving the chart instead of using overlay, but it was quite "messy", especially for Tall layout, when the thing you drag suddenly changes size in the middle of page scroll, I think it's more clean when we need to "confirm" the action. Let me know if the new behavior is still confusing, it would be also great to get @KerstinFaye's view on this 🎨

We've also discussed limiting the width of the edit / publish views with @KerstinFaye, I'll introduce the changes in later PRs 👍

@KerstinFaye
Copy link

Thanks a lot for your great work @bprusinowski and for your testing and feedback @ptbrowne.

@bprusinowski bprusinowski merged commit 6dfaa73 into main Feb 1, 2024
4 of 6 checks passed
@bprusinowski bprusinowski deleted the feat/layout-drag-and-drop-kit branch February 1, 2024 08:16
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.

3 participants