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: Reordering documents in collection #1283

Conversation

anteprimorac
Copy link
Contributor

@anteprimorac anteprimorac commented May 22, 2020

This PR implements #622.

  • sort collection documents
  • sort document children
  • improve animation by updating UI before server response
  • moving document in the hierarchy
    • display placeholder where the document would be dropped
    • move document anywhere in the hierarchy of the same collection
      • disable moving the document to itself or child droppable
      • expand document hierarchy when dragging over it
      • allow dropping to document that currently doesn't have any child document
    • move the document to another collection
      • expand collection hierarchy when dragging over it
  • improve destination detection and dragging animations

@anteprimorac anteprimorac changed the title WIP feature: Reordering documents in collection WIP: feature: Reordering documents in collection May 22, 2020
@anteprimorac anteprimorac changed the title WIP: feature: Reordering documents in collection feat: Reordering documents in collection May 22, 2020
@tommoor
Copy link
Member

tommoor commented May 23, 2020

This is great! Definitely need to figure out how to prevent the jittering in the frontend, but really psyched to see it working – you can even have two tabs open and the existing websocket messages automatically keep the sidebars in sync 😉

@anteprimorac
Copy link
Contributor Author

@tommoor I've just pushed applying reordering of documents to the UI before sending a request to the server.

The only feature that needs to implement is moving documents between parent documents or collections. But that should be straightforward because the reordering logic on the client and server-side support it already.

@tommoor
Copy link
Member

tommoor commented May 25, 2020

I've just pushed applying reordering of documents to the UI before sending a request to the server.

Nice! Works well. One thing I noticed is that in this situation where you have nested documents when you drag the child documents it will often drag the outer "Documentation" – I think dragging anywhere "inside" should not drag the out doc. Does that make sense?

image

@anteprimorac
Copy link
Contributor Author

@tommoor that make sense and that's how it's supposed to work, but I think it might be a styling issue because the only way that could happen is that keydown event gets propagated to the parent component.

I've pushed an implementation for moving documents in the hierarchy. But it needs more work. The main thing that bothers me is disabling moving document into itself. I will update the PR description to reflect all issues and missing features I found.

@anteprimorac
Copy link
Contributor Author

The implementation of displaying a placeholder where the dragged document would be dropped will be complicated because of react-beautiful-dnd limitations. There are some community examples that implement this feature on top of the library but none of them works out of the box.

@tommoor tommoor changed the base branch from master to develop June 13, 2020 07:03
@tommoor
Copy link
Member

tommoor commented Jun 18, 2020

Hey @anteprimorac – really excited for the potential of this feature, is there anything your waiting on from me as a maintainer here? If you think you won't realistically get time to take this over the finish line let me know and I'll see if we can get other engineers to finish those final TODO's.

Many thanks!

@anteprimorac
Copy link
Contributor Author

Hey @tommoor, thank you for bringing this up. Unfortunately, I didn't have time to finish all TODO's listed in the description, but hopefully, in the next few weeks, I would have time for this.

@tommoor
Copy link
Member

tommoor commented Jun 21, 2020

Please note this recently merged PR (#1309) has created a lot of conflicts, but they're easily resolved by following the same steps detailed in the PR in this branch before updating from the default.

@anteprimorac
Copy link
Contributor Author

@tommoor all conflicts should be fixed, and all features I've listed in the description of this PR are implemented now. But currently, the dragging UX is broken because the distinction between droppable components is very small and in some situations, it's hard to drop the item where you want. So, that's currently my main focus.

@tommoor
Copy link
Member

tommoor commented Jul 7, 2020

I'd agree with your assessment, particularly two things stand out:

  • Trying to drop into a nested doc is very finicky, there seems to be a very small amount of vertical space where you can drop
  • There are a couple of spots you can get in where it will just flicker back and forth.

Would it be a heavy lift to show a blue line where the doc will be dropped?

@anteprimorac
Copy link
Contributor Author

It's doable, but we will have to position it on our own because the library doesn't have support for it, but hopefully, I've found a few community examples. I think it's worth doing it because it will improve UX a lot and give us better visibility into other UX issues.

@tommoor
Copy link
Member

tommoor commented Jul 7, 2020

Alternatively a "ghosted" version of the document would probably be even better.

Really appreciate your willingness to get this to the appropriate level of polish 💪

@holtmeister
Copy link

holtmeister commented Jul 26, 2020

Any progress on this one? Am really keen to see it in action. Unfortunately I'm an end user and not a programmer tho :/ so I can't work on this myself...

@tommoor
Copy link
Member

tommoor commented Sep 9, 2020

Having seen the difficulty of implementing the ghost I'm not so fussed about that, really gotta just fine tune those drop areas though – I'm hoping to take a look this week – It's time this landed.

@icrotz
Copy link

icrotz commented Nov 17, 2020

@tommoor Hello, do you have any news on this PR ? Can we help on that ?

Thanks !

@tommoor
Copy link
Member

tommoor commented Nov 18, 2020

Yes, @thenanyu is currently rebuilding the sidebar to allow this to be finished – once we tried to get into the details of drag and drop between nested documents it became apparent that the current way it's built was not going to ever work well with react-beautiful-dnd.

That work is in progress now.

@tommoor
Copy link
Member

tommoor commented Dec 31, 2020

@anteprimorac wanted to say a heartfelt thanks for the effort on this PR, it pushed us forward and helped us to negotiate this work. In the end we had to rebuild the sidebar to not use the nested structure at all in order for dnd to work as users expect which made this PR unworkable.

You can see how it ended up here: #1722

@tommoor tommoor closed this Dec 31, 2020
@anteprimorac
Copy link
Contributor Author

@tommoor thank you, the nested structure was the main issue with dnd UX. I had started experimenting with react-dnd library that is more flexible than react-beautiful-dnd but I didn't have enough bandwidth to finish it.

Glad to see that you've managed to add this feature to the project.

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.

None yet

4 participants