feat: Improve Date panel keyboard navigation and semantics#986
Conversation
|
@Pareder is attempting to deploy a commit to the afc163's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough为 Picker 组件补充弹出层、输入框、预设项、日期表格、单元格和面板导航的无障碍语义；预设项改为 button，日期面板统一传递标签与单元格属性，并补充相关测试。 Changes无障碍增强与预设按钮重构
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Input
participant Popup
participant PanelBody
User->>Input: 打开日期选择器
Input->>Popup: 传递 open 状态
Popup->>PanelBody: 渲染 dialog 内的日期 grid
PanelBody-->>User: 返回表格、单元格和当前日期 ARIA 属性
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces accessibility improvements across the picker components by transitioning presets from list items to buttons, adding appropriate ARIA roles and attributes (such as role="dialog", aria-expanded, and role="gridcell"), and removing unnecessary tabIndex attributes. The review feedback highlights several key accessibility improvements: removing aria-hidden="true" from the table header to prevent screen reader regressions, adding onFocus and onBlur handlers to preset buttons for keyboard users, applying role="combobox" to inputs using aria-expanded, and ensuring the week picker row correctly sets aria-selected.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| {headerCells && ( | ||
| <thead> | ||
| <thead aria-hidden="true"> | ||
| <tr>{headerCells}</tr> | ||
| </thead> | ||
| )} |
There was a problem hiding this comment.
Applying aria-hidden="true" to the <thead> element removes the column headers (e.g., "Mo", "Tu", etc.) from the accessibility tree. This prevents screen readers from announcing the column headers when users navigate through the grid cells, which is a major accessibility regression. Please remove aria-hidden="true" to keep the column headers accessible.
| {headerCells && ( | |
| <thead> | |
| <thead aria-hidden="true"> | |
| <tr>{headerCells}</tr> | |
| </thead> | |
| )} | |
| {headerCells && ( | |
| <thead> | |
| <tr>{headerCells}</tr> | |
| </thead> | |
| )} |
| <button | ||
| key={index} | ||
| type="button" | ||
| onClick={() => { | ||
| onClick(executeValue(value)); | ||
| }} | ||
| onMouseEnter={() => { | ||
| onHover(executeValue(value)); | ||
| }} | ||
| onMouseLeave={() => { | ||
| onHover(null); | ||
| }} |
There was a problem hiding this comment.
Since this PR focuses on improving keyboard navigation and accessibility, we should ensure that preset buttons also trigger the hover preview state when they receive focus via keyboard tab navigation. Adding onFocus and onBlur handlers will match the mouse hover behavior (onMouseEnter/onMouseLeave) for keyboard users.
<button
key={index}
type="button"
onClick={() => {
onClick(executeValue(value));
}}
onMouseEnter={() => {
onHover(executeValue(value));
}}
onMouseLeave={() => {
onHover(null);
}}
onFocus={() => {
onHover(executeValue(value));
}}
onBlur={() => {
onHover(null);
}}
>
| <Component | ||
| ref={inputRef} | ||
| aria-invalid={invalid} | ||
| aria-haspopup="dialog" | ||
| aria-expanded={open} | ||
| autoComplete="off" |
There was a problem hiding this comment.
According to ARIA specifications, the aria-expanded attribute is not supported on a standard textbox input element unless it has role="combobox". Adding role="combobox" ensures that screen readers correctly interpret and announce the expanded/collapsed state of the date picker popup.
| <Component | |
| ref={inputRef} | |
| aria-invalid={invalid} | |
| aria-haspopup="dialog" | |
| aria-expanded={open} | |
| autoComplete="off" | |
| <Component | |
| ref={inputRef} | |
| role="combobox" | |
| aria-invalid={invalid} | |
| aria-haspopup="dialog" | |
| aria-expanded={open} | |
| autoComplete="off" |
| rows.push( | ||
| <tr key={row} className={rowClassName?.(rowStartDate!)}> | ||
| <tr key={row} role="row" className={rowClassName?.(rowStartDate!)}> | ||
| {rowNode} | ||
| </tr>, | ||
| ); |
There was a problem hiding this comment.
For the week picker (type === 'week'), the entire row represents the selected week rather than individual cells. To make this accessible to screen readers, we should apply aria-selected directly to the <tr> element when the row's week matches the selected value.
const isRowSelected = type === 'week' && matchValues(rowStartDate!);
rows.push(
<tr
key={row}
role="row"
aria-selected={isRowSelected}
className={rowClassName?.(rowStartDate!)}
>
{rowNode}
</tr>,
);
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #986 +/- ##
==========================================
+ Coverage 98.81% 98.93% +0.12%
==========================================
Files 66 66
Lines 2698 2732 +34
Branches 749 736 -13
==========================================
+ Hits 2666 2703 +37
+ Misses 29 27 -2
+ Partials 3 2 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (3)
src/PickerPanel/MonthPanel/index.tsx (1)
60-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
aria-current="date"对非日期单元格语义不够准确（同样适用于 Quarter/Year/Decade 面板）。
aria-current的date取值用于标识“当前日期”。月份（以及季度、年、十年）单元格代表的不是具体日期，按 ARIA 规范在没有匹配的具体取值时更合适使用通用值true。屏幕阅读器对未识别取值通常按true处理，因此影响轻微，可按需调整。♻️ 可选调整
- return { 'aria-current': 'date' }; + return { 'aria-current': true };🤖 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 `@src/PickerPanel/MonthPanel/index.tsx` around lines 60 - 65, The cell accessibility state in MonthPanel is using aria-current="date" for a non-day unit, which should be generalized. Update getCellAttributes in MonthPanel, and apply the same pattern in the Quarter/Year/Decade panel helpers, so the currently selected month-like cell uses aria-current="true" instead of "date". Keep the logic tied to the existing generateConfig/isSameMonth-style checks and the getCellAttributes method names so the change is consistent across panels.src/PickerInput/Popup/index.tsx (1)
222-222: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value建议补充 dialog 的可访问名称。
role="dialog"已添加，但缺少aria-label/aria-labelledby，部分屏幕阅读器进入对话框时无法播报名称；如适用也可考虑aria-modal。属于无障碍完整性的可选增强，可按需采纳。🤖 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 `@src/PickerInput/Popup/index.tsx` at line 222, The Popup dialog currently has role="dialog" but no accessible name, so update the dialog element in the Popup component to include an appropriate aria-label or aria-labelledby tied to an existing visible title, and add aria-modal if the popup is truly modal. Use the dialog markup in Popup/index.tsx and the surrounding title/content elements to choose the most suitable label source.src/PickerInput/Selector/Input.tsx (1)
410-411: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
aria-expanded建议显式取布尔值。
aria-haspopup="dialog"恒定存在，而open可能为undefined（useInputProps中会在清理阶段删除 undefined 字段），此时aria-expanded不会被渲染，辅助技术无法获知折叠/展开状态。建议强制为布尔值，保证属性始终存在。♿ 建议修改
aria-haspopup="dialog" - aria-expanded={open} + aria-expanded={!!open}🤖 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 `@src/PickerInput/Selector/Input.tsx` around lines 410 - 411, `Input` 里的 `aria-expanded` 直接绑定了 `open`，当 `open` 为 `undefined` 时属性会被省略，导致辅助技术无法稳定获取展开状态。请在 `Selector/Input.tsx` 的输入渲染逻辑中，针对 `aria-expanded` 强制转换为布尔值，确保该属性始终存在，并保留与 `aria-haspopup="dialog"` 和 `useInputProps` 清理行为一致的可访问性语义。
🤖 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 `@assets/index.less`:
- Around line 175-180: The preset button container in the &-presets styles only
sets layout, so the new native button element still shows browser default
border, padding, background, and font styles. Update the button styling in the
presets section of assets/index.less to reset the button appearance (for example
in the same ruleset that styles the preset items) so the visual matches the old
li-based UI consistently across browsers.
---
Nitpick comments:
In `@src/PickerInput/Popup/index.tsx`:
- Line 222: The Popup dialog currently has role="dialog" but no accessible name,
so update the dialog element in the Popup component to include an appropriate
aria-label or aria-labelledby tied to an existing visible title, and add
aria-modal if the popup is truly modal. Use the dialog markup in Popup/index.tsx
and the surrounding title/content elements to choose the most suitable label
source.
In `@src/PickerInput/Selector/Input.tsx`:
- Around line 410-411: `Input` 里的 `aria-expanded` 直接绑定了 `open`，当 `open` 为
`undefined` 时属性会被省略，导致辅助技术无法稳定获取展开状态。请在 `Selector/Input.tsx` 的输入渲染逻辑中，针对
`aria-expanded` 强制转换为布尔值，确保该属性始终存在，并保留与 `aria-haspopup="dialog"` 和
`useInputProps` 清理行为一致的可访问性语义。
In `@src/PickerPanel/MonthPanel/index.tsx`:
- Around line 60-65: The cell accessibility state in MonthPanel is using
aria-current="date" for a non-day unit, which should be generalized. Update
getCellAttributes in MonthPanel, and apply the same pattern in the
Quarter/Year/Decade panel helpers, so the currently selected month-like cell
uses aria-current="true" instead of "date". Keep the logic tied to the existing
generateConfig/isSameMonth-style checks and the getCellAttributes method names
so the change is consistent across panels.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f700c8f0-e182-4801-91be-9cf73b993afc
⛔ Files ignored due to path filters (3)
tests/__snapshots__/panel.spec.tsx.snapis excluded by!**/*.snaptests/__snapshots__/picker.spec.tsx.snapis excluded by!**/*.snaptests/__snapshots__/range.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
assets/index.lesssrc/PickerInput/Popup/PresetPanel.tsxsrc/PickerInput/Popup/index.tsxsrc/PickerInput/Selector/Input.tsxsrc/PickerInput/Selector/hooks/useInputProps.tssrc/PickerPanel/DatePanel/index.tsxsrc/PickerPanel/DecadePanel/index.tsxsrc/PickerPanel/MonthPanel/index.tsxsrc/PickerPanel/PanelBody.tsxsrc/PickerPanel/PanelHeader.tsxsrc/PickerPanel/QuarterPanel/index.tsxsrc/PickerPanel/YearPanel/index.tsxtests/new-range.spec.tsxtests/picker.spec.tsxtests/range.spec.tsx
| &-presets { | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: 4px; | ||
| background: #ccccff; | ||
|
|
||
| ul { | ||
| margin: 0; | ||
| padding: 0; | ||
| list-style: none; | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
补上预设按钮的样式重置。
这里把 li 换成原生 button 后，只改了容器布局，没有重置按钮自身样式。浏览器默认的边框、内边距、背景和字体会直接暴露出来，预设面板会出现明显的视觉回归，而且不同浏览器表现也不一致。
建议修改
&-presets {
display: flex;
flex-direction: column;
gap: 4px;
background: `#ccccff`;
+
+ > button {
+ padding: 0;
+ font: inherit;
+ color: inherit;
+ text-align: start;
+ background: transparent;
+ border: 0;
+ cursor: pointer;
+ }
}📝 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.
| &-presets { | |
| display: flex; | |
| flex-direction: column; | |
| gap: 4px; | |
| background: #ccccff; | |
| ul { | |
| margin: 0; | |
| padding: 0; | |
| list-style: none; | |
| } | |
| } | |
| &-presets { | |
| display: flex; | |
| flex-direction: column; | |
| gap: 4px; | |
| background: `#ccccff`; | |
| > button { | |
| padding: 0; | |
| font: inherit; | |
| color: inherit; | |
| text-align: start; | |
| background: transparent; | |
| border: 0; | |
| cursor: pointer; | |
| } | |
| } |
🤖 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 `@assets/index.less` around lines 175 - 180, The preset button container in the
&-presets styles only sets layout, so the new native button element still shows
browser default border, padding, background, and font styles. Update the button
styling in the presets section of assets/index.less to reset the button
appearance (for example in the same ruleset that styles the preset items) so the
visual matches the old li-based UI consistently across browsers.
1 similar comment
…-panel-keyboard-navigation
1 participant
Summary
This PR is a small chunk of a big one #972 focusing on providing correct semantics and making elements keyboard accessible for Date panels.
PresetPanelrenders native button elements (together withtype="button")Popuphasrole="dialog"DatePanel/DecadePanel/MonthPanel/QuarterPanel/YearPanelimprovements:tabIndex={-1}from header buttonsSummary by CodeRabbit