Skip to content

Conversation

@LFDanLu
Copy link
Member

@LFDanLu LFDanLu commented Mar 8, 2023

Closes

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

RSP

…thout

note: this is pretty broken, awaiting feedback from spectrum since it seems like there might be some cases that need to be handled
commited them so they wouldnt just be in my stash
using this information to get rid of the extra wrapping row group when there are sections
parent key of the section is the body but this key isnt in the keymap
renders extra whitespace for some reason
toggling selection on and off in a table with sections behaves strangely
pausing here since collectionbuilder will be changed in the future
…to a section

as per feedback from spectrum design meeting
…xist

also make ListView section have title as a required prop
@rspbot
Copy link

rspbot commented Mar 8, 2023

@rspbot
Copy link

rspbot commented May 3, 2023

@rspbot
Copy link

rspbot commented May 10, 2023

LFDanLu added 2 commits May 10, 2023 15:24
without the extra run all timers, the section header layout height is incorrectly stuck at 48 which is the default in listlayout but we are mocking scrollheight to 40 in the test which useVirtualizerItem should use to update the header height
@rspbot
Copy link

rspbot commented May 10, 2023

@rspbot
Copy link

rspbot commented May 10, 2023

@rspbot
Copy link

rspbot commented May 10, 2023

## API Changes

unknown top level export { type: 'identifier', name: 'Section' }
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' }
undefined already in set
undefined already in set
undefined already in set
undefined already in set

@react-aria/autocomplete

AriaSearchAutocompleteOptions

 AriaSearchAutocompleteOptions<T> {
   inputRef: RefObject<HTMLInputElement>
-  keyboardDelegate?: KeyboardDelegate
+  keyboardDelegate?: KeyboardDelegate<T>
   listBoxRef: RefObject<HTMLElement>
   popoverRef: RefObject<HTMLDivElement>
 }

it changed:

  • useSearchAutocomplete

@react-aria/combobox

AriaComboBoxOptions

 AriaComboBoxOptions<T> {
   buttonRef?: RefObject<Element>
   inputRef: RefObject<HTMLInputElement>
-  keyboardDelegate?: KeyboardDelegate
+  keyboardDelegate?: KeyboardDelegate<T>
   listBoxRef: RefObject<HTMLElement>
   popoverRef: RefObject<Element>
 }

it changed:

  • useComboBox

@react-aria/dnd

DroppableCollectionOptions

 DroppableCollectionOptions {
   dropTargetDelegate: DropTargetDelegate
-  keyboardDelegate: KeyboardDelegate
+  keyboardDelegate: KeyboardDelegate<unknown>
 }

it changed:

  • useDroppableCollection

DropIndicatorProps

 DropIndicatorProps {
+  keyboardDelegate?: KeyboardDelegate<unknown>
   target: DropTarget
 }

it changed:

  • useDropIndicator

@react-aria/grid

useGrid

changed by:

  • GridProps
 useGrid<T> {
-  props: GridProps
+  props: GridProps<T>
   state: GridState<T, GridCollection<T>>
   ref: RefObject<HTMLElement>
   returnVal: undefined
 }

GridProps

-GridProps {
-  focusMode?: 'row' | 'cell' = 'row'
-  getRowText?: (Key) => string = (key) => state.collection.getItem(key)?.textValue
-  isVirtualized?: boolean
-  keyboardDelegate?: KeyboardDelegate
-  onCellAction?: (Key) => void
-  onRowAction?: (Key) => void
-  scrollRef?: RefObject<HTMLElement>
-}
+

it changed:

  • useGrid

useGridSectionAnnouncement

-
+useGridSectionAnnouncement<T> {
+  state: GridSelectionState<T>
+  returnVal: undefined
+}

undefined

-
+GridProps<T> {
+  focusMode?: 'row' | 'cell' = 'row'
+  getRowText?: (Key) => string = (key) => state.collection.getItem(key)?.textValue
+  isVirtualized?: boolean
+  keyboardDelegate?: KeyboardDelegate<T>
+  onCellAction?: (Key) => void
+  onRowAction?: (Key) => void
+  scrollRef?: RefObject<HTMLElement>
+}

@react-aria/gridlist

AriaGridListOptions

 AriaGridListOptions<T> {
   disabledBehavior?: DisabledBehavior
   isVirtualized?: boolean
-  keyboardDelegate?: KeyboardDelegate
+  keyboardDelegate?: KeyboardDelegate<T>
   onAction?: (Key) => void
 }

it changed:

  • useGridList

useGridListSection

-
+useGridListSection<T> {
+  props: AriaGridListSectionOptions
+  state: ListState<T>
+  returnVal: undefined
+}

@react-aria/listbox

AriaListBoxProps

 AriaListBoxOptions<T> {
   isVirtualized?: boolean
-  keyboardDelegate?: KeyboardDelegate
+  keyboardDelegate?: KeyboardDelegate<T>
   shouldFocusOnHover?: boolean
   shouldSelectOnPressUp?: boolean
   shouldUseVirtualFocus?: boolean
 }

@react-aria/menu

AriaMenuOptions

 AriaMenuOptions<T> {
   isVirtualized?: boolean
-  keyboardDelegate?: KeyboardDelegate
+  keyboardDelegate?: KeyboardDelegate<T>
 }

it changed:

  • useMenu

@react-aria/select

AriaSelectOptions

 AriaSelectOptions<T> {
-  keyboardDelegate?: KeyboardDelegate
+  keyboardDelegate?: KeyboardDelegate<T>
 }

it changed:

  • useSelect

@react-aria/selection

useSelectableCollection

changed by:

  • AriaSelectableCollectionOptions
-useSelectableCollection {
-  options: AriaSelectableCollectionOptions
-  returnVal: undefined
-}
+

useSelectableList

changed by:

  • AriaSelectableListOptions
-useSelectableList {
-  props: AriaSelectableListOptions
-  returnVal: undefined
-}
+

AriaSelectableCollectionOptions

-AriaSelectableCollectionOptions {
-  allowsTabNavigation?: boolean
-  autoFocus?: boolean | FocusStrategy = false
-  disallowEmptySelection?: boolean = false
-  disallowSelectAll?: boolean = false
-  disallowTypeAhead?: boolean = false
-  isVirtualized?: boolean
-  keyboardDelegate: KeyboardDelegate
-  ref: RefObject<HTMLElement>
-  scrollRef?: RefObject<HTMLElement>
-  selectOnFocus?: boolean = false
-  selectionManager: MultipleSelectionManager
-  shouldFocusWrap?: boolean = false
-  shouldUseVirtualFocus?: boolean
-}
+

it changed:

  • useSelectableCollection

AriaSelectableListOptions

-AriaSelectableListOptions {
-  allowsTabNavigation?: boolean
-  autoFocus?: boolean | FocusStrategy = false
-  collection: Collection<Node<unknown>>
-  disabledKeys: Set<Key>
-  disallowEmptySelection?: boolean = false
-  disallowTypeAhead?: boolean = false
-  isVirtualized?: boolean
-  keyboardDelegate?: KeyboardDelegate
-  ref?: RefObject<HTMLElement>
-  selectOnFocus?: boolean = false
-  selectionManager: MultipleSelectionManager
-  shouldFocusWrap?: boolean = false
-  shouldUseVirtualFocus?: boolean
-}
+

it changed:

  • useSelectableList

AriaTypeSelectOptions

 AriaTypeSelectOptions {
-  keyboardDelegate: KeyboardDelegate
+  keyboardDelegate: KeyboardDelegate<unknown>
   onTypeSelect?: (Key) => void
   selectionManager: MultipleSelectionManager
 }

it changed:

  • useTypeSelect

undefined

-
+useSelectableCollection<T> {
+  options: AriaSelectableCollectionOptions<T>
+  returnVal: undefined
+}

undefined

-
+useSelectableList<T> {
+  props: AriaSelectableListOptions<T>
+  returnVal: undefined
+}

undefined

-
+AriaSelectableCollectionOptions<T> {
+  allowsTabNavigation?: boolean
+  autoFocus?: boolean | FocusStrategy = false
+  disallowEmptySelection?: boolean = false
+  disallowSelectAll?: boolean = false
+  disallowTypeAhead?: boolean = false
+  isVirtualized?: boolean
+  keyboardDelegate: KeyboardDelegate<T>
+  ref: RefObject<HTMLElement>
+  scrollRef?: RefObject<HTMLElement>
+  selectOnFocus?: boolean = false
+  selectionManager: MultipleSelectionManager
+  shouldFocusWrap?: boolean = false
+  shouldUseVirtualFocus?: boolean
+}

undefined

-
+AriaSelectableListOptions<T> {
+  allowsTabNavigation?: boolean
+  autoFocus?: boolean | FocusStrategy = false
+  collection: Collection<Node<unknown>>
+  disabledKeys: Set<Key>
+  disallowEmptySelection?: boolean = false
+  disallowTypeAhead?: boolean = false
+  isVirtualized?: boolean
+  keyboardDelegate?: KeyboardDelegate<T>
+  ref?: RefObject<HTMLElement>
+  selectOnFocus?: boolean = false
+  selectionManager: MultipleSelectionManager
+  shouldFocusWrap?: boolean = false
+  shouldUseVirtualFocus?: boolean
+}

@react-spectrum/list

ListSection

-
+SpectrumListSectionProps<T> {
+  title: ReactNode
+}

@react-spectrum/tag

TagGroup

 TagGroup<T extends {}> {
   actionLabel?: string
   onAction?: () => void
-  renderEmptyState?: () => JSX.Element
 }

SpectrumTagGroupProps

 SpectrumTagGroupProps<T> {
   actionLabel?: string
   onAction?: () => void
-  renderEmptyState?: () => JSX.Element
 }

@react-stately/layout

ListLayout

 ListLayout<T> {
   allowDisabledKeyFocus: boolean
   buildChild: (Node<T>, number, number) => LayoutNode
   buildCollection: () => Array<LayoutNode>
   buildItem: (Node<T>, number, number) => LayoutNode
   buildNode: (Node<T>, number, number) => LayoutNode
   buildSection: (Node<T>, number, number) => LayoutNode
   collection: Collection<Node<T>>
   constructor: (ListLayoutOptions<T>) => void
   disabledKeys: Set<Key>
   getContentSize: () => void
   getDropTargetFromPoint: (number, number, (DropTarget) => boolean) => DropTarget
   getFinalLayoutInfo: (LayoutInfo) => void
-  getFirstKey: () => Key | null
+  getFirstKey: (any, any, (Node<T>) => boolean) => Key | null
   getInitialLayoutInfo: (LayoutInfo) => void
-  getKeyAbove: (Key) => Key | null
-  getKeyBelow: (Key) => Key | null
+  getKeyAbove: (Key, (Node<T>) => boolean) => Key | null
+  getKeyBelow: (Key, (Node<T>) => boolean) => Key | null
   getKeyForSearch: (string, Key) => Key | null
-  getKeyPageAbove: (Key) => Key | null
-  getKeyPageBelow: (Key) => Key | null
-  getLastKey: () => Key | null
+  getKeyPageAbove: (Key, (Node<T>) => boolean) => Key | null
+  getKeyPageBelow: (Key, (Node<T>) => boolean) => Key | null
+  getLastKey: (any, any, (Node<T>) => boolean) => Key | null
   getLayoutInfo: (Key) => void
   getVisibleLayoutInfos: (Rect) => void
   isLoading: boolean
   isValid: (Node<T>, number) => void
   updateItemSize: (Key, Size) => void
   updateLayoutNode: (Key, LayoutInfo, LayoutInfo) => void
   validate: (InvalidationContext<Node<T>, unknown>) => void
 }
 

@react-stately/select

SelectStateOptions

-SelectStateOptions<T> {
 
-}

it changed:

  • useSelectState

@LFDanLu LFDanLu changed the title (WIP) Supporting sections in ListView Supporting sections in ListView May 10, 2023
@LFDanLu
Copy link
Member Author

LFDanLu commented May 10, 2023

PR is in a functionally stable state now (behavior/implementation wise). Will either revisit when a RAC collection transition happens to ListView or we can aim to review/move ahead with this as is.

Comment on lines +93 to +94
// Allow section layout info to be updated as a result of the initial render's useVirtualizerItem call
act(() => {jest.runAllTimers();});
Copy link
Member Author

Choose a reason for hiding this comment

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

This is actually something that we are missing in our tests in main for ListBox, we just haven't encountered it because we don't have PageUp/Down tests. useVirtualizerItem updates the section header height in a useLayoutEffect to match the test's mocked scrollHeight from the default of 48 which ListLayout sets initially. This virtualizer.updateItemSize call triggers a raf here, hence the need for a second runAllTimers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants