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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (16)
🚧 Files skipped from review as they are similar to previous changes (16)
WalkthroughPicker 组件新增弹出层、输入框、预设按钮和日期面板的无障碍属性。日期表格支持网格语义、单元格状态和当前日期标识。面板导航支持自定义标签。相关测试同步更新。 Changes无障碍增强与预设按钮重构
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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" |
There was a problem hiding this comment.
Gemini's suggestion can take a look.
| 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.87% 98.99% +0.11%
==========================================
Files 68 68
Lines 2857 2891 +34
Branches 815 798 -17
==========================================
+ Hits 2825 2862 +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
| ))} | ||
| </ul> | ||
| {presets.map(({ label, value }, index) => ( | ||
| <button |
There was a problem hiding this comment.
I'd like to keep the ul -> li -> button strcture. The presets is a list here. Button may less the semantic meaning.
…-panel-keyboard-navigation
|
@zombieJ Fixed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/PickerInput/Popup/PresetPanel.tsx`:
- Around line 34-45: Update the hover handling around the preset item’s
onMouseEnter, onMouseLeave, onFocus, and onBlur callbacks to track pointer and
focus activity independently. Keep the preview active while either state remains
active, and call onHover(null) only after both states are inactive.
🪄 Autofix
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 Plus
Run ID: 0a8c97dc-1077-4310-addc-7b556ef52ac9
⛔ Files ignored due to path filters (2)
tests/__snapshots__/picker.spec.tsx.snapis excluded by!**/*.snaptests/__snapshots__/range.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
src/PickerInput/Popup/PresetPanel.tsxsrc/PickerInput/Popup/index.tsxsrc/PickerInput/Selector/Input.tsxtests/new-range.spec.tsxtests/picker.spec.tsxtests/range.spec.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/new-range.spec.tsx
- tests/range.spec.tsx
- tests/picker.spec.tsx
- src/PickerInput/Selector/Input.tsx
- src/PickerInput/Popup/index.tsx
31e9fea to
123d8a0
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
2 participants
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
新功能
测试