Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code changes for React Aria Components #4150

Merged
merged 7 commits into from Mar 6, 2023

Conversation

devongovett
Copy link
Member

This PR is the first in a series, implementing the React Aria Components RFC. This pulls out the code changes to our existing hooks necessary to support the new implementation. I tried to separate the code into individual commits for different areas:

  • Returning interaction states from more of the hooks, e.g. isPressed, isFocused, isSelected, etc.
  • Improving the TypeScript definitions to be more reusable/correct. There were also some Spectrum-specific things in some of the types that shouldn't have been there.
  • Updates to support the new collections implementation:
    • You can now pass in a collection as an option to the Stately hooks, which will be used instead of the default implementation which walks the JSX tree. Closes Improve extensibility #3405.
    • The childNodes property of a Node is deprecated in favor of a collection.getChildren(key) method, which allows immutable Node objects to be more easily cloned without needing to do so recursively to all parents. I added an internal util which supports both options for backward compatibility.
  • Implements the disabledBehavior prop for useTable and useGrid as we have in useGridList (though not currently in the Spectrum implementation).
  • Fixes a couple bugs with our mergeRefs and useObjectRefs utilities.
  • Some very minor documentation fixes.

@rspbot
Copy link

rspbot commented Mar 2, 2023

@@ -19,7 +21,12 @@ interface GridCollectionOptions {

const ROW_HEADER_COLUMN_KEY = 'row-header-column-' + Math.random().toString(36).slice(2);

function buildHeaderRows<T>(keyMap: Map<Key, GridNode<T>>, columnNodes: GridNode<T>[]): GridNode<T>[] {
/** @private */
export function buildHeaderRows<T>(keyMap: Map<Key, GridNode<T>>, columnNodes: GridNode<T>[]): GridNode<T>[] {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about exporting this. I need it for the new TableCollection implementation, which currently lives in the react-aria-components package. I suppose I could copy paste it but 🤷

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it was copy pasted to the react-aria-components, would this copy in stately go away stop being used if we replaced our table implementation with the react-aria-component?

if so, then I think I'm more ok with copying it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also un-export it later if we move the new implementation into here.

@rspbot
Copy link

rspbot commented Mar 2, 2023

@rspbot
Copy link

rspbot commented Mar 2, 2023

## API Changes

unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }

@react-aria/checkbox

CheckboxAria

 CheckboxAria {
   inputProps: InputHTMLAttributes<HTMLInputElement>
+  isDisabled: boolean
+  isPressed: boolean
+  isReadOnly: boolean
+  isSelected: boolean
 }

it changed:

  • useCheckbox

@react-aria/overlays

OverlayTriggerAria

 OverlayTriggerAria {
-  overlayProps: DOMAttributes
+  overlayProps: DOMProps
   triggerProps: AriaButtonProps
 }

it changed:

  • useOverlayTrigger

@react-aria/progress

AriaProgressBarProps

 AriaProgressBarProps {
   formatOptions?: Intl.NumberFormatOptions = {style: 'percent'}
   isIndeterminate?: boolean
   label?: ReactNode
   maxValue?: number = 100
   minValue?: number = 0
-  showValueLabel?: boolean
   value?: number = 0
   valueLabel?: ReactNode
 }

@react-aria/radio

RadioAria

 RadioAria {
   inputProps: InputHTMLAttributes<HTMLInputElement>
   isDisabled: boolean
+  isPressed: boolean
   isSelected: boolean
 }

it changed:

  • useRadio

@react-aria/selection

SelectableItemStates

 SelectableItemStates {
   allowsSelection: boolean
   hasAction: boolean
   isDisabled: boolean
+  isFocused: boolean
   isPressed: boolean
   isSelected: boolean
 }

SelectableItemAria

 SelectableItemAria {
   allowsSelection: boolean
   hasAction: boolean
   isDisabled: boolean
+  isFocused: boolean
   isPressed: boolean
   isSelected: boolean
   itemProps: DOMAttributes
 }

it changed:

  • useSelectableItem

@react-aria/switch

SwitchAria

 SwitchAria {
   inputProps: InputHTMLAttributes<HTMLInputElement>
+  isDisabled: boolean
+  isPressed: boolean
+  isReadOnly: boolean
+  isSelected: boolean
 }

it changed:

  • useSwitch

@react-aria/tabs

TabAria

 TabAria {
   isDisabled: boolean
+  isPressed: boolean
   isSelected: boolean
   tabProps: DOMAttributes
 }

it changed:

  • useTab

@react-aria/toggle

ToggleAria

 ToggleAria {
   inputProps: InputHTMLAttributes<HTMLInputElement>
+  isDisabled: boolean
+  isPressed: boolean
+  isReadOnly: boolean
+  isSelected: boolean
 }

it changed:

  • useToggle

@react-spectrum/datepicker

DatePicker

 SpectrumDateFieldProps<T extends DateValue> {
   granularity?: Granularity
   hideTimeZone?: boolean = false
   hourCycle?: number | number
   isDateUnavailable?: (DateValue) => boolean
   isQuiet?: boolean = false
   maxValue?: DateValue
   minValue?: DateValue
   placeholderValue?: DateValue
-  shouldFlip?: boolean = true
   showFormatHelpText?: boolean = false
 }

@react-stately/collections

useCollection

 useCollection<C extends Collection<Node<T>> = Collection<Node<T>>, T extends {}> {
-  props: CollectionBase<T>
+  props: CollectionStateBase<T, C>
   factory: CollectionFactory<T, C>
   context?: unknown
   invalidators: Array<any>
   returnVal: undefined
 

getItemCount

 getItemCount<T> {
-  collection: Iterable<Node<T>>
+  collection: Collection<Node<T>>
   returnVal: undefined
 }

getChildNodes

-
+getChildNodes<T> {
+  node: Node<T>
+  collection: Collection<Node<T>>
+  returnVal: undefined
+}

getFirstItem

-
+getFirstItem<T> {
+  iterable: Iterable<T>
+  returnVal: undefined
+}

getLastItem

-
+getLastItem<T> {
+  iterable: Iterable<T>
+  returnVal: undefined
+}

getNthItem

-
+getNthItem<T> {
+  iterable: Iterable<T>
+  index: number
+  returnVal: undefined
+}

@react-stately/grid

GridCollection

 GridCollection<T> {
   at: (number) => void
   columnCount: number
   constructor: (GridCollectionOptions<T>) => void
+  getChildren: (Key) => Iterable<GridNode<T>>
   getFirstKey: () => void
   getItem: (Key) => void
   getKeyAfter: (Key) => void
   getKeyBefore: (Key) => void
   getLastKey: () => void
   keyMap: Map<Key, GridNode<T>>
   rows: Array<GridNode<T>>
   size: any
   undefined: () => void
 }
 

@react-stately/list

ListCollection

 ListCollection<T> {
   at: (number) => void
   constructor: (Iterable<Node<T>>) => void
+  getChildren: (Key) => Iterable<Node<T>>
   getFirstKey: () => void
   getItem: (Key) => void
   getKeyAfter: (Key) => void
   getKeyBefore: (Key) => void
   getLastKey: () => void
   size: any
   undefined: () => void
 }
 

@react-stately/menu

MenuTriggerProps

 MenuTriggerProps {
-  align?: Alignment = 'start'
-  closeOnSelect?: boolean = true
-  direction?: 'bottom' | 'top' | 'left' | 'right' | 'start' | 'end' = 'bottom'
-  shouldFlip?: boolean = true
   trigger?: MenuTriggerType = 'press'
 }

@react-stately/radio

RadioGroupState

 RadioGroupState {
   isDisabled: boolean
   isReadOnly: boolean
+  isRequired: boolean
   lastFocusedValue: string | null
   selectedValue: string | null
   setLastFocusedValue: (string) => void
   setSelectedValue: (string) => void
-  validationState: ValidationState
+  validationState: ValidationState | null
 }

it changed:

  • useRadioGroupState

@react-stately/select

useSelectState

 useSelectState<T extends {}> {
-  props: SelectProps<T>
+  props: SelectStateOptions<T>
   returnVal: undefined
 }

@react-stately/table

TableCollection

changed by:

  • TableCollection
 TableCollection<T> {
   _size: number
   at: (number) => void
   body: GridNode<T>
   columns: Array<GridNode<T>>
-  constructor: (Iterable<GridNode<T>>, TableCollection<T>, GridCollectionOptions) => void
+  constructor: (Iterable<GridNode<T>>, ITableCollection<T>, GridCollectionOptions) => void
   getFirstKey: () => void
   getItem: (Key) => void
   getKeyAfter: (Key) => void
   getKeyBefore: (Key) => void
   getLastKey: () => void
   headerRows: Array<GridNode<T>>
   rowHeaderColumnKeys: Set<Key>
   size: any
   undefined: () => void
 }
 

it changed:

  • TableCollection

@react-stately/tabs

useTabListState

changed by:

  • TabListStateOptions
 useTabListState<T extends {}> {
-  props: TabListProps<T>
+  props: TabListStateOptions<T>
   returnVal: undefined
 }

TabListStateOptions

+TabListStateOptions<T> {
 
+}

it changed:

  • useTabListState

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested most in chrome, lgtm

Comment on lines +24 to +28
if (props.collection) {
return props.collection;
}

let nodes = builder.build(props as CollectionBase<T>, context);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'm trying to understand the collection changes here. Previously we would use the items + render functions or the direct children provided to a collection component (traversing JSX tree) to generate the collection of nodes via CollectionBuilder and the provided factory (aka TableCollection/ListCollection/etc). This meant when items changed, we'd get an updated collection, or in Table's case there was some additional invalidationContext stuff that would cause the collection to update its nodes (aka selectionMode, etc)

With this change, we can now provide a collection directly through the props and skip the CollectionBuilder stuff. The initial collection would be built outside via extending BaseCollection like shown for the aria Table component. Any updates (like item removals/content updates) are now handled by the new useCollection and more specifically by the fake DOM approach mentioned here using Document which will return updated collections via .getCollection

Is this accurate? So essentially we won't need the old invalidation context approach since the fake DOM Table would rerender with all the required updates? For example, we used to need to invalidate each Row to rebuild its children to include the checkbox cell if selectionMode went from "none" to "single/multiple", but with the new collections approach the table in the fake DOM would rerender with checkbox cells, giving us a new up to date collection? This last part I'm the most unsure about, still looking at the Table aria components implementation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's all correct. In the new Table component, the checkbox column isn't a special column that needs to be added while building the collection anymore. It's just a normal column in the table, as you can see in the docs. If you want to provide a component where the checkboxes are automatically added like we do in Spectrum, then you'd create wrapper Row and TableHeader components like this. This adds the extra cells and uses composition to nest the rest of the collection. This all works because the components are just normal React components, and React takes care of rendering everything and efficiently updating our fake DOM instead of needing CollectionBuilder to do that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, thanks for the docs links, that cleared up what the Collection component is used for for me as well. Definitely much simpler than the CollectionBuild stuff for sure!

@devongovett devongovett merged commit 4cbeeaf into main Mar 6, 2023
@devongovett devongovett deleted the aria-components-code-changes branch March 6, 2023 23:24
@snowystinger snowystinger mentioned this pull request May 26, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve extensibility
4 participants