Skip to content

Followup ListView hook refactor#3127

Merged
LFDanLu merged 4 commits into
mainfrom
followup_listview_hook_refactor
May 17, 2022
Merged

Followup ListView hook refactor#3127
LFDanLu merged 4 commits into
mainfrom
followup_listview_hook_refactor

Conversation

@LFDanLu
Copy link
Copy Markdown
Member

@LFDanLu LFDanLu commented May 13, 2022

Addresses followup comments from #3086 (review)
Removes duplicated intl files in favor of exporting useHighlightSelectionDescription from react-aria/grid and creating
useGridSelectionAnnouncement

✅ 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:

Test that selections made in ListView and TableView are still properly announced. Confirmation that the highlight selection announcement will need to be done outside the storybook due to main landmark element preventing announcement

🧢 Your Project:

RSP

LFDanLu added 2 commits May 13, 2022 13:33
exports some more things from aria/grid package and adds a new hook for selection announcements
Comment on lines +30 to +37
interface GridSelectionState<T> {
/** A collection of items in the grid. */
collection: Collection<Node<T>>,
/** A set of items that are disabled. */
disabledKeys: Set<Key>,
/** A selection manager to read and update multiple selection state. */
selectionManager: SelectionManager
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

A more generalized state type than GridState since we actually provide a state of type ListState from useList. I could make this GridState<T, GridCollection<T>> but would then need to do some typescript hackiness (read: as any) in useList's call of useGridSelectionAnnouncement

isVirtualized,
keyboardDelegate,
getRowText = (key) => state.collection.getItem(key)?.textValue,
getRowText,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No default provided here explicitly contrary to what the AriaListOptions state, but the default is setup in the useGridSelectionAnnouncement hook

@adobe-bot
Copy link
Copy Markdown

Build successful! 🎉

Copy link
Copy Markdown
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 in Safari VO MacOS

@adobe-bot
Copy link
Copy Markdown

Build successful! 🎉

@LFDanLu LFDanLu merged commit e67b29f into main May 17, 2022
@LFDanLu LFDanLu deleted the followup_listview_hook_refactor branch May 17, 2022 16:10
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.

5 participants