Skip to content

chore: adjust align logic #580

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

Merged
merged 1 commit into from
Sep 23, 2025
Merged

chore: adjust align logic #580

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
26 changes: 18 additions & 8 deletions src/UniqueProvider/UniqueBody.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ const UniqueBody = (props: UniqueBodyProps) => {
offsetY,
);

// Cache for offsetStyle when ready is true
const cachedOffsetStyleRef = React.useRef(offsetStyle);

// Update cached offset style when ready is true
if (ready) {
cachedOffsetStyleRef.current = offsetStyle;
}
Comment on lines +60 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

high

这部分代码存在两个问题：

  1. 高危：UI 闪烁 (Flicker)
    此实现在浏览器中会导致 UI 闪烁。当弹窗首次打开时（open 变为 true），ready 此时为 false。offsetStyle 会返回一个将弹窗移出屏幕的样式。然而，cachedOffsetStyleRef.current 仍然保持着组件首次渲染时的值（通常是 { left: 0, top: 0, ... }）。这导致弹窗在计算出正确位置之前，会在 0,0 位置闪现一帧。

  2. 中危：在渲染函数中产生副作用
    在渲染函数中直接修改 ref (cachedOffsetStyleRef.current = offsetStyle;) 是一个副作用。React 的函数组件应该设计为纯函数，在渲染期间执行副作用可能会在未来的 React 版本（特别是并发模式下）导致不可预测的行为。

为了解决 jsdom 中对齐无意义的问题，同时避免在浏览器中产生闪烁，需要更精细的逻辑。一个可能的思路是仅在弹窗关闭动画期间使用缓存的样式。例如：

const styleToUse = !open ? cachedOffsetStyleRef.current : offsetStyle;

然后将 styleToUse 应用到 div 上。但这可能会让弹窗在 jsdom 环境下不可见，与本次修改的初衷相悖。请重新评估此方案，找到一个能同时满足测试环境和生产环境需求的方案，避免引入UI缺陷。


// Apply popup size if available
const sizeStyle: React.CSSProperties = {};
if (popupSize) {
Expand Down Expand Up @@ -85,14 +93,16 @@ const UniqueBody = (props: UniqueBodyProps) => {
return (
<div
className={cls}
style={{
'--arrow-x': `${arrowPos?.x || 0}px`,
'--arrow-y': `${arrowPos?.y || 0}px`,
...offsetStyle,
...sizeStyle,
...motionStyle,
...uniqueBgStyle,
} as React.CSSProperties}
style={
{
'--arrow-x': `${arrowPos?.x || 0}px`,
'--arrow-y': `${arrowPos?.y || 0}px`,
...cachedOffsetStyleRef.current,
...sizeStyle,
...motionStyle,
...uniqueBgStyle,
} as React.CSSProperties
}
/>
);
}}
Expand Down
24 changes: 12 additions & 12 deletions src/UniqueProvider/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,35 +105,35 @@ const UniqueProvider = ({ children, postTriggerProps }: UniqueProviderProps) =>
] = useAlign(
open,
popupEle,
options?.target,
options?.popupPlacement,
options?.builtinPlacements || {},
options?.popupAlign,
mergedOptions?.target,
mergedOptions?.popupPlacement,
mergedOptions?.builtinPlacements || {},
mergedOptions?.popupAlign,
undefined, // onPopupAlign
false, // isMobile
);

const alignedClassName = React.useMemo(() => {
if (!options) {
if (!mergedOptions) {
return '';
}

const baseClassName = getAlignPopupClassName(
options.builtinPlacements || {},
options.prefixCls || '',
mergedOptions.builtinPlacements || {},
mergedOptions.prefixCls || '',
alignInfo,
false, // alignPoint is false for UniqueProvider
);

return classNames(
baseClassName,
options.getPopupClassNameFromAlign?.(alignInfo),
mergedOptions.getPopupClassNameFromAlign?.(alignInfo),
);
}, [
alignInfo,
options?.getPopupClassNameFromAlign,
options?.builtinPlacements,
options?.prefixCls,
mergedOptions?.getPopupClassNameFromAlign,
mergedOptions?.builtinPlacements,
mergedOptions?.prefixCls,
]);

const contextValue = React.useMemo<UniqueContextProps>(
Expand Down Expand Up @@ -171,7 +171,7 @@ const UniqueProvider = ({ children, postTriggerProps }: UniqueProviderProps) =>
return (
<UniqueContext.Provider value={contextValue}>
{children}
{options && (
{mergedOptions && (
<TriggerContext.Provider value={triggerContextValue}>
<Popup
ref={setPopupRef}
Expand Down
Loading