fix(select): close select by pressing selector button #3374
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 727a229 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe patch introduces a fix for the Changes
Sequence Diagram(s)Not applicable as the changes are relatively straightforward, focusing on minor logic updates and test modifications. Assessment against linked issues
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
packages/components/select/__tests__/select.test.tsx (1)
Line range hint
567-589: Add Assertions to Validate Initial State of Select ComponentEnsure that the initial state of the select component is validated before interactions.
+ // assert that the select is initially closed + expect(select).toHaveAttribute("aria-expanded", "false");
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- .changeset/curvy-students-judge.md (1 hunks)
- packages/components/select/tests/select.test.tsx (3 hunks)
- packages/components/select/src/use-select.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- .changeset/curvy-students-judge.md
Additional comments not posted (3)
packages/components/select/src/use-select.ts (2)
547-547: Verify the Dependency Array forgetSelectorIconPropsEnsure that the dependency array in
getSelectorIconPropsincludes all necessary dependencies to avoid potential issues with stale closures.Verification successful
Dependency Array for
getSelectorIconPropsis CorrectThe dependency array for the
getSelectorIconPropsfunction correctly includes all necessary dependencies to avoid potential issues with stale closures. No issues were found.
slotsclassNames?.selectorIconstate.isOpenScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the dependency array for `getSelectorIconProps`. # Test: Search for `getSelectorIconProps` and verify its dependency array. Expect: Correct dependencies. rg --type python $'getSelectorIconProps'Length of output: 70
Script:
#!/bin/bash # Description: Verify the dependency array for `getSelectorIconProps` in the TypeScript file. # Search for `getSelectorIconProps` in the specified file and extract the relevant code to inspect the dependency array. rg 'getSelectorIconProps' packages/components/select/src/use-select.ts -A 10 -B 10Length of output: 947
527-527: Ensure Correctness ofariaShouldCloseOnInteractOutsideUsageThe logic update to use
domRefinariaShouldCloseOnInteractOutsideis essential for the fix. Verify thatdomRefis correctly initialized and passed toariaShouldCloseOnInteractOutsideto avoid potential issues.Verification successful
Initialization and Usage of
domRefVerifiedThe
domRefis correctly initialized and used in theuse-select.tsfile before being passed toariaShouldCloseOnInteractOutside.
packages/components/select/src/use-select.ts: const domRef = useDOMRef(ref);packages/components/select/src/use-select.ts: : (element: Element) => ariaShouldCloseOnInteractOutside(element, domRef, state),Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization and usage of `domRef` in the codebase. # Test: Search for `domRef` initialization and usage in the codebase. Expect: Correct initialization and usage. rg --type python $'domRef'Length of output: 56
Script:
#!/bin/bash # Description: Verify the initialization and usage of `domRef` in the codebase. # Locate the initialization of `domRef` in TypeScript files. rg 'domRef' --type ts # Locate the usage of `ariaShouldCloseOnInteractOutside` to confirm the context of `domRef` usage. rg 'ariaShouldCloseOnInteractOutside' --type tsLength of output: 24902
packages/components/select/__tests__/select.test.tsx (1)
617-650: Test Case for Closing Listbox by Clicking Selector Button AgainThe test case correctly verifies the functionality to close the listbox by clicking the selector button again. Ensure that the test covers all edge cases.
Closes #3276
📝 Description
domReftoariaShouldCloseOnInteractOutsideinstead so that clicking selector button won't trigger the incorrect logic.⛳️ Current behavior
pressing select button won't close the select (if open)
🚀 New behavior
pr3374-demo.webm
💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
Summary by CodeRabbit
Bug Fixes
Tests