Skip to content

dnd: computeDrop hover.atTop check#199

Closed
edimov wants to merge 2 commits intobrimdata:mainfrom
edimov:feature/compute-drop-sblings
Closed

dnd: computeDrop hover.atTop check#199
edimov wants to merge 2 commits intobrimdata:mainfrom
edimov:feature/compute-drop-sblings

Conversation

@edimov
Copy link

@edimov edimov commented Dec 9, 2023

Description

This MR:

  • adds hover.atTop check to allow siblings drop on nodes with empty children
  • removes unnecessary computeDrop calculations when hovering over same node

Screenshots

Before
Screen.Recording.2023-12-09.at.20.42.11.mov
After
Screen.Recording.2023-12-09.at.21.23.53.mov

@edimov edimov force-pushed the feature/compute-drop-sblings branch from 255b91c to 7e58836 Compare December 12, 2023 08:02
@edimov edimov changed the title dnd: computeDrop hover.atBottom check dnd: computeDrop hover.atTop check Dec 12, 2023
@jameskerr
Copy link
Contributor

This is great. I'm pulling your branch now to run the tests.

@jameskerr
Copy link
Contributor

I think this is a bug. If a folder is open and has children you should not be able to place an item as the folder's sibling unless you drag it to the bottom of it's child list.

Area.mp4

@jameskerr
Copy link
Contributor

We could check if the above node isOpen and hasNoChildren, then allow the drop as sibling. Would that work for you?

@edimov
Copy link
Author

edimov commented Dec 13, 2023

Awesome! That's the check I've been missing. I've updated with 9bcbf67. Do you think you had something like this in mind?

@pivanov
Copy link

pivanov commented Dec 13, 2023

Ship it! :D

@edimov edimov force-pushed the feature/compute-drop-sblings branch from 9bcbf67 to c9af3ab Compare December 13, 2023 10:42
@jameskerr
Copy link
Contributor

Yeah that is what I was thinking. I'll run my own tests again. Let's ship today.

jameskerr added a commit that referenced this pull request Dec 13, 2023
Based off #199

Thanks to the understanding and work of @edimov, we can now drop a node as a sibling (or child) of an open, but childless node. This should unlock people who want to create trees where every single node is an open, internal node.

Co-authored-by: Evgeni Dimov <edimov@plandelta.com>
@jameskerr
Copy link
Contributor

Merged in #202

@jameskerr jameskerr closed this Dec 13, 2023
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