Skip to content
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

refator: update rc-dialog to remove rc-animate #26940

Merged
merged 11 commits into from
Sep 29, 2020
Merged

refator: update rc-dialog to remove rc-animate #26940

merged 11 commits into from
Sep 29, 2020

Conversation

zombieJ
Copy link
Member

@zombieJ zombieJ commented Sep 29, 2020

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English Refactor Dialog. It will remove all dom element when closed.
🇨🇳 Chinese 重构 Dialog 组件,现在关闭时将完全清理相关 Dom 节点。

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Sep 29, 2020

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Sep 29, 2020

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 29, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 688e14e:

Sandbox Source
antd reproduction template Configuration

@github-actions
Copy link
Contributor

github-actions bot commented Sep 29, 2020

Size Change: -619 B (0%)

Total Size: 793 kB

Filename Size Change
./dist/antd-with-locales.min.js 314 kB -305 B (0%)
./dist/antd.min.js 279 kB -314 B (0%)
ℹ️ View Unchanged
Filename Size Change
./dist/antd.compact.min.css 66.4 kB 0 B
./dist/antd.dark.min.css 67.7 kB 0 B
./dist/antd.min.css 66.4 kB 0 B

compressed-size-action

@zombieJ
Copy link
Member Author

zombieJ commented Sep 29, 2020

重构 5 小时,修复测试 2 天。每个直接在 document.body 上额外渲染的 React Instance 都是伤不起的天使!😭

Copy link
Member

@afc163 afc163 left a comment

Choose a reason for hiding this comment

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

效果看着不错。

@codecov
Copy link

codecov bot commented Sep 29, 2020

Codecov Report

Merging #26940 into feature will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           feature   #26940   +/-   ##
========================================
  Coverage    99.97%   99.97%           
========================================
  Files          385      385           
  Lines         7409     7409           
  Branches      2023     2072   +49     
========================================
  Hits          7407     7407           
  Misses           2        2           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01d757b...688e14e. Read the comment docs.

Copy link
Member

@shaodahong shaodahong left a comment

Choose a reason for hiding this comment

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

只有单元测试,碰到 dom 和动画只能 hack ,😅

@zombieJ
Copy link
Member Author

zombieJ commented Sep 29, 2020

@shaodahong 这个合完了,你的 image 也升级一下依赖哈~~

@zombieJ zombieJ merged commit 442c146 into feature Sep 29, 2020
@shaodahong
Copy link
Member

shaodahong commented Sep 29, 2020

@shaodahong 这个合完了,你的 image 也升级一下依赖哈~~

#26915

已经升了……

理解错了,明白了,明天升级下

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.

5 participants