search

adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
13.08k stars 1.14k forks source link

Single-item GridList with DragAndDrop reorder caveats #7386

Open jgarplind opened 1 week ago

jgarplind commented 1 week ago

Provide your feedback here.

We're using GridList with drag and drop for re-ordering. Depending on the dynamically sourced data, it happens that the GridList only contains one GridListItem.

When that happens, we would prefer not to have to render a <Button slot="drag">. However, this warning is emitted, causing us to reconsider: https://github.com/adobe/react-spectrum/blob/d8161f6a9157193da3ab66b732f660300a39b720/packages/react-aria-components/src/GridList.tsx#L337-L342

I tried following the logic and found myself at https://github.com/adobe/react-spectrum/blob/69109f0036d58b0afe7b3539c69361bec934aa43/packages/react-aria-components/src/useDragAndDrop.tsx#L114

I could easily do (and tried) something like getItems: myList.length <= 1 ? undefined : regularLogic, and that worked fine, except, the behavior was not disabled. In other words, I could still reach the reordering state with keyboard interactions, I just couldn't see where the focus went when it would previously have gone to the drag handle.

In the end, combining isDisabled: myList.length <= 1 with the getItems-shenanigans from above seems to seal the deal, but it left me with a feeling of having battled rather than collaborated with the library.

I think there are a few different approaches here, and would be happy to learn which you think is most in line with how this library is intended to function:

  1. Bypass this whole issue by conditionally rendering a GridList if there are 2+ items in the list, opting for other solutions otherwise.
  2. Just accept that the drag handle is visible even when it has no functionality.
  3. Do something like what was described above.

🔦 Context

We're just battling some warnings trying to write maintainable, user friendly code.

💻 Code Sample

No response

Version

react-aria-components 1.4.1

What browsers are you seeing the problem on?

No response

If other, please specify

No response

What operating system are you using?

No response

nwidynski commented 9 hours ago

@jgarplind I guess the issue lies with the overloaded usage of drag and drop for re-ordering. Even when re-ordering might infeasible, a drag action of the item remains possible (e.g. dragging item into another list), thus the warning is still required.

I think the best way forward in your scenario would be to conditionally pass undefined to the dragAndDropHooks property of <GridList /> 👍

jgarplind commented 8 hours ago

Thanks for weighing in!

Conditionally passing undefined was our first solution, but IIRC it broke things quite badly when the condition changed between renders on an already mounted component (React had to handle a different amount of hooks, no bueno).

Currently we have settled for letting the warning be, since we believe we know our use case good enough to ignore it, but it's still a bit unfortunate to have it slightly polluting the console.

nwidynski commented 8 hours ago

I see - only thing that comes to mind then would be to intercept the button context and manually invoke the callback ref that's normally spread onto the drag button. This is pretty ugly but at least it gets you what you are looking for.

I don't think there are many options for the react-aria team to have this behavior built-in, besides maybe skipping the warning when the item is disabled 👍