chore: adjust align logic #580
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough在 UniqueBody 中新增偏移样式缓存并在 ready 为 true 时同步；在 UniqueProvider 中把各处对 options.* 的使用统一替换为 mergedOptions.*，涵盖对齐、类名计算、依赖项与渲染门控。无公开 API 变更，仅内部渲染与样式流程调整。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Component
participant P as UniqueProvider
participant A as Align/Trigger
participant B as UniqueBody
rect rgb(240,248,255)
note over P: 计算 mergedOptions（含 postTriggerProps 合并）
C->>P: 渲染并传入 props
P->>A: useAlign(target, placement, builtinPlacements, popupAlign) 使用 mergedOptions.*
P-->>A: 提供 getPopupClassNameFromAlign / prefixCls 等
end
rect rgb(245,255,240)
note over B: 样式渲染与偏移缓存
A->>B: 传入对齐/位置信息与样式
alt ready 为 true
B->>B: cachedOffsetStyleRef.current = 最新 offsetStyle
else 未就绪
B-->>B: 保留已有 cachedOffsetStyleRef.current
end
B-->>C: 使用 {sizeStyle + motionStyle + uniqueBgStyle + cachedOffsetStyleRef.current} 渲染
end
note right of A: 对齐与类名计算统一引用 mergedOptions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分钟 Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
Summary of ChangesHello @zombieJ, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the component alignment logic by introducing a caching mechanism for offset styles, which helps stabilize positioning once a component is ready, particularly in environments where layout calculations might be unreliable. Additionally, it improves code consistency by ensuring that all alignment-related configurations are derived from a merged set of options rather than raw input options. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #580 +/- ##
==========================================
- Coverage 96.07% 95.97% -0.10%
==========================================
Files 17 17
Lines 942 945 +3
Branches 268 278 +10
==========================================
+ Hits 905 907 +2
- Misses 37 38 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| const cachedOffsetStyleRef = React.useRef(offsetStyle); | ||
|
|
||
| // Update cached offset style when ready is true | ||
| if (ready) { | ||
| cachedOffsetStyleRef.current = offsetStyle; | ||
| } |
There was a problem hiding this comment.
这部分代码存在两个问题：
-
高危：UI 闪烁 (Flicker)
此实现在浏览器中会导致 UI 闪烁。当弹窗首次打开时（open变为true），ready此时为false。offsetStyle会返回一个将弹窗移出屏幕的样式。然而，cachedOffsetStyleRef.current仍然保持着组件首次渲染时的值（通常是{ left: 0, top: 0, ... }）。这导致弹窗在计算出正确位置之前，会在0,0位置闪现一帧。 -
中危：在渲染函数中产生副作用
在渲染函数中直接修改 ref (cachedOffsetStyleRef.current = offsetStyle;) 是一个副作用。React 的函数组件应该设计为纯函数，在渲染期间执行副作用可能会在未来的 React 版本（特别是并发模式下）导致不可预测的行为。
为了解决 jsdom 中对齐无意义的问题，同时避免在浏览器中产生闪烁，需要更精细的逻辑。一个可能的思路是仅在弹窗关闭动画期间使用缓存的样式。例如：
const styleToUse = !open ? cachedOffsetStyleRef.current : offsetStyle;然后将 styleToUse 应用到 div 上。但这可能会让弹窗在 jsdom 环境下不可见，与本次修改的初衷相悖。请重新评估此方案，找到一个能同时满足测试环境和生产环境需求的方案，避免引入UI缺陷。
调整一下对齐的逻辑，平移的对齐部分 jsdom 模拟都没有什么意义
Summary by CodeRabbit