fix(dropdown): add aria information to the dropdown component #3913
Conversation
WalkthroughThe changes update accessibility attributes across dropdown components. The aria-haspopup attribute value in the renderless trigger logic is changed from 'list' to 'menu', and ARIA roles (menu, menuitem) and aria-labels are added to dropdown template elements to improve semantic accessibility. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![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
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/renderless/src/dropdown/index.ts(1 hunks)packages/vue/src/dropdown-item/src/pc.vue(1 hunks)packages/vue/src/dropdown-menu/src/pc.vue(1 hunks)packages/vue/src/dropdown/src/pc.vue(3 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). (2)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
- GitHub Check: verify-main-build
🔇 Additional comments (3)
packages/renderless/src/dropdown/index.ts (1)
182-182: LGTM! Correct ARIA semantics for menu popup.The change from
'list'to'menu'correctly aligns with the ARIA roles being added elsewhere in this PR (role="menu"on the dropdown menu,role="menuitem"on items). This follows ARIA best practices for menu button patterns.packages/vue/src/dropdown-item/src/pc.vue (1)
33-33: LGTM! Proper ARIA role for menu items.Adding
role="menuitem"correctly identifies this element as a menu item within the menu structure. This aligns with therole="menu"being added to the parent dropdown menu.packages/vue/src/dropdown-menu/src/pc.vue (1)
21-21: LGTM! Correct ARIA role for menu container.Adding
role="menu"properly identifies this as a menu container. This works correctly with therole="menuitem"attributes being added to the dropdown items.
| disabled={disabled} | ||
| reset-time={0}> | ||
| <ButtonIconDown class={visibleClass}></ButtonIconDown> | ||
| <ButtonIconDown class={visibleClass} aria-label="down"></ButtonIconDown> |
There was a problem hiding this comment.
Replace generic aria-label="down" with descriptive, internationalized labels.
The aria-label="down" additions describe the icon's visual direction rather than the element's purpose. For accessibility:
-
Use action-oriented labels: Screen reader users need to understand what the element does, not what it looks like. Consider "Open menu", "Toggle dropdown", or similar.
-
Internationalize the labels: The hardcoded English string conflicts with the i18n TODO comment (line 93). Use a translatable string or prop.
-
Apply to interactive elements only: Decorative icons should be
aria-hidden="true", with the aria-label on the containing button/trigger instead.
🔎 Suggested approach
For split button (line 155):
-<ButtonIconDown class={visibleClass} aria-label="down"></ButtonIconDown>
+<ButtonIconDown class={visibleClass} aria-hidden="true"></ButtonIconDown>And add aria-label to the button itself:
<tiny-button
ref="trigger"
type={type}
size={size}
class={`tiny-dropdown__caret-button ${triggerClass}`}
disabled={disabled}
- reset-time={0}>
+ reset-time={0}
+ aria-label={t('ui.dropdown.toggleMenu')}>Similar patterns apply to the other locations (lines 173, 200, 209). Consider adding an ariaLabel prop to allow customization while providing a sensible default.
Also applies to: 172-174, 199-200, 208-209
🤖 Prompt for AI Agents
packages/vue/src/dropdown/src/pc.vue around line 155 (and similarly lines
172-174, 199-200, 208-209): the ButtonIconDown currently uses aria-label="down"
which describes appearance not action and is hardcoded English; change so the
icon itself is decorative (add aria-hidden="true" and remove aria-label) and
move a descriptive, action-oriented, translatable aria-label onto the
interactive button/trigger (e.g., "Open menu" or "Toggle dropdown") by adding an
ariaLabel prop with a sensible i18n default and using that prop on the button;
apply the same pattern to the other listed locations so all interactive controls
use an i18n-aware ariaLabel prop and icons are aria-hidden.
PR
为dropdown 添加 aria-* 信息
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
✏️ Tip: You can customize this high-level summary in your review settings.