fix: the latest value was not used during position reversal#534
fix: the latest value was not used during position reversal#534Fang328 wants to merge 1 commit intoreact-component:masterfrom
Conversation
Walkthrough本次变更优化了 Changes
Sequence Diagram(s)sequenceDiagram
participant Popup
participant useAlign
participant Target
Popup->>useAlign: 请求对齐点计算
useAlign->>useAlign: 检查是否需要翻转
alt 需要翻转
useAlign->>useAlign: 映射并转换 points 数组
end
useAlign->>Popup: 返回调整后的对齐点
Popup->>Target: 应用对齐
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/hooks/useAlign.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs+fabric@4.0.1_postcss@8.5.3/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (5)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the position-flip logic in useAlign to ensure the latest nextAlignInfo.points values are used when reversing alignment points across different flip scenarios.
- Replace hardcoded
[popupPoints, targetPoints]arrays with a.map()-based reversal usingsplitPoints. - Apply the new reversal logic for all four flip directions (
bt,tb,rl,lr).
Comments suppressed due to low confidence (2)
src/hooks/useAlign.ts:445
- Introduce or update unit tests for each flip scenario (bt, tb, rl, lr) to verify that
nextAlignInfo.points.map(split→reverse)produces the correct reversed strings under various alignments.
nextAlignInfo.points = nextAlignInfo.points.map((point) =>
src/hooks/useAlign.ts:445
- [nitpick] The callback parameter
pointcould be renamed topointStrto more clearly indicate it's a string before splitting into components.
nextAlignInfo.points = nextAlignInfo.points.map((point) =>
| nextAlignInfo.points = nextAlignInfo.points.map((point) => | ||
| reversePoints(splitPoints(point), 0), | ||
| ); |
There was a problem hiding this comment.
The mapping logic for reversing points is repeated across multiple flip branches. Extract this into a helper (e.g., reverseAlignPoints(pointsArray, axisIndex)) to avoid duplication and improve readability.
| nextAlignInfo.points = nextAlignInfo.points.map((point) => | |
| reversePoints(splitPoints(point), 0), | |
| ); | |
| nextAlignInfo.points = reverseAlignPoints(nextAlignInfo.points, 0); |
| reversePoints(popupPoints, 0), | ||
| reversePoints(targetPoints, 0), | ||
| ]; | ||
| nextAlignInfo.points = nextAlignInfo.points.map((point) => |
There was a problem hiding this comment.
[nitpick] Add a comment explaining why splitPoints is applied before reversePoints—this clarifies the transformation of a two-character string into a Points tuple for future maintainers.
1.修复issue: ant-design/ant-design#53432
Summary by CodeRabbit