Skip to content

Make drag-and-drop usable in the outliner#431

Merged
thesophiaxu merged 34 commits intomainfrom
alex/dnd
Apr 13, 2022
Merged

Make drag-and-drop usable in the outliner#431
thesophiaxu merged 34 commits intomainfrom
alex/dnd

Conversation

@dragonman225
Copy link
Contributor

@dragonman225 dragonman225 commented Apr 1, 2022

Functionality Changes

  • Revamped drag-and-drop
    • Add drag handles to note blocks, to make it clear how to drag and drop them.
    • Dropping a note block to any of its descendants is not allowed.
    • Useless dropping is not allowed.
      • Dropping before or after itself.
      • Dropping after its previous sibling or before its next sibling.
    • Global drag-and-drop (e.g. to/from the "Favorites" section) is temporarily disabled to prevent unexpected bugs/crashes.
    • Drag-and-drop across different note tabs still works.

Code Changes / Refactoring / Fixes

  • Rewrite code related to managing collapsed state of outlines (see packages/unigraph-dev-explorer/src/examples/notes/useOutlineCollapsed.ts):
    • Fix a bug: A collapsed outline becomes non-collapsed after drag-and-drop.
    • Simplify code: No need to pass props around.
  • Support adding items before indexes in a list (see packages/default-packages/unigraph.core/executables/addItemToList.js).
    • Otherwise, it would be impossible to drop a note block before a note block at index 0 in another list.
  • Extract <Outline> component from packages/unigraph-dev-explorer/src/examples/notes/NoteBlock.tsx since the file is too big and hard to navigate.

@dragonman225 dragonman225 marked this pull request as ready for review April 1, 2022 11:15
@dragonman225 dragonman225 requested a review from thesophiaxu April 2, 2022 08:29
@dragonman225 dragonman225 changed the title Add usable drag and drop to the outliner Make drag-and-drop usable in the outliner Apr 2, 2022
Copy link
Contributor

@TheExGenesis TheExGenesis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pulled but the drag and drop wasn't updating block position.

@dragonman225
Copy link
Contributor Author

dragonman225 commented Apr 3, 2022

I pulled but the drag and drop wasn't updating block position.

Could you elaborate more? Do you mean when dropping the block it doesn't get reordered/moved? (a screencast would be very helpful)

@TheExGenesis
Copy link
Contributor

TheExGenesis commented Apr 3, 2022

Hey I recorded a video, see if you can reproduce some of the problems I faced. Sometimes blocks won't move, sometimes they'll disappear, one time a block got cloned: https://www.loom.com/share/a226dfe5d3db43519e296be377a712c1

I'm really excited about this feature btw, glad you're working on it :)

@dragonman225
Copy link
Contributor Author

Thank you for the video! Looks like the detection of drop target jumps a lot. What browser/OS are you using? And where is the server running (on the same machine / in the same local network / on a cloud service / etc)?

@TheExGenesis
Copy link
Contributor

Using Brave (which is Chromium), server running locally.

@dragonman225
Copy link
Contributor Author

dragonman225 commented Apr 4, 2022

I downloaded Brave (on Linux) and drag-and-drop worked normally. But I did see some of the bugs (block not moving on drop, block duplicates) when I was in a tab that was idle for a long time.

At this moment drag-and-drop calls API to update data, and the UI reflects changes through data subscriptions, so if the network/server/subscription doesn't work correctly, drag-and-drop would fail.

@thesophiaxu
Copy link
Collaborator

Just tried the latest commit - it is still very hard to click on the note blocks (bottom 25% is completely unclickable, for example) - I've tried to make the dropTargetHeight into 10 and it solves the problem, but I don't know if we can't have a larger detection area w/o blocking onClick - maybe it is a limitation of react-dnd?

Otherwise everything looks fine, and feel free to merge this once the unclickable issue is done :)

@dragonman225
Copy link
Contributor Author

dragonman225 commented Apr 11, 2022

@thesophiaxu Discovered the "Sortable" examples today. In fact we can infer the drop action without the need of explicit drop target elements. So no more drop targets now, clicking into the note blocks should work like before.

@dragonman225
Copy link
Contributor Author

Thanks @thesophiaxu for your input on switching the executable approach to direct unigraph.updateObject! It's a lot faster.

There's a little annoying effect, which is that the dropped block may flicker its text to show its uid for a bit, though I think we can address it later.

Video in 0.5x speed:

kazam_screencast_00000.out.mp4

@thesophiaxu
Copy link
Collaborator

Thanks @thesophiaxu for your input on switching the executable approach to direct unigraph.updateObject! It's a lot faster.

There's a little annoying effect, which is that the dropped block may flicker its text to show its uid for a bit, though I think we can address it later.

Video in 0.5x speed:

kazam_screencast_00000.out.mp4

I've found that as well - the issue is with how subscriptions are handled and it will probably be fixed later

@thesophiaxu thesophiaxu merged commit e412296 into main Apr 13, 2022
@thesophiaxu thesophiaxu deleted the alex/dnd branch April 13, 2022 16:05
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