-
-
Notifications
You must be signed in to change notification settings - Fork 233
fix: the latest value was not used during position reversal #534
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.
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
base: master
Are you sure you want to change the base?
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[email protected][email protected]/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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
point
could be renamed topointStr
to 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); |
Copilot uses AI. Check for mistakes.
reversePoints(popupPoints, 0), | ||
reversePoints(targetPoints, 0), | ||
]; | ||
nextAlignInfo.points = nextAlignInfo.points.map((point) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.
Copilot uses AI. Check for mistakes.
1.修复issue: ant-design/ant-design#53432
Summary by CodeRabbit