Skip to content

feat: refactor Footer component to be accessible #990

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.

Sign up for GitHub

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

Open
Pareder wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: refactor Footer component to be accessible #990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions assets/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,6 @@
margin: 0;
padding: 0;
overflow: hidden;
list-style: none;

> li {
display: inline-block;
}
}

&-ok {
Expand Down
35 changes: 15 additions & 20 deletions src/PickerInput/Popup/Footer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ export default function Footer(props: FooterProps) {
const {
prefixCls,
locale,
button: Button = 'button',
nowButton: NowButton = 'button',
okButton: OkButton = 'button',
classNames,
styles,
} = React.useContext(PickerContext);
Expand All @@ -70,35 +71,29 @@ export default function Footer(props: FooterProps) {
}
};

const nowPrefixCls = `${prefixCls}-now`;
const nowBtnPrefixCls = `${nowPrefixCls}-btn`;

const presetNode = showNow && (
<li className={nowPrefixCls}>
<a
className={clsx(nowBtnPrefixCls, nowDisabled && `${nowBtnPrefixCls}-disabled`)}
aria-disabled={nowDisabled}
onClick={onInternalNow}
>
{internalMode === 'date' ? locale.today : locale.now}
</a>
</li>
<NowButton
type="button"
className={`${prefixCls}-now`}
disabled={nowDisabled}
onClick={onInternalNow}
>
{internalMode === 'date' ? locale.today : locale.now}
</NowButton>
);

// >>> OK
const okNode = needConfirm && (
<li className={`${prefixCls}-ok`}>
<Button disabled={invalid} onClick={onSubmit}>
{locale.ok}
</Button>
</li>
<OkButton type="button" disabled={invalid} className={`${prefixCls}-ok`} onClick={onSubmit}>
{locale.ok}
</OkButton>
Comment on lines 86 to +89

@coderabbitai coderabbitai Bot Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

不要把 onSubmit 直接传给原生 button 的 onClick。

这里的 onSubmit 在 RangePicker 中是 triggerPartConfirm(date?)。改成原生 button 后，点击 OK 会把 MouseEvent 作为第一个参数传进去，区间选择的确认流会把事件对象当成日期提交，后续状态会被污染。

建议修复 
-  const okNode = needConfirm && (
-    <OkButton type="button" disabled={invalid} className={`${prefixCls}-ok`} onClick={onSubmit}>
+  const okNode = needConfirm && (
+    <OkButton
+      type="button"
+      disabled={invalid}
+      className={`${prefixCls}-ok`}
+      onClick={() => onSubmit()}
+    >
       {locale.ok}
     </OkButton>
   );
📝 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.

Suggested change
const okNode = needConfirm && (
<li className={`${prefixCls}-ok`}>
<Button disabled={invalid} onClick={onSubmit}>
{locale.ok}
</Button>
</li>
<OkButton type="button" disabled={invalid} className={`${prefixCls}-ok`} onClick={onSubmit}>
{locale.ok}
</OkButton>
const okNode = needConfirm && (
<OkButton
type="button"
disabled={invalid}
className={`${prefixCls}-ok`}
onClick={() => onSubmit()}
>
{locale.ok}
</OkButton>
🤖 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/Footer.tsx` around lines 86 - 89, The OkButton in
Footer.tsx should not pass RangePicker’s onSubmit directly to the native button
onClick, because triggerPartConfirm(date?) will receive a MouseEvent instead of
a date. Update the OkButton wiring so the click handler invokes onSubmit without
forwarding the event, keeping the confirmation flow in Popup/Footer and
RangePicker consistent and preventing event objects from being treated as dates.

);

const rangeNode = (presetNode || okNode) && (
<ul className={`${prefixCls}-ranges`}>
<div className={`${prefixCls}-ranges`}>
{presetNode}
{okNode}
</ul>
</div>
);

// ======================== Render ========================
Expand Down
6 changes: 4 additions & 2 deletions src/PickerInput/RangePicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,8 @@ function RangePicker<DateType extends object = any>(
prefixCls,
locale,
generateConfig,
button: components.button,
nowButton: components.nowButton,
okButton: components.okButton,
input: components.input,
classNames: mergedClassNames,
styles: mergedStyles,
Expand All @@ -726,7 +727,8 @@ function RangePicker<DateType extends object = any>(
prefixCls,
locale,
generateConfig,
components.button,
components.nowButton,
components.okButton,
components.input,
mergedClassNames,
mergedStyles,
Expand Down
6 changes: 4 additions & 2 deletions src/PickerInput/SinglePicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,8 @@ function Picker<DateType extends object = any>(
prefixCls,
locale,
generateConfig,
button: components.button,
nowButton: components.nowButton,
okButton: components.okButton,
input: components.input,
classNames: mergedClassNames,
styles: mergedStyles,
Expand All @@ -613,7 +614,8 @@ function Picker<DateType extends object = any>(
prefixCls,
locale,
generateConfig,
components.button,
components.nowButton,
components.okButton,
components.input,
mergedClassNames,
mergedStyles,
Expand Down
3 changes: 2 additions & 1 deletion src/PickerInput/context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ export interface PickerContextProps<DateType = any> {
locale: Locale;
generateConfig: GenerateConfig<DateType>;
/** Customize button component */
button?: Components['button'];
nowButton?: Components['nowButton'];
okButton?: Components['okButton'];
input?: Components['input'];
classNames: FilledClassNames;
styles: FilledStyles;
Expand Down
11 changes: 3 additions & 8 deletions src/interface.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ export interface SharedPanelProps<DateType extends object = any> {

export type Components<DateType extends object = any> = Partial<
Record<InternalMode, React.ComponentType<SharedPanelProps<DateType>>> & {
button?: React.ComponentType<any> | string;
nowButton?: React.ComponentType<any> | string;
okButton?: React.ComponentType<any> | string;
input?: React.ComponentType<any> | string;
}
>;
Expand Down Expand Up @@ -316,13 +317,7 @@ export type SemanticName = 'root' | 'prefix' | 'input' | 'suffix';
export type PreviewValueType = 'hover';

export type PanelSemanticName =
| 'root'
| 'header'
| 'body'
| 'content'
| 'item'
| 'footer'
| 'container';
'root' | 'header' | 'body' | 'content' | 'item' | 'footer' | 'container';

export interface SharedPickerProps<DateType extends object = any>
extends
Expand Down
2 changes: 1 addition & 1 deletion tests/components.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('Picker.Components', () => {
good: [null, null],
}}
components={{
button: Button,
okButton: Button,
}}
picker="time"
open
Expand Down
8 changes: 4 additions & 4 deletions tests/multiple.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('Picker.Multiple', () => {
expect(isOpen()).toBeTruthy();

// Confirm
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));
expect(onChange).toHaveBeenCalledWith(expect.anything(), [
'1990-09-01',
'1990-09-03',
Expand All @@ -72,7 +72,7 @@ describe('Picker.Multiple', () => {
selectCell(3);

// Confirm
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));
expect(onChange).toHaveBeenCalledWith(expect.anything(), ['1990-09-01', '1990-09-05']);
});

Expand Down Expand Up @@ -112,7 +112,7 @@ describe('Picker.Multiple', () => {
expect(container.querySelectorAll('.rc-picker-selection-item')).toHaveLength(1);

// Confirm
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));
expect(onChange).toHaveBeenCalledWith(expect.anything(), ['2000-01-28']);
});

Expand Down Expand Up @@ -247,7 +247,7 @@ describe('Picker.Multiple', () => {
);

// Confirm
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));
expect(onChange).toHaveBeenCalledWith(expect.anything(), ['1998-10-23']);
});
});
36 changes: 18 additions & 18 deletions tests/new-range.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ describe('NewPicker.Range', () => {
expect(onCalendarChange).toHaveBeenCalled();

// Check button disabled
expect(document.querySelector('.rc-picker-ok button')).toBeDisabled();
expect(document.querySelector('.rc-picker-ok')).toBeDisabled();
});

it('not trigger on disabled all hours', () => {
Expand All @@ -413,7 +413,7 @@ describe('NewPicker.Range', () => {
expect(onCalendarChange).toHaveBeenCalled();

// Check button disabled
expect(document.querySelector('.rc-picker-ok button')).toBeDisabled();
expect(document.querySelector('.rc-picker-ok')).toBeDisabled();
});

it('not trigger onChange if presets is invalidate', () => {
Expand Down Expand Up @@ -454,10 +454,10 @@ describe('NewPicker.Range', () => {
);

openPicker(container);
fireEvent.click(document.querySelector('.rc-picker-now-btn'));
fireEvent.click(document.querySelector('.rc-picker-now-btn'));
fireEvent.click(document.querySelector('.rc-picker-now'));
fireEvent.click(document.querySelector('.rc-picker-now'));

expect(document.querySelector('.rc-picker-now-btn-disabled')).toBeTruthy();
expect(document.querySelector('.rc-picker-now')).toBeDisabled();

expect(onChange).not.toHaveBeenCalled();
expect(onCalendarChange).not.toHaveBeenCalled();
Expand Down Expand Up @@ -546,15 +546,15 @@ describe('NewPicker.Range', () => {
value: '01:00:00',
},
});
expect(document.querySelector('.rc-picker-ok button')).not.toBeDisabled();
expect(document.querySelector('.rc-picker-ok')).not.toBeDisabled();

// Disabled
fireEvent.change(input, {
target: {
value: '00:00:00',
},
});
expect(document.querySelector('.rc-picker-ok button')).toBeDisabled();
expect(document.querySelector('.rc-picker-ok')).toBeDisabled();
});

it('disabledDate provides info.type', () => {
Expand Down Expand Up @@ -602,7 +602,7 @@ describe('NewPicker.Range', () => {
onCalendarChange.mockReset();

// End field
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));
selectCell(7);
expect(onCalendarChange).toHaveBeenCalledWith(
expect.anything(),
Expand Down Expand Up @@ -655,14 +655,14 @@ describe('NewPicker.Range', () => {
// Changed by click OK
openPicker(container);
selectCell(10);
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));

expect(container.querySelectorAll('input')[0]).toHaveValue('1990-09-10');

// End time
selectCell(15, 1);
expect(onChange).not.toHaveBeenCalled();
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));

expect(onChange).toHaveBeenCalledWith(expect.anything(), ['1990-09-10', '1990-10-15']);
});
Expand Down Expand Up @@ -721,10 +721,10 @@ describe('NewPicker.Range', () => {
const { container } = render(<DayRangePicker onChange={onChange} showTime />);
openPicker(container);
selectCell(15);
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));

selectCell(16);
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));

expect(onChange).toHaveBeenCalledWith(expect.anything(), [
'1990-09-15 00:00:00',
Expand All @@ -738,11 +738,11 @@ describe('NewPicker.Range', () => {
expect(container.querySelectorAll('input')[0]).toHaveFocus();
expect(container.querySelectorAll('input')[1]).not.toHaveFocus();

fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));
openPicker(container, 1);
expect(container.querySelectorAll('input')[1]).toHaveFocus();
selectCell(2);
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));
expect(onChange).toHaveBeenCalledWith(expect.anything(), [
'1990-09-01 00:00:00',
'1990-09-02 00:00:00',
Expand Down Expand Up @@ -1381,8 +1381,8 @@ describe('NewPicker.Range', () => {
const { container } = render(<DayRangePicker showNow onChange={onChange} />);

openPicker(container);
fireEvent.click(document.querySelector('.rc-picker-now-btn'));
fireEvent.click(document.querySelector('.rc-picker-now-btn'));
fireEvent.click(document.querySelector('.rc-picker-now'));
fireEvent.click(document.querySelector('.rc-picker-now'));

expect(onChange).toHaveBeenCalledWith(expect.anything(), ['1990-09-03', '1990-09-03']);
});
Expand Down Expand Up @@ -1412,12 +1412,12 @@ describe('NewPicker.Range', () => {

// Start
fireEvent.click(document.querySelectorAll('li.rc-picker-time-panel-cell')[2]);
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));
await waitFakeTimer();

// End
fireEvent.click(document.querySelectorAll('li.rc-picker-time-panel-cell')[2]);
fireEvent.click(document.querySelector('.rc-picker-ok button'));
fireEvent.click(document.querySelector('.rc-picker-ok'));
await waitFakeTimer();

expect(onChange).toHaveBeenCalledWith(expect.anything(), [
Expand Down
Loading
Loading