fix(core/menu-category): make sure to not close itself during touch i… - #2682
Conversation
✅ Deploy Preview for ix-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: 8903822 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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 |
|
Warning Review limit reached
Next review available in: 34 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesMenu category event fix
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/components/menu-category/test/menu-category.ct.ts`:
- Around line 132-179: Add an accessibility test to the menu-category component
suite using makeAxeBuilder(), covering the mounted menu-category fixture and
asserting that no axe violations are reported. Keep the existing
hydration/render coverage and regression interaction test unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 60444853-17ed-465b-965c-233b5201e0f3
📒 Files selected for processing (4)
.changeset/tall-badgers-reply.mdpackages/core/src/components.d.tspackages/core/src/components/menu-category/menu-category.tsxpackages/core/src/components/menu-category/test/menu-category.ct.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/components/menu-category/test/menu-category.ct.ts (1)
148-194: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd a companion assertion for the "close other categories" case.
This test confirms that a category ignores its own
closeOtherCategoriesevent. It does not confirm that acloseOtherCategoriesevent still closes a different category's dropdown. Add a second category dropdown open, then dispatch the event with the first category's ID, and assert the second category's dropdown closes. Without this companion case, a change that breaks the "close other categories" feature (for example, if the listener always returns early) would not be caught here.♻️ Proposed test extension
await page.evaluate((id) => { window.dispatchEvent( new CustomEvent('closeOtherCategories', { detail: id, bubbles: true, composed: true, }) ); }, sourceCategoryId); await expect(dropdownOne).toBeVisible(); + + const categoryTwo = page.locator('ix-menu-category').nth(1); + const dropdownTwo = categoryTwo.locator('ix-dropdown'); + await categoryTwo.hover(); + await expect(dropdownTwo).toBeVisible(); + + await page.evaluate((id) => { + window.dispatchEvent( + new CustomEvent('closeOtherCategories', { + detail: id, + bubbles: true, + composed: true, + }) + ); + }, sourceCategoryId); + + await expect(dropdownTwo).not.toBeVisible(); } );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/components/menu-category/test/menu-category.ct.ts` around lines 148 - 194, Add a companion assertion to the existing regression test by opening Category 2’s dropdown alongside Category 1, dispatching closeOtherCategories with Category 1’s ID, and asserting Category 1 remains visible while Category 2’s ix-dropdown closes. Reuse the existing category hover and event-dispatch flow, locating the second category through the visible menu-category elements.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/core/src/components/menu-category/test/menu-category.ct.ts`:
- Around line 148-194: Add a companion assertion to the existing regression test
by opening Category 2’s dropdown alongside Category 1, dispatching
closeOtherCategories with Category 1’s ID, and asserting Category 1 remains
visible while Category 2’s ix-dropdown closes. Reuse the existing category hover
and event-dispatch flow, locating the second category through the visible
menu-category elements.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a616c29a-1ed6-4ecd-a1ff-7aa0181c8cf5
📒 Files selected for processing (2)
packages/core/src/components.d.tspackages/core/src/components/menu-category/test/menu-category.ct.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/tall-badgers-reply.md:
- Line 5: Rewrite the changeset summary to describe the user-visible behavior:
ix-menu-category remains open during touch interaction. Remove the
implementation detail about checking the component ID.
In `@packages/core/src/components/menu-category/test/menu-category.ct.ts`:
- Around line 148-195: Extend the regression test around categoryOne and the
closeOtherCategories dispatch to open the second menu category first, capture
its dropdown, and assert it is visible before dispatching sourceCategoryId.
After the event, assert the second category’s dropdown is hidden while
dropdownOne remains visible, covering both receiver-side closing and
source-category preservation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 16cd1cd3-2b87-4de3-9cc6-4e8b6fc1d125
📒 Files selected for processing (4)
.changeset/tall-badgers-reply.mdpackages/core/src/components.d.tspackages/core/src/components/menu-category/menu-category.tsxpackages/core/src/components/menu-category/test/menu-category.ct.ts
|
2 participants
…nteraction
💡 What is the current behavior?
GitHub Issue Number: #
🆕 What is the new behavior?
🏁 Checklist
A pull request can only be merged if all of these conditions are met (where applicable):
pnpm test)pnpm lint)pnpm build, changes pushed)👨💻 Help & support
Summary by CodeRabbit
Bug Fixes
Improvements