Uh oh!
There was an error while loading. Please reload this page.
test(ui): add tests for mobile menu default closed state - #2195
Conversation
…vior Covers issue requirements: - Menu is closed by default (dialog not in DOM) - Opens when prop is set to true - Closes when prop changes back to false - Emits update:open on backdrop click - Emits update:open on close button click Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughA new Vitest test suite for the Nuxt Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 24804db7-68df-4724-9496-6879bbe8aa62
📒 Files selected for processing (1)
test/nuxt/components/Header/MobileMenu.spec.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The Nuxt auto-generated name for Header/MobileMenu.client.vue is HeaderMobileMenu, not MobileMenuClient. Also removes unused `ref` import. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/nuxt/components/Header/MobileMenu.spec.ts (1)
88-89: Consider a more type-safe pattern for array access.Lines 89 and 105 use the non-null assertion operator
!to accesswrapper.emitted('update:open')![0]. Whilst the preceding.toBeTruthy()check ensures the array exists, it doesn't explicitly verify that index[0]exists.♻️ Suggested refactor for stricter type safety
For lines 88-89:
- expect(wrapper.emitted('update:open')).toBeTruthy()- expect(wrapper.emitted('update:open')![0]).toEqual([false])+ const emitted = wrapper.emitted('update:open')+ expect(emitted).toBeTruthy()+ expect(emitted?.[0]).toEqual([false])For lines 104-105:
- expect(wrapper.emitted('update:open')).toBeTruthy()- expect(wrapper.emitted('update:open')![0]).toEqual([false])+ const emitted = wrapper.emitted('update:open')+ expect(emitted).toBeTruthy()+ expect(emitted?.[0]).toEqual([false])As per coding guidelines, strictly type-safe code should check when accessing an array value by index.
Also applies to: 104-105
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1fe96943-99c4-4330-9b64-7f0e9f132fb1
📒 Files selected for processing (1)
test/nuxt/components/Header/MobileMenu.spec.ts
Uh oh!
There was an error while loading. Please reload this page.
Switch from dynamic `await import('#components')` to static top-level
import to match the pattern used by HeaderConnectorModal.spec.ts.
Add required `type: 'link'` to NavigationLink test data.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
🔗 Linked issue
Closes#1505
🧭 Context
The mobile menu had no tests verifying its default state or basic open/close behavior.
📚 Description
Adds a test suite for
MobileMenu.client.vuecovering:openis falserole="dialog"andaria-modal="true"appears whenopenis trueopengoes from true to falseupdate:openwithfalseupdate:openwithfalseMocks
useConnector,useAtproto, anduseFocusTrapto isolate the menu logic. Follows existing patterns fromHeaderConnectorModal.spec.tsandTooltip.spec.ts(Teleport + cleanup).What it doesn't change