fix: Support hidden="until-found" in DisclosureGroup #7199
Merged
Conversation
This file contains 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
2 tasks
|
Controlled doesn't seem to quite work right? https://reactspectrum.blob.core.windows.net/reactspectrum/f0457c575ce0bc023f31532191d0d5e809ccc052/storybook-s2/index.html?path=/story/accordion--controlled-open |
reidbarber
changed the title
reidbarber
changed the title
yihuiliao
reviewed
Oct 18, 2024
## API Changes
@react-aria/disclosure/@react-aria/disclosure:useDisclosure useDisclosure {
props: AriaDisclosureProps
state: DisclosureState
- ref?: RefObject<Element | null>
+ ref: RefObject<Element | null>
returnVal: undefined
} |
snowystinger
approved these changes
Oct 21, 2024
There was a problem hiding this comment.
Tested in Chrome, appears to be working as expected.
- Uncontrolled, opens when text is found
- Uncontrolled allows multiple expanded, all of them open sequentially as expected and stay open
- Disabled disclosures do not open even if text is in them
- Controlled, opens when text is found, key is updated. Can open other panel and redo my actions
- Controlled works with allows multiple expanded
- Controlled always open does not appear to ever open the accordion with the text we're looking for
Any other cases I should be trying?
I did notice that some of the Accordion s2 stories have disclosures without id's, not part of this PR, but I thought those were required?
| @@ -172,8 +172,9 @@ describe('useDisclosure', () => { | |||
| }); | |||
|
|
|||
| expect(result.current.state.isExpanded).toBe(false); | |||
| expect(ref.current.getAttribute('hidden')).toBeNull(); | |||
| expect(onExpandedChange).not.toHaveBeenCalled(); | |||
| expect(ref.current.getAttribute('hidden')).toBe('until-found'); | |||
There was a problem hiding this comment.
I listed a lot of test cases in my review, can we add them either here or in followup?
devongovett
approved these changes
Oct 21, 2024
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.
✅ Pull Request Checklist:
📝 Test Instructions:
Test Ctrl+F behavior for all S2 Accordion and Disclosure stories.
🧢 Your Project: