Thoughts on Collection API consistency about react-spectrum HOT 9 OPEN

My first thought here would be it would be a breaking change if we changed how things like empty state was provided to a collection component (or at least we'd have to deprecate renderEmptyState and support both ways of providing it). As for allowing the render function of the ListBox to have renderProps, is there a particular use case you are trying to achieve with it? The contents of the ListBox itself are pretty specific and rigid (aka it expects only sections and items) so it feels like it would mainly be used for styling based on the state of the listbox but the ListBoxItem's themselves have access to renderProps in their own render functions for this.

from react-spectrum.

Yup, this would absolutely be a breaking change, but in the name of consistency, flexibility and extensibility.

And just to reiterate, this is not exclusive to the ListBox. It is applicable to all RAC that implement the Collection API.

The use case is being able to access the entire collection state at the top level. Which would allow for the creation of custom components that can maintain the flexibility of static and dynamic collection items without needing to reimplement and control state.

This would enable the custom component to establish functionality based on this collection state such as filtering, searching, custom layout, etc. Alternatively, the custom component could establish context slots for composition of this functionality.

MyCustomComponent (receives all props of below RAC plus any custom, passes them through)
AnyRACCollectionComponent
MyCustomComponentInner (receives render props from above RAC plus any custom from parent)

Here the Inner component now has full access to the RAC internal state and nothing had to be reimplemented. It can now choose to establish slots with a Provider or custom layout and functionality, etc. And MyCustomComponent retained all of the flexibility of the underlying RAC

from react-spectrum.

A more concrete example would be to implement a De/Select All button. This would contextually need to know the state of the whole collection, what is currently selected, and access to the selection manager to be able to call methods.

As things currently stand the only way to do this is to drop support for statically defined items, implement useControlledState and useListState, and control the RAC completely from the parent thereby also allowing for the state and selection manager to be exposed as described above

With the proposed changes, the entire complexity of reimplementing state and selection would be negated

from react-spectrum.

I see, we've have historically suggested that users drop down to the hooks levels at this point but I see your point about wanting more flexibility here by making the Collection RAC components support more of a wrapper like api. I remember there were issues in the past (infinite rerenders/etc) when trying to provide the internal state object via render props for some of the collection components like ComboBox and Select, but those are a bit of a different case so I'm not sure if the same issues would arise. I'll bring this up with the team and see what opinions they have.

from react-spectrum.

Talked with the team and maybe the work in #6608 can help enable this for you. The exported Collection stuff from that PR will allow you to follow the same pattern that ComboBox and Select have aka the ListBox creates the collection of items that Combobox/Select can then use in their own hooks. That sounds like the structure you want here where you are sharing the state between a parent component and the actual collection component that is generating the collection of items.

We are a bit wary of performing the original proposed changes since it is such a big breaking change and since we weren't sure it would actually work in the dynamic case due to the parent state needing the collection to be passed in which would be problematic if ListBoxItems was now responsible for setting up the collection via CollectionBuilder (aka it is now at a lower level than were the top level state is setup)

from react-spectrum.

I'll check out the PR you linked in detail tomorrow. In the meantime, my gut reaction to how to make the original proposal work would be to have <ListBoxItems> be required to implement a slot prop, such that <ListBox> would pass down a ref and be able to access state / children. Technically unnecessary if the <ListBoxItems>'s context is just implemented on the DEFAULT_SLOT. Just an untested idea:

<ListBox
  aria-label="Pick a Pokemon"
  items={list.items}
  selectionMode="single"
>
  <ListBoxItems slot="items">{(item) => <ListBoxItem id={item.name}>{item.name}</ListBoxItem>}</ListBoxItems>
  <EmptyState slot="empty">No items</EmptyState>
</ListBox>

<ListBox
  aria-label="Pick a Pokemon"
  selectionMode="single"
>
  <ListBoxItems slot="items">
    <ListBoxItem>Foo</ListBoxItem>
    <ListBoxItem>Bar</ListBoxItem>
  </ListBoxItems>
  <EmptyState slot="empty">No items</EmptyState>
</ListBox>

So, <ListBox, implements <Provider values={[[ListboxItemsContext, { ref: itemsRef }]]}>. And potentially instead of the ref attaching to the DOM node, it connects to an imperative handle that exposes access to it's collection, maybe?

from react-spectrum.

The problem is that there's a circular dependency. In order to construct the state, you first need the collection. So we can't pass the state as a render prop because it depends on the children which would be returned by the render prop function rendering first.

from react-spectrum.

True, the initial render would be guaranteed to have an empty collection in the case of static jsx options (items prop would fulfill per usual, immediate if static, on second render if asynchronous/dynamic) until it was able to retrieve that state. Maybe not perfect, but not unworkable, and actually consistent with the asynchronous dynamic items

from react-spectrum.

Alternative approach: Support a third type of rendering combination

• Approach 1: items is undefined, children are static jsx collection items
• Approach 2: items is an array, children is an iterator function
• Approach 3: items is an array, children is a new component <CollectionRenderProps>{(renderProps) => {...}}</CollectionRenderProps>; where renderProps represents the full render props state of the parent RAC, which would include the collection, selectionManager, and any other applicable state (already outlined in the RAC's style renderProps)

No breaking change, no circular dependency, full access to internal state. Win, win, win?

And it should be relatively easy to determine which approach is being utilized to change how the collection is initialized

The only downside is the lack of support for static jsx items combined with being able to access render props children

from react-spectrum.