-
Notifications
You must be signed in to change notification settings - Fork 324
fix(tabs): Fix the issue where multiple clicks on mobile-first tabs do not take effect #3669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a public "click" event emission from dropdown-item carrying (itemData, MouseEvent); updates renderless tabs handler to (navItem, event), emit that click, then call clickMore(navItem.name); updates Tab Bar to bind per-item click handlers and pass full option objects as itemData. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant TB as Tab Bar (mobile-first)
participant DI as DropdownItem
participant TR as Tabs Renderless
U->>DI: click(item)
DI-->>TB: emit "click"(itemData, event)
TB->>TR: handleClickDropdownItem(itemData, event)
TR-->>TB: $emit "click"(navItem, event)
TR->>TR: clickMore(navItem.name)
note right of TR: navigation triggered by name
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai
coderabbitai
bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/design/aurora/src/dropdown-item/index.ts(2 hunks)packages/renderless/src/tabs-mf/index.ts(1 hunks)packages/vue/src/dropdown-item/src/mobile-first.vue(1 hunks)packages/vue/src/tabs/src/mobile-first/tab-bar.vue(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (3)
packages/vue/src/dropdown-item/src/mobile-first.vue (1)
43-43: Add 'click' to emits — OK; ensure types/docs updatedGood addition to avoid Vue warns. Please also update any TS declaration (if applicable) and component docs to include the new "click(itemData, event)" payload.
packages/vue/src/tabs/src/mobile-first/tab-bar.vue (2)
84-101: Delegating per-item click to DropdownItem — LGTMThis matches the new
DropdownItem"click" event and should resolve the multi-click issue.
84-101: No lingeringitem-clicklisteners intab-bar.vue; removal is safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not emit "click" for disabled items
Currently "click" fires even when props.disabled is true, which can trigger navigation from disabled entries. Guard both branches.
Apply this diff:
@@ - emit('click', dataStore.itemData, event) + if (!props.disabled) { + emit('click', dataStore.itemData, event) + } @@ - emit('click', props.itemData, event) + if (!props.disabled) { + emit('click', props.itemData, event) + }
Also applies to: 51-51
🤖 Prompt for AI Agents
In packages/design/aurora/src/dropdown-item/index.ts around lines 40-41 (and
also line 51), the component currently emits 'click' even when props.disabled is
true; update both places so that if props.disabled is truthy the handler returns
early or skips emitting—i.e., add a guard like if (props.disabled) return or
wrap emit calls in if (!props.disabled) { emit('click', dataStore.itemData,
event) } to prevent navigation/actions from disabled items.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Align behavior with tab-nav clicks; validate input and skip disabled
Keep event order consistent with handleNavItemClick (emit after activation), and avoid acting on invalid/disabled items.
Apply this diff:
-export const handleClickDropdownItem = (tabs) => (navItem, event) => { - tabs.$emit('click', navItem, event) - tabs.clickMore(navItem.name) -} +export const handleClickDropdownItem = (tabs) => (navItem, event) => { + if (!navItem || !navItem.name || navItem.disabled) return + tabs.clickMore(navItem.name) + tabs.$emit('click', navItem, event) +}
📝 Committable suggestion
!!️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
🤖 Prompt for AI Agents
In packages/renderless/src/tabs-mf/index.ts around lines 291 to 294, the
dropdown click handler currently emits before activation and doesn't validate
input or skip disabled items; update handleClickDropdownItem to first check that
navItem exists and is not disabled (and has a valid name), then call
tabs.clickMore(navItem.name) to activate the tab, and only after successful
activation call tabs.$emit('click', navItem, event); return early if validation
fails to avoid acting on invalid/disabled items.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass disabled state and use stable keys
- Without passing
disabled, disabled items won’t render/behave correctly. - Randomized keys cause unnecessary remounts. Use a stable key (e.g., name).
Apply this diff:
- h( - DropdownItem, - { key: key(opt), on: { click: handleClickDropdownItem }, props: { itemData: opt } }, - [opt.slotTitle ? opt.slotTitle() : opt.title] - ) + h( + DropdownItem, + { + key: opt.name, + on: { click: handleClickDropdownItem }, + props: { itemData: opt, disabled: !!opt.disabled } + }, + [opt.slotTitle ? opt.slotTitle() : opt.title] + )
📝 Committable suggestion
!!️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
🤖 Prompt for AI Agents
packages/vue/src/tabs/src/mobile-first/tab-bar.vue around lines 93 to 97: the
DropdownItem is not receiving the disabled state and is using an unstable key
which causes unnecessary remounts; update the element to pass the disabled flag
from the option (e.g., props: { itemData: opt, disabled: opt.disabled }) and
replace the randomized key with a stable unique identifier from the option
(e.g., key: opt.name or opt.value), ensuring click handlers respect the disabled
state if needed.
Uh oh!
There was an error while loading. Please reload this page.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Refactor