Skip to content

fix: Fix Popup localization issue when NeedConfirm=false #941

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
wants to merge 4 commits into
base: master
Choose a base branch
from
+35 −4
Open

fix: Fix Popup localization issue when NeedConfirm=false #941

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
354b722
fix: Fix Popup localization issue when NeedConfirm=false
Sep 10, 2025
7bda580
fix: 修复 SinglePicker 组件中未传递 activeIndex 的问题，并更新 Popup 组件的注释以解释 React …
Sep 10, 2025
cc480fc
fix: 更新 PopupProps 中的 index 属性为可选，并添加注释以说明其内部用途
Sep 10, 2025
ab9f5ed
fix: 尝试解决代码覆盖率问题
Sep 10, 2025
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
13 changes: 13 additions & 0 deletions docs/examples/range.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,19 @@ export default () => {
disabledDate={disabledDate}
/>
</div>
<div>
<h3>needConfirm is false</h3>
<RangePicker<Moment>
{...sharedProps}
value={undefined}
locale={zhCN}
needConfirm={false}
picker="time"
ranges={{
test: [moment(), moment().add(1, 'hour')],
}}
/>
</div>
</div>
</div>
);
Expand Down
19 changes: 15 additions & 4 deletions src/PickerInput/Popup/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ export interface PopupProps<DateType extends object = any, PresetValue = DateTyp
activeInfo?: [activeInputLeft: number, activeInputRight: number, selectorWidth: number];
// Direction
direction?: 'ltr' | 'rtl';
// focus index
index: number;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

将新增的 index 改为可选并标注为内部属性，避免破坏性变更

把 index 作为必选公开属性会对所有直接使用 Popup 的调用方造成 TS 编译破坏。建议将其设为可选并标注内部使用，运行时仍保持解构默认值为 0，不影响现有逻辑。

应用如下 diff：

-  // focus index
-  index: number;
+  /** @internal: active input index for layout retry. Not part of public API. */
+  index?: number;
📝 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
// focus index
index: number;
/** @internal: active input index for layout retry. Not part of public API. */
index?: number;
🤖 Prompt for AI Agents 
In src/PickerInput/Popup/index.tsx around lines 37-38, make the newly added
property `index` optional and mark it as internal to avoid TypeScript breaking
changes; change its type to `index?: number` in the props/interface and add an
internal JSDoc comment (e.g. @internal) to indicate it's for internal use only,
while keeping the runtime destructuring default to 0 (e.g. const { index = 0 } =
props) so existing behavior remains unchanged.


// Fill
/** TimePicker or showTime only */
Expand All @@ -58,7 +60,7 @@ export default function Popup<DateType extends object = any>(props: PopupProps<D
range,
multiple,
activeInfo = [0, 0, 0],

index = 0,
// Presets
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

避免将内部属性 index 透传到子组件/DOM

当前仍以 {...props} 透传给 <PopupPanel> 与 <Footer>，可能让未知 prop（index）一路落到 DOM，触发 React “Unknown prop” 警告或影响 SSR 对齐。建议在本组件消费掉 index，并显式排除后再下发。

在本段解构保持不变的前提下，新增一个“去除 index 的转发 props”，并替换两处传参：

// 紧跟解构之后添加
const forwardProps = React.useMemo(() => {
  const { index: _internalIndex, ...rest } = props;
  return rest as Omit<PopupProps<DateType>, 'index'>;
}, [props]);

然后将下方两处替换为：

- <PopupPanel {...props} value={popupPanelValue} />
+ <PopupPanel {...forwardProps} value={popupPanelValue} />

- <Footer
-   {...props}
+ <Footer
+   {...forwardProps}
    showNow={multiple ? false : showNow}
    invalid={disableSubmit}
    onSubmit={onFooterSubmit}
  />
🤖 Prompt for AI Agents 
In src/PickerInput/Popup/index.tsx around lines 63-64, avoid forwarding the
internal prop `index` to child components: consume `index` locally and create a
memoized `forwardProps` that omits `index` (e.g. using object rest to strip
`index` and casting to the appropriate Omit type), then replace `{...props}`
with `{...forwardProps}` when rendering <PopupPanel> and <Footer> so the
internal `index` does not propagate to DOM/children.

presets,
onPresetHover,
Expand All @@ -80,7 +82,6 @@ export default function Popup<DateType extends object = any>(props: PopupProps<D
onOk,
onSubmit,
} = props;

const { prefixCls } = React.useContext(PickerContext);
const panelPrefixCls = `${prefixCls}-panel`;

Expand Down Expand Up @@ -118,7 +119,8 @@ export default function Popup<DateType extends object = any>(props: PopupProps<D
// Arrow Offset
const wrapperRect = wrapperRef.current.getBoundingClientRect();
if (!wrapperRect.height || wrapperRect.right < 0) {
setRetryTimes((times) => Math.max(0, times - 1));
// Index is designed to be compatible with the bug of inconsistency between React 18 useEffect and React 19
Copy link
Preview

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

The comment mentions React 19 but the description indicates this is addressing React 18 useEffect issues. The comment should be corrected to accurately reflect that this is a workaround for React 18 useEffect behavior.

Suggested change
// Index is designed to be compatible with the bug of inconsistency between React 18 useEffect and React 19
// Index is designed to be compatible with the bug of inconsistency in React 18 useEffect behavior

Copilot uses AI. Check for mistakes.

setRetryTimes((times) => Math.max(0, times - index - 1));
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

当前解释此变通方案的注释（“Index is designed to be compatible with the bug of inconsistency between React 18 useEffect and React 19”）有些模糊，并且提到了 React 19，可能会引起困惑。为了提高代码的可维护性，建议提供更详细的说明，解释为什么在 React 18 中 useEffect 的行为不一致，以及为何减去 index 可以解决这个问题。这将帮助未来的开发者更好地理解这段代码的意图。

        // 这是一个绕过 React 18 中 useEffect 行为不一致问题的变通方案。
        // 在某些情况下（例如切换输入框后），此 useEffect 在 React 18 中可能会比在 React 17 中多执行一次，
        // 导致在布局稳定前重试次数就已耗尽。
        // 通过减去当前激活的输入框索引 `index` (0 或 1) 来补偿这次额外的执行，
        // 确保定位逻辑有足够的时间在 `wrapperRef` 可用后执行。
        // 关联 issue: https://github.com/ant-design/ant-design/issues/54885
        setRetryTimes((times) => Math.max(0, times - index - 1));

return;
}

Expand All @@ -138,7 +140,16 @@ export default function Popup<DateType extends object = any>(props: PopupProps<D
setContainerOffset(0);
}
}
}, [retryTimes, rtl, containerWidth, activeInputLeft, activeInputRight, selectorWidth, range]);
}, [
retryTimes,
rtl,
containerWidth,
activeInputLeft,
activeInputRight,
selectorWidth,
range,
index,
]);

// ======================== Custom ========================
function filterEmpty<T>(list: T[]) {
Expand Down
1 change: 1 addition & 0 deletions src/PickerInput/RangePicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,7 @@ function RangePicker<DateType extends object = any>(
range
multiplePanel={multiplePanel}
activeInfo={activeInfo}
index={activeIndex}
// Disabled
disabledDate={mergedDisabledDate}
// Focus
Expand Down
Loading