-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[groups] bring back group item dragging #7045
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
base: david/groups-ui-2
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
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.
Don't think we should show the separator here, it's a noop action that doesn't change anything:
CleanShot.2025-11-03.at.13.44.05.mp4
When dragging a group that is between groups the separator shows in a strange spot:
CleanShot.2025-11-03.at.13.43.28.mp4
On mobile it opens the context menu and then shows the separator on top of it:
ScreenRecording_11-03-2025.13-48-47_1.MP4
| } | ||
| } | ||
|
|
||
| async function executeGroupOperations(app: TldrawApp, groupId: string, operation: any) { |
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.
| async function executeGroupOperations(app: TldrawApp, groupId: string, operation: any) { | |
| async function executeGroupOperations(app: TldrawApp, groupId: string, operation: DragGroupOperation) { |
| newIndex = getIndexBetween(beforeGroup?.index, afterGroup?.index) | ||
| } | ||
|
|
||
| await app.z.mutate.updateOwnGroupUser({ groupId, index: newIndex }) |
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.
| await app.z.mutate.updateOwnGroupUser({ groupId, index: newIndex }) | |
| app.z.mutate.updateOwnGroupUser({ groupId, index: newIndex }) |
tried and discussed this quite extensively with Mai Linh, we decided that it was better overall to show the indicator even if it's a noop. without it the lack of hints for what the interaction involves can feel confusing, especially because if the groups are open it can take a looong while for the mouse to move to a position where the drag would have an effect. |
|
i'll fix the other things, good catches! |
had to add a drag threshold thingy to make this not annoying af
Change type
otherNote
Enables dragging groups in the sidebar to reorder them, adds a drag-start threshold and cursor indicator, and introduces shared types and hooks to support the behavior.
TlaSidebarGroupItemdraggable and reorderable with a global reorder cursor shown after a drag threshold (hasDragStarted).data-is-draggingand new CSS; show drop cursor only when dragging has begun.detectGroupOperationandexecuteGroupOperationsto compute insert position and update group order (fractional indexing viaupdateOwnGroupUser).groupItems) and add a small movement threshold before activating drag.useIsDragginghook; replace inline dragging checks in file/group components.DragStatewithhasDragStartedand support forDragGroupOperation.DragGroupOperationtype and adjust reorder indicator constants/names.hasDragStarted(e.g., recent files, reorder cursor).Written by Cursor Bugbot for commit dbb8bd1. This will update automatically on new commits. Configure here.