Conversation
snowystinger
left a comment
Member
There was a problem hiding this comment.
Hi, thanks for the PR. You have two other open PRs with waiting comments. Please revisit those before opening anymore.
| : {}, | ||
| { | ||
| role: 'listbox', | ||
| 'aria-disabled': props.isDisabled || undefined, |
Member
There was a problem hiding this comment.
I don't think aria-disabled should be be applied for disabledBehavior: 'selection'
I think we've forgotten to include it in the props for ListBox, but it's used in RAC ListBox.
| selectionManager: state.selectionManager, | ||
| collection: state.collection, | ||
| disabledKeys: state.disabledKeys, | ||
| disabledKeys: props.isDisabled ? new Set(state.collection.getKeys()) : state.disabledKeys, |
Member
There was a problem hiding this comment.
don't love this extra full collection iteration, at the very least we should memo it.
otherwise, we'll want to consider expanding the api to "all" or a new isDisabled prop meaning the entire selectable list. That may end up adding the new partial behaviour to every selectable list though, so would need to be careful with that.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5837
Summary
Add a top-level
isDisabledprop to ListBox. The prop is applied in the shared list state so every item key is treated as disabled by the existing selection, focus, and interaction machinery. The listbox also exposesaria-disabled="true", and a Storybook example demonstrates the disabled state.This lets applications disable a ListBox without enumerating all of its items in
disabledKeys, including dynamically loaded collections.✅ Pull Request Checklist:
📝 Test Instructions:
Focused tests:
yarn test packages/@adobe/react-spectrum/test/listbox/ListBox.test.js --runInBandyarn test packages/react-aria-components/test/ListBox.test.js --runInBandyarn test:ssr packages/@adobe/react-spectrum/test/listbox/ListBox.ssr.test.js packages/react-aria-components/test/ListBox.ssr.test.js --runInBandyarn check-typesyarn oxlint packages/react-stately/src/list/useListState.ts packages/react-aria/src/listbox/useListBox.ts packages/react-aria-components/test/ListBox.test.js packages/@adobe/react-spectrum/test/listbox/ListBox.test.jsyarn oxfmt --check packages/react-stately/src/list/useListState.ts packages/react-aria/src/listbox/useListBox.ts packages/react-aria-components/test/ListBox.test.js packages/@adobe/react-spectrum/test/listbox/ListBox.test.js packages/@adobe/react-spectrum/stories/listbox/ListBox.stories.tsxThe focused ListBox suites passed (
43/43and105 passed, 1 skipped); the focused SSR suites passed (2 suites, 3 tests); and the full SSR suite passed (60 suites, 74 tests). The full Jest run passed7,998tests across373suites; four unrelated suites remain environment/baseline failures: empty locale fixtures in NumberField/NumberParser, Windows temp-directory permission/timeout in a codemod CLI test, and Windows path-separator expectations in LocalesResolver.🧢 Your Project:
dvd233
AI assistance was used to inspect the issue, prepare the implementation, and run verification. I reviewed the final diff and verified the behavior and test results.