-
Notifications
You must be signed in to change notification settings - Fork 336
fix(dropdown): add aria information to the dropdown component #3913
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
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 |
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: 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
…wdong/feat-select-docs * 'dev' of https://github.com/opentiny/tiny-vue: (152 commits) feat: add triggering conditions (opentiny#3922) fix:修复初始化tab内容无法加载问题 (opentiny#3920) fix(modal): 修复 modal 组件在 alert, confirm 的弹窗场景下标题图标没有垂直局中的问题 (opentiny#3917) fix(dropdown): add aria information to the dropdown component (opentiny#3913) feat:下拉树支持懒加载 (opentiny#3915) docs: update introduce doc (opentiny#3914) feat:select-wrapper多端示例以及文档 (opentiny#3882) feat:select-wrapper多端 (opentiny#3881) fix(transfer): fix it works when item data receive empty object (opentiny#3912) feat(switch): add width property to support custom switch width (opentiny#3879) fix: optimize the training ground (opentiny#3911) fix(e2e): fix e2e error caused by select refactor (opentiny#3905) feat: automatically trigger updates to the docs document (opentiny#3906) feat: add auto-build-main-module-docs yml (opentiny#3910) fix(split): add saas style (opentiny#3908) fix(grid): fix visible columns change wloud delete insert row (opentiny#3897) chore: add svg icon-layout-vertical (opentiny#3904) fix(saas-icon): add lost icon (opentiny#3903) fix: 修复saas-common 构建报错 (opentiny#3901) fix(grid): add border-top when set border is true (opentiny#3896) ...
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.