-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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(<DraggableTabs>): Add new draggable tabs component to storybook #73239
feat(<DraggableTabs>): Add new draggable tabs component to storybook #73239
Conversation
fcb5318
to
bc56fac
Compare
Bundle ReportChanges will increase total bundle size by 171.96kB ⬆️
|
f7abe95
to
4060eaa
Compare
dragging an item onto itself seems to move it to the next position CleanShot.2024-06-25.at.17.06.39.mp4 |
feels like the draggable element is hijacking the click to switch tabs CleanShot.2024-06-25.at.17.11.52.mp4 |
Fixed in the latest commits, thanks for catching this! |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good start! We can improve on the drag interactions in subsequent PRs, but before merging this one I'd like to try a couple things:
- Add a test file with some basic tests (similar to the original tabs component). Ideally we would want a test that validates the drag and drop is working correctly, but that may be difficult with the jsdom test env. You can try something like this https://testing-library.com/docs/user-event/pointer and see if that works though.
- Ideally we would want to use some of the Tab components directly instead of copy/pasting them. I'd give that a shot and at least uncover what kind of issues you run in to and note them down so that we can tackle the code deduplication later on.
Note: This PR merges into the
msun/draggableTabsComponent
, notmaster
. Themsun/draggableTabsComponent
branch contains thecomponents/draggableTabs
directory with with the files fromcomponents/tabs
(except for the test file). This is so that the diff for this PR is easier to read and more intuitive – it effectively shows all upgrades to the existing tabs components to make the tabs draggable.This PR implements an MVP for drag and drop tabs. Visually, these tabs are identical to the current tabs component, but you can drag them and drop them within the TabList, and a vertical line appears to indicate the drop position for a tab that is currently being dragged. You can play around with this component in the vercel deployment's storybook.
Quick Demo:
Screen.Recording.2024-06-25.at.2.15.21.PM.mov
Known issues: