cms icon indicating copy to clipboard operation
cms copied to clipboard

[5.x] Ability to drag/drop sets between replicators

Open jacksleight opened this issue 1 year ago • 4 comments

This PR adds the ability to drag and drop sets between different replicators:

https://github.com/statamic/cms/assets/126740/a3b0c63d-6e16-4d45-a134-a6e9c4247870

Notes

  • I had to change the way the draggable instances are initialised so that the same instance can be shared between multiple lists, but this shouldn't impact any non-replicator lists.
  • The sortable list component accepts a new group prop that tells it which instance to share, and a groupValidator function that's used to decide whether the item is valid in the target container.
  • Replicators use a list of set config hashes to decide whether the set being dragged is compatible with the target replicator.
  • I had to give the set container a minimum height so that there was still a drop area available when the replicator is empty. This may need some adjustment.
  • If group is set the input event emitted from the sortable list component has different data, allowing the parent component to decide how to handle the changes.
  • The sortable list move helper function has been moved to globals and renamed/split up so that functionality can be used elsewhere.
  • I’ve added not-allowed cursors when dragging sets over incompatible replicators. The only way I could make that work well with nested replicators was by using the new-ish CSS scope feature. That has decent but not full browser support right now.
  • Annoyingly there’s a bug in Shopify draggable that causes an uncaught error when dragging a set into an empty replicator. It doesn’t break anything, but it shows up in console. A fix was submitted but it doesn’t look like anything is happening with it unfortunately.

Other Changes

I’ve made a couple of changes that are not strictly drag-and-drop related, but were required to implement this nicely:

  • The hashes and blink keys implemented in https://github.com/statamic/cms/pull/10280 have been slightly refactored. This PR requires hashes for each set type, and that PR needs the same thing. Rather than generating those twice both features now use the same list of set hashes.
  • I’ve also tweaked some of the other hashing to avoid having to hash the same config multiple times (which adds unnecessary overhead). Each field instance now only hashes its config once, which is implemented via a new configHash() method on the Field class (only called when needed).
  • I’ve updated a test that was creating a Bard object without setting a field. This meant that the field could be null when generating the config hash, resulting in blink keys that are pretty meaningless. Rather than allowing null I thought it made more sense to adjust the test.

jacksleight avatar Jun 24 '24 20:06 jacksleight

Love this idea! From a UX perspective, it might be a good idea to add some sort of indicator if a set can be placed in another replicator. Like in your demo above, you can't move an image set into a text-only replicator. But this is not apparent to the user. So that's quite confusing.

aerni avatar Jun 28 '24 15:06 aerni

This is ready for review!

@aerni Following your suggestion I added the not-allowed cursors, but I think you probably meant something more, like highlighting the compatible replicators? I tried to come up with a nice UI for that but couldn't figure out anything that a) was obvious but not too obtrusive and b) worked well with nested replicators. I’ll wait for Jack and the team to review and see if they have any suggestions.

jacksleight avatar Jul 04 '24 10:07 jacksleight

The not-allowed cursor is already a good hint. But yeah, would probably reduce the opacity on the replicators or something like that. Let's see what Jack thinks.

aerni avatar Jul 04 '24 14:07 aerni

would probably reduce the opacity on the replicators

This would be slick, and I tried it, but if you're dragging between nested replicators and the parent isn't compatible that will fade out, fading out the nested ones as well.

jacksleight avatar Jul 04 '24 14:07 jacksleight

@jacksleight This looks fantastic! I've been dreaming about this exact feature, just for asset fields. Do you see some sort of overlap between such a feature and the additions in your PR, in terms of code re-use? Or would this be better off in a separate implementation over in asset land?

daun avatar Dec 06 '24 16:12 daun

@daun I think you could totally make use of some of the additions here. Only about a half of this is replicator specific, the other half is general changes to the global sortable list component that allows:

  1. Putting multiple lists into a group, so items can be dragged between them.
  2. Changes to the events that are fired if a group is set, so you can handle move-out/in and validate drop targets.

jacksleight avatar Dec 06 '24 17:12 jacksleight

@jacksleight Nice! I'll wait until this one gets merged then, and see what I can re-use if/when I give it a try.

daun avatar Dec 06 '24 17:12 daun