Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 3 additions & 11 deletions assets/index.less
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,10 @@

// Preset
&-presets {
display: flex;
flex-direction: column;
gap: 4px;
background: #ccccff;

ul {
margin: 0;
padding: 0;
list-style: none;
}
}

&-footer,
Expand All @@ -191,11 +188,6 @@
margin: 0;
padding: 0;
overflow: hidden;
list-style: none;

> li {
display: inline-block;
}
}

&-ok {
Expand Down
30 changes: 10 additions & 20 deletions src/PickerInput/Popup/Footer.tsx
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,24 @@ 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 className={`${prefixCls}-now`} disabled={nowDisabled} onClick={onInternalNow}>
{internalMode === 'date' ? locale.today : locale.now}
</NowButton>
Comment on lines +75 to +77
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.

medium

Add type="button" to the NowButton to prevent accidental form submissions when the picker is placed inside an HTML <form>.

Suggested change
<NowButton className={`${prefixCls}-now`} disabled={nowDisabled} onClick={onInternalNow}>
{internalMode === 'date' ? locale.today : locale.now}
</NowButton>
<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 disabled={invalid} className={`${prefixCls}-ok`} onClick={onSubmit}>
{locale.ok}
</OkButton>
Comment on lines +82 to +84
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.

medium

Add type="button" to the OkButton to prevent accidental form submissions when the picker is placed inside an HTML <form>.

Suggested change
<OkButton disabled={invalid} className={`${prefixCls}-ok`} onClick={onSubmit}>
{locale.ok}
</OkButton>
<OkButton type="button" disabled={invalid} className="${prefixCls}-ok" onClick={onSubmit}>
{locale.ok}
</OkButton>

Comment on lines +75 to +84
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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

给默认按钮补上 type="button"

NowButton / OkButton 默认回退为原生 <button>,这里没传 type,放进 <form> 里点击会触发表单提交,直接打断选择流程。

建议修改
-    <NowButton className={`${prefixCls}-now`} disabled={nowDisabled} onClick={onInternalNow}>
+    <NowButton
+      type="button"
+      className={`${prefixCls}-now`}
+      disabled={nowDisabled}
+      onClick={onInternalNow}
+    >
       {internalMode === 'date' ? locale.today : locale.now}
     </NowButton>
@@
-    <OkButton disabled={invalid} className={`${prefixCls}-ok`} onClick={onSubmit}>
+    <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
<NowButton 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 disabled={invalid} className={`${prefixCls}-ok`} onClick={onSubmit}>
{locale.ok}
</OkButton>
<NowButton
type="button"
className={`${prefixCls}-now`}
disabled={nowDisabled}
onClick={onInternalNow}
>
{internalMode === 'date' ? locale.today : locale.now}
</NowButton>
);
// >>> OK
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 75 - 84, The NowButton and
OkButton are rendered as native buttons without a type, so when placed inside a
form clicking them can trigger form submission; update the JSX for NowButton
(used with onInternalNow, className `${prefixCls}-now`) and OkButton (rendered
when needConfirm is true, disabled based on invalid, with onClick onSubmit and
className `${prefixCls}-ok`) to include type="button" to prevent accidental form
submits.

);

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

// ======================== Render ========================
Expand Down
34 changes: 16 additions & 18 deletions src/PickerInput/Popup/PresetPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,22 @@ export default function PresetPanel<DateType extends object = any>(

return (
<div className={`${prefixCls}-presets`}>
<ul>
{presets.map(({ label, value }, index) => (
<li
key={index}
onClick={() => {
onClick(executeValue(value));
}}
onMouseEnter={() => {
onHover(executeValue(value));
}}
onMouseLeave={() => {
onHover(null);
}}
>
{label}
</li>
))}
</ul>
{presets.map(({ label, value }, index) => (
<button
key={index}
onClick={() => {
onClick(executeValue(value));
Comment on lines +27 to +30
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.

medium

Add type="button" to the preset <button> to prevent accidental form submissions when the picker is placed inside an HTML <form>.

Suggested change
<button
key={index}
onClick={() => {
onClick(executeValue(value));
<button
key={index}
type="button"
onClick={() => {

}}
onMouseEnter={() => {
onHover(executeValue(value));
}}
onMouseLeave={() => {
onHover(null);
}}
>
{label}
</button>
Comment on lines +27 to +40
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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preset 按钮也需要显式声明 type="button"

现在这些 preset 项已经变成原生 <button>,未设置 type 时会默认提交外层表单,和“仅回填日期”的预期不符。

建议修改
         <button
           key={index}
+          type="button"
           onClick={() => {
             onClick(executeValue(value));
           }}
📝 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
<button
key={index}
onClick={() => {
onClick(executeValue(value));
}}
onMouseEnter={() => {
onHover(executeValue(value));
}}
onMouseLeave={() => {
onHover(null);
}}
>
{label}
</button>
<button
key={index}
type="button"
onClick={() => {
onClick(executeValue(value));
}}
onMouseEnter={() => {
onHover(executeValue(value));
}}
onMouseLeave={() => {
onHover(null);
}}
>
{label}
</button>
🤖 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/PresetPanel.tsx` around lines 27 - 40, The preset
buttons in PresetPanel are missing an explicit type and currently default to
type="submit", causing outer forms to submit; update the <button> elements
rendered in the PresetPanel component to include type="button" so clicks only
trigger onClick/onMouseEnter/onMouseLeave handlers (preserve executeValue,
onClick, onHover usage and label rendering).

))}
</div>
);
}
34 changes: 33 additions & 1 deletion src/PickerInput/Popup/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ export type PopupShowTimeConfig<DateType extends object = any> = Omit<
Pick<SharedTimeProps<DateType>, 'disabledTime'>;

export interface PopupProps<DateType extends object = any, PresetValue = DateType>
extends Pick<React.InputHTMLAttributes<HTMLDivElement>, 'onFocus' | 'onBlur'>,
extends
Pick<React.InputHTMLAttributes<HTMLDivElement>, 'onFocus' | 'onBlur'>,
FooterProps<DateType>,
PopupPanelProps<DateType> {
panelRender?: SharedPickerProps['panelRender'];
containerRef?: React.Ref<HTMLDivElement>;

// Presets
presets: ValueDate<DateType>[];
Expand All @@ -45,6 +47,7 @@ export interface PopupProps<DateType extends object = any, PresetValue = DateTyp
onOk: VoidFunction;

onPanelMouseDown?: React.MouseEventHandler<HTMLDivElement>;
onEscapeKeyDown?: VoidFunction;

classNames?: SharedPickerProps['classNames'];
styles?: SharedPickerProps['styles'];
Expand All @@ -71,6 +74,8 @@ export default function Popup<DateType extends object = any>(props: PopupProps<D
onFocus,
onBlur,
onPanelMouseDown,
containerRef,
onEscapeKeyDown,

// Direction
direction,
Expand Down Expand Up @@ -213,11 +218,38 @@ export default function Popup<DateType extends object = any>(props: PopupProps<D
const marginLeft = 'marginLeft';
const marginRight = 'marginRight';

const onContainerKeyDown: React.KeyboardEventHandler<HTMLDivElement> = (e) => {
if (e.key === 'Tab') {
const container = e.currentTarget;
const focusable = Array.from(
container.querySelectorAll<HTMLElement>(
'button:not([disabled]), a:not([aria-disabled="true"]), td[tabindex="0"], li[tabindex="0"]',
),
).filter((el) => window.getComputedStyle(el).visibility !== 'hidden');
Comment on lines +221 to +228
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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

焦点循环的选择器写死了内置 DOM,定制内容会被跳过。

这里现在只会收集 button / a / td[tabindex="0"] / li[tabindex="0"]。但这个弹窗同时支持 panelRender,并且这次 PR 还把 components.nowButton / components.okButton 暴露成公开插槽;这些扩展点完全可能渲染成 inputselecttextarea 或任意带 tabIndex 的自定义节点。结果就是 Tab 循环会直接越过这些可聚焦控件,键盘导航在自定义场景下仍然是不完整的。

建议把选择器改成通用的 focusable 集合(表单控件、[href][tabindex]:not([tabindex="-1"]) 等),再统一过滤 disabled / aria-disabled / 不可见节点。

可参考的修正方向
-      const focusable = Array.from(
-        container.querySelectorAll<HTMLElement>(
-          'button:not([disabled]), a:not([aria-disabled="true"]), td[tabindex="0"], li[tabindex="0"]',
-        ),
-      ).filter((el) => window.getComputedStyle(el).visibility !== 'hidden');
+      const focusable = Array.from(
+        container.querySelectorAll<HTMLElement>(
+          [
+            'button',
+            'a[href]',
+            'input',
+            'select',
+            'textarea',
+            '[tabindex]:not([tabindex="-1"])',
+          ].join(', '),
+        ),
+      ).filter((el) => {
+        const style = window.getComputedStyle(el);
+        return (
+          !el.hasAttribute('disabled') &&
+          el.getAttribute('aria-disabled') !== 'true' &&
+          style.display !== 'none' &&
+          style.visibility !== 'hidden'
+        );
+      });
🤖 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` around lines 221 - 228, The Tab-focus
collection in onContainerKeyDown currently queries only 'button', 'a',
'td[tabindex="0"]', 'li[tabindex="0"]' so custom slot content (e.g.,
components.nowButton/components.okButton or panelRender outputs) gets skipped;
update the selector used in container.querySelectorAll inside onContainerKeyDown
to a generic focusable set (e.g., include input, select, textarea, [href], and
[tabindex]:not([tabindex="-1"]) ), then keep the existing filtering step but
extend it to exclude elements that are disabled, have aria-disabled="true", or
are not visible via getComputedStyle; ensure the rest of the function (focus
wrap logic) uses that new focusable list.


if (!focusable.length) return;

e.preventDefault();
const idx = focusable.indexOf(document.activeElement as HTMLElement);
const next = e.shiftKey
? (idx - 1 + focusable.length) % focusable.length
: (idx + 1) % focusable.length;
focusable[next].focus();
Comment on lines +233 to +237
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.

high

When document.activeElement is not in the focusable list (e.g., when focus is on the container itself), idx is -1. In this case, if the user presses Shift+Tab, the formula (idx - 1 + focusable.length) % focusable.length evaluates to focusable.length - 2 (for focusable.length > 1), which focuses the second-to-last element instead of the last one. This skips the last focusable element. We should explicitly handle idx === -1 to focus the last element on Shift+Tab and the first element on Tab.

      const idx = focusable.indexOf(document.activeElement as HTMLElement);
      const next =
        idx === -1
          ? e.shiftKey
            ? focusable.length - 1
            : 0
          : e.shiftKey
          ? (idx - 1 + focusable.length) % focusable.length
          : (idx + 1) % focusable.length;
      focusable[next].focus();

} else if (e.key === 'Escape') {
e.preventDefault();
e.stopPropagation();
onEscapeKeyDown?.();
}
};

// Container
let renderNode = (
<div
ref={containerRef}
onMouseDown={onPanelMouseDown}
onKeyDown={onContainerKeyDown}
tabIndex={-1}
role="dialog"
className={clsx(
containerPrefixCls,
// Used for Today Button style, safe to remove if no need
Expand Down
61 changes: 48 additions & 13 deletions src/PickerInput/RangePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import useFieldsInvalidate from './hooks/useFieldsInvalidate';
import useFilledProps from './hooks/useFilledProps';
import useOpen from './hooks/useOpen';
import usePickerRef from './hooks/usePickerRef';
import usePopupFocus, { focusPopupActiveCell } from './hooks/usePopupFocus';
import usePresets from './hooks/usePresets';
import useRangeActive from './hooks/useRangeActive';
import useRangeDisabledDate from './hooks/useRangeDisabledDate';
Expand Down Expand Up @@ -56,8 +57,10 @@ export type RangeValueType<DateType> = [
/** Used for change event, it should always be not undefined */
export type NoUndefinedRangeValueType<DateType> = [start: DateType | null, end: DateType | null];

export interface BaseRangePickerProps<DateType extends object>
extends Omit<SharedPickerProps<DateType>, 'showTime' | 'id'> {
export interface BaseRangePickerProps<DateType extends object> extends Omit<
SharedPickerProps<DateType>,
'showTime' | 'id'
> {
// Structure
id?: SelectorIdType;

Expand Down Expand Up @@ -132,7 +135,8 @@ export interface BaseRangePickerProps<DateType extends object>
}

export interface RangePickerProps<DateType extends object>
extends BaseRangePickerProps<DateType>,
extends
BaseRangePickerProps<DateType>,
Omit<RangeTimeProps<DateType>, 'format' | 'defaultValue' | 'defaultOpenValue'> {}

function getActiveRange(activeIndex: number) {
Expand Down Expand Up @@ -227,6 +231,7 @@ function RangePicker<DateType extends object = any>(

// ========================= Refs =========================
const selectorRef = usePickerRef(ref);
const popupRef = React.useRef<HTMLDivElement>(null);

// ======================= Semantic =======================
const [mergedClassNames, mergedStyles] = useSemantic(propClassNames, propStyles);
Expand All @@ -241,6 +246,8 @@ function RangePicker<DateType extends object = any>(
}
};

const forceClose = () => triggerOpen(false, { force: true });

// ======================== Values ========================
const [mergedValue, setInnerValue, getCalendarValue, triggerCalendarChange, triggerOk] =
useInnerValue(
Expand Down Expand Up @@ -319,6 +326,15 @@ function RangePicker<DateType extends object = any>(
const internalMode: InternalMode =
mergedMode === 'date' && mergedShowTime ? 'datetime' : mergedMode;

// ===================== Keyboard Focus =====================
const { isFocusOpenSuppressed } = usePopupFocus(
mergedOpen,
mergedMode,
activeIndex,
popupRef,
() => selectorRef.current?.focus({ index: activeIndex ?? 0 }),
);

// ====================== PanelCount ======================
const multiplePanel = internalMode === picker && internalMode !== 'time';

Expand Down Expand Up @@ -426,7 +442,7 @@ function RangePicker<DateType extends object = any>(
flushSubmit(activeIndex, nextIndex === null);

if (nextIndex === null) {
triggerOpen(false, { force: true });
forceClose();
} else if (!skipFocus) {
selectorRef.current.focus({ index: nextIndex });
}
Expand Down Expand Up @@ -454,7 +470,7 @@ function RangePicker<DateType extends object = any>(

const onSelectorClear = () => {
triggerSubmitChange(null);
triggerOpen(false, { force: true });
forceClose();
};

// ======================== Hover =========================
Expand Down Expand Up @@ -502,7 +518,7 @@ function RangePicker<DateType extends object = any>(

if (passed) {
lastOperation('preset-click');
triggerOpen(false, { force: true });
forceClose();
}
};

Expand All @@ -517,7 +533,11 @@ function RangePicker<DateType extends object = any>(

// >>> Focus
const onPanelFocus: React.FocusEventHandler<HTMLElement> = (event) => {
triggerOpen(true);
// A programmatic focus move into the panel (open / drill-down) must not
// reopen a popup that is in the middle of closing.
if (!isFocusOpenSuppressed()) {
triggerOpen(true);
}
onSharedFocus(event);
};

Expand Down Expand Up @@ -597,6 +617,8 @@ function RangePicker<DateType extends object = any>(
onFocus={onPanelFocus}
onBlur={onSharedBlur}
onPanelMouseDown={onPanelMouseDown}
containerRef={popupRef}
onEscapeKeyDown={forceClose}
// Mode
picker={picker}
mode={mergedMode}
Expand Down Expand Up @@ -668,9 +690,11 @@ function RangePicker<DateType extends object = any>(

lastOperation('input');

triggerOpen(true, {
inherit: true,
});
if (!isFocusOpenSuppressed()) {
triggerOpen(true, {
inherit: true,
});
}

// When click input to switch the field, it will not trigger close.
// Which means it will lose the part confirm and we need fill back.
Expand All @@ -696,7 +720,16 @@ function RangePicker<DateType extends object = any>(

const onSelectorKeyDown: SelectorProps['onKeyDown'] = (event, preventDefault) => {
if (event.key === 'Tab') {
triggerPartConfirm(null, true);
if (mergedOpen && !event.shiftKey) {
// Popup is open: move focus into the panel's active cell so keyboard
// users can navigate the calendar, rather than Tab jumping to the other
// field. The input blur this triggers would close the popup, but the
// panel's `onFocus` (→ `triggerOpen(true)`) keeps it open.
event.preventDefault();
focusPopupActiveCell(popupRef);
} else {
triggerPartConfirm(null, true);
}
}

onKeyDown?.(event, preventDefault);
Expand All @@ -708,7 +741,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 @@ -717,7 +751,8 @@ function RangePicker<DateType extends object = any>(
prefixCls,
locale,
generateConfig,
components.button,
components.nowButton,
components.okButton,
components.input,
mergedClassNames,
mergedStyles,
Expand Down
4 changes: 4 additions & 0 deletions src/PickerInput/Selector/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export interface InputProps extends Omit<React.InputHTMLAttributes<HTMLInputElem
format?: string;
validateFormat: (value: string) => boolean;
active?: boolean;
open?: boolean;
/** Used for single picker only */
showActiveCls?: boolean;
suffixIcon?: React.ReactNode;
Expand All @@ -54,6 +55,7 @@ const Input = React.forwardRef<InputRef, InputProps>((props, ref) => {
const {
className,
active,
open,
showActiveCls = true,
suffixIcon,
format,
Expand Down Expand Up @@ -407,6 +409,8 @@ const Input = React.forwardRef<InputRef, InputProps>((props, ref) => {
<Component
ref={inputRef}
aria-invalid={invalid}
aria-haspopup="dialog"
aria-expanded={open}
autoComplete="off"
{...restProps}
onKeyDown={onSharedKeyDown}
Expand Down
2 changes: 2 additions & 0 deletions src/PickerInput/Selector/hooks/useInputProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ export default function useInputProps<DateType extends object = any>(

active: activeIndex === index,

open,

helped: allHelp || (activeHelp && activeIndex === index),

disabled: getProp(disabled),
Expand Down
Loading