search

boonebgorges / buddypress-docs

GNU General Public License v3.0
106 stars 44 forks source link

Drag drop folders #621

Closed dcavins closed 6 years ago

dcavins commented 6 years ago

First pass at adding drag-and-drop doc-to-folder organization on the docs table. Adapts code that was already in folder.js for another purpose.

boonebgorges commented 6 years ago

Woo! Excited to see this.

I haven't looked at the code, but I did take a second to try it in action. Can you say a word or two about how it should work? I can't seem to find a drop target.

peek 2018-04-19 19-07

dcavins commented 6 years ago

Ha, I left an important line out of the commit. I committed the comment, but not the function call, to reinitialize the draggables and droppables after expanding a folder. :)

dcavins commented 6 years ago

Here's a video of it working: https://media.giphy.com/media/Zd5TayL5gGCFwZtIoj/giphy.gif

boonebgorges commented 6 years ago

Cool, seems to work! Might be nice to have some visual indicators of ajax-in-progress and success. Maybe an opacity drop while in progress, followed by a flashing border on success? Or something like that.

I won't have time in the foreseeable future to do a full code review. If there's any specific aspect you'd like reviewed, please let me know and I'll make a bit of time. Otherwise I trust your good judgment.

dcavins commented 6 years ago

Hi Boone-

Thanks for the feedback. I've made some updates to simplify the code a bit and improve the user feedback. Current example: updated-interaction

The outstanding todo is updating the meta row information: "Viewing 1-8 docs of 8 docs." We could add spans around those numbers and increment/decrement them as changes are made, but it feels a little gross. So if you have thoughts about that, or think the grossness is fine, let me know. Otherwise, this change is pretty close to working well in my testing.

Thanks!

boonebgorges commented 6 years ago

Awesome, the changes look good!

I would not worry about the pagination grossness for the time being. Handling it in the client seems like it's going to be a mess - full of corner cases. In the long run, another option might be to have a separate endpoint for fetching a context's pagination, or perhaps it could be sent back as part of the payload of the "drop" ajax request. (The former idea seems cleaner, though it requires another round trip.) But I don't think it's worth holding up this improvement for that.

dcavins commented 6 years ago

This code was merged into 2.1.x beginning with 2e189ec977f3e9327bd2275c68cbeeb16a9fdc52.