Skip to content

fix: vertical animation not being affected by inline #578

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

Merged
merged 4 commits into from
Jan 17, 2023

Conversation

JarvisArt
Copy link
Contributor

这是个祖传bug,一两年前就有这个问题
ant-design/ant-design#39283

mode 为 vertical 和 horizontal 才需要用到 PopupTrigger,
快速切换时 inline 的 Motion 会影响到 vertical 的结束动画。

@codecov
Copy link

codecov bot commented Dec 30, 2022

Codecov Report

Merging #578 (7f557dc) into master (4bf8568) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 7f557dc differs from pull request most recent head 2e8843c. Consider uploading reports for the commit 2e8843c to get more accurate results

@@           Coverage Diff           @@
##           master     #578   +/-   ##
=======================================
  Coverage   99.85%   99.85%           
=======================================
  Files          26       26           
  Lines         707      710    +3     
  Branches      193      194    +1     
=======================================
+ Hits          706      709    +3     
  Misses          1        1           
Impacted Files Coverage Δ
src/SubMenu/PopupTrigger.tsx 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@afc163
Copy link
Member

afc163 commented Dec 30, 2022

好写 test case 么

@JarvisArt
Copy link
Contributor Author

好写 test case 么

我看看

@JarvisArt
Copy link
Contributor Author

如果没问题发个 minor 版本保险点

@@ -62,9 +62,20 @@ export default function PopupTrigger({
const popupPlacement = popupPlacementMap[mode];

const targetMotion = getMotion(mode, motion, defaultMotions);
const [innerMotion, setInnerMotion] = React.useState(targetMotion);

React.useEffect(() => {
Copy link
Member

Choose a reason for hiding this comment

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

Menu 这块越来越复杂了,v4 改由数据驱动后,重构这部分逻辑的活需要提上日程 @MadCcc

@zombieJ zombieJ requested a review from MadCcc January 3, 2023 08:08
* When collapsed is unfolded, the inline animation will destroy the vertical animation.
*/
if (mode !== 'inline') {
setInnerMotion(targetMotion);
Copy link
Member

Choose a reason for hiding this comment

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

这个不是和默认值一样嘛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里的处理主要是 modeinline 的时候,不改变上一次 modeMotioninlineMotion 会破坏其他 mode 的隐藏动画
image

Copy link
Contributor Author

@JarvisArt JarvisArt Jan 17, 2023

Choose a reason for hiding this comment

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

比如一个弹窗在鼠标失去焦点后 other 有个动画 300ms 后会让这个弹窗隐藏,但是如果300ms内快速切换成 inline mode 的动画,那么 other 的隐藏动画就会失效

Copy link
Member

Choose a reason for hiding this comment

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

懂了

Copy link
Member

@MadCcc MadCcc Jan 17, 2023

Choose a reason for hiding this comment

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

那是不是用个 ref 就好了,不需要副作用?这样少一次渲染

const motionRef = useRef(targetMotion);
if (mode !== 'inline') {
  motionRef.current = targetMotion;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

那是不是用个 ref 就好了,不需要副作用?这样少一次渲染

确实~ 等会我试下

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants