rfc: Adopting Async React in React Aria Components#9894
rfc: Adopting Async React in React Aria Components#9894devongovett wants to merge 12 commits intomainfrom
Conversation
|
Build successful! 🎉 |
|
Build successful! 🎉 |
738e9d9 to
f08bc63
Compare
|
Build successful! 🎉 |
51e810e to
ec5a08a
Compare
|
Build successful! 🎉 |
| function Example() { | ||
| return ( | ||
| <ListBox> | ||
| <ListBoxSuspense | ||
| // Initial loading state | ||
| fallback={<ProgressCircle />} | ||
| renderError={error => `Error loading data: ${error}`}> | ||
| <Page url="https://pokeapi.co/api/v2/pokemon" /> | ||
| </ListBoxSuspense> | ||
| </ListBox> | ||
| ); | ||
| } |
There was a problem hiding this comment.
so just to verify the API: as mentioned below about the composability, a user who wants to have sections that each load from a different data source and have individual spinners would have separate <ListBoxSuspense>s wrapping a different <Page> (or w/e they would call it in their implementation), and each Page would be a Collection that renders that individual section and its async loaded items? I guess the top level suspense wouldn't be necessary unless they wanted a spinner to wait for all of the section to do an initial load?
There was a problem hiding this comment.
yeah exactly. if you wanted a single loading state for all sections, you'd wrap the suspense around the entire list. If you wanted separate loading states for each section, you'd wrap suspense around each section individually.
There was a problem hiding this comment.
Hm, I'm kinda wondering about the styling logistics of that. Loader items typically consume very little height, so one would likely always trigger multiple section loads when reaching the end of the first section, while you would typically only want to load the successor? Can probably be solved with styling in userland, but wondering whether there should maybe be something more intuitive to account for that.
There was a problem hiding this comment.
As I've done in this example with the Page component, you could nest them so that the second section only loads after the first one (if it is in view). Whether you want parallel or sequential loading, separate or a single spinner, is all up to the application / design requirements.
|
|
||
| * Do pending states make sense in all of these components? Supporting these within Spectrum will require input from design. | ||
| * How do we want to support pending states that aren't displayed as a progress bar / spinner (e.g. a "shimmer")? We may need to announce something, even if a progress bar is not present in the DOM. | ||
| * How do we want to handle components that have both a loading state for data and a pending state for an action? For example, Select and ComboBox support loading states for their list of items, but may also support a changeAction when the user selects an item. Would these both display the same spinner UI? |
There was a problem hiding this comment.
ideally, I think we'd want this to be customizable by the user, so perhaps we have additional render props along side isPending/or some way of informing the user what triggered said pending state?
| * Do pending states make sense in all of these components? Supporting these within Spectrum will require input from design. | ||
| * How do we want to support pending states that aren't displayed as a progress bar / spinner (e.g. a "shimmer")? We may need to announce something, even if a progress bar is not present in the DOM. | ||
| * How do we want to handle components that have both a loading state for data and a pending state for an action? For example, Select and ComboBox support loading states for their list of items, but may also support a changeAction when the user selects an item. Would these both display the same spinner UI? | ||
| * For components with multiple actions, do we want individual pending states (e.g. `isChangePending`, `isSubmitPending`) or a single pending state that aggregates these? |
There was a problem hiding this comment.
I think I'd lean towards an aggregated pending state for now?
| }); | ||
| }, [setControlledValue, setOptimisticValue, setOptimisticError, changeAction]); | ||
|
|
||
| return [optimisticValue, isPending, setValue, optimisticError]; |
There was a problem hiding this comment.
nit: https://react.dev/reference/react/useActionState has the isPending ordered after the setter, might want to do the same?
| function Example() { | ||
| return ( | ||
| <ListBox> | ||
| <ListBoxSuspense | ||
| // Initial loading state | ||
| fallback={<ProgressCircle />} | ||
| renderError={error => `Error loading data: ${error}`}> | ||
| <Page url="https://pokeapi.co/api/v2/pokemon" /> | ||
| </ListBoxSuspense> | ||
| </ListBox> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Hm, I'm kinda wondering about the styling logistics of that. Loader items typically consume very little height, so one would likely always trigger multiple section loads when reaching the end of the first section, while you would typically only want to load the successor? Can probably be solved with styling in userland, but wondering whether there should maybe be something more intuitive to account for that.
| ## Open Questions | ||
|
|
||
| * Do pending states make sense in all of these components? Supporting these within Spectrum will require input from design. | ||
| * How do we want to support pending states that aren't displayed as a progress bar / spinner (e.g. a "shimmer")? We may need to announce something, even if a progress bar is not present in the DOM. |
There was a problem hiding this comment.
It would have to be a user-supplied component or a very generic announcement though, right? Otherwise how would we pull the appropriate context/msg for announcement?
There was a problem hiding this comment.
Generic announcement is better than no announcement. We could at least use the accessible name of the element that's pending so it could announce something like "Save, pending" if the button's label was "Save". For a more contextual message, perhaps a prop.
There was a problem hiding this comment.
Sure, but that is only if the pending element has an accessible name in the first place. This is the case with all components that useLabel currently, which is mostly fields, but for the ones that dont it might be difficult / breaking to require? Also see this somewhat related issue I opened a while back #7240 (comment)
There was a problem hiding this comment.
All components that would support a pending state already require some kind of accessiblity label. Even components that don't use useLabels internally still have a visible label / aria-label.
There was a problem hiding this comment.
The vast majority of the proposed ones yeah, because as I meant, they are mostly fields. Some of them are exceptions though, e.g. Tabs, ToggleButton and Disclosure, which neither have a default aria-label nor enforce presence of a aria-label prop or visible label. That can probably be worked around though with some sane defaults, so its not a strict blocker.
There was a problem hiding this comment.
All of those require visible labels (children) / aria-labels too. We might not always emit a warning ourselves (that can be improved) but accessibility checkers like aXe definitely will.
View Rendered RFC
Comment here
This PR also includes a prototype implementation in several components for discussion.