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.

Sign up for GitHub

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

Jump to bottom

chore: 💄 Improve site experience and fix #26528 #26538

Merged
merged 5 commits into from Sep 2, 2020
Merged

chore: 💄 Improve site experience and fix #26528 #26538

merged 5 commits into from Sep 2, 2020

Conversation

afc163
Copy link
Member

@afc163 afc163 commented Sep 2, 2020 •
edited

[中文版模板 / 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

close #26528
close #26510

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English Fix Modal closing cause scroll position jump up.
🇨🇳 Chinese 修复 Modal 关闭时导致滚动条向上跳动的问题。

☑️ 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 2, 2020 •
edited

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Sep 2, 2020 •
edited

@hengkx
Copy link
Member

hengkx commented Sep 2, 2020

modal的问题 没有 解决 依然那样

@hengkx
Copy link
Member

hengkx commented Sep 2, 2020

bug

@afc163
Copy link
Member Author

afc163 commented Sep 2, 2020 •
edited

解决的是 #26528 (comment) 这个问题。

确实还有问题。

@afc163
Copy link
Member Author

afc163 commented Sep 2, 2020

和宽度没关系，全屏也会出问题。

@afc163
Copy link
Member Author

afc163 commented Sep 2, 2020

/rebase

@shaodahong
Copy link
Member

解决的是 #26528 (comment) 这个问题。

确实还有问题。

这个是 focus tatget 的问题，和 site 没关系

@shaodahong
Copy link
Member

如果想要不 focus 可以加上 focusTriggerAfterClose: false

@afc163
Copy link
Member Author

afc163 commented Sep 2, 2020

focus 的按钮在视窗可见范围为啥会引起滚动条跳动？

@afc163
Copy link
Member Author

afc163 commented Sep 2, 2020 •
edited

好像和触发时机有关系，试试把 focusTriggerAfterClose 的行为放到滚动条重置之后再进行。

@afc163
Copy link
Member Author

afc163 commented Sep 2, 2020

/rebase

@shaodahong
Copy link
Member

嗯，其实最好的属性应该是 scrollIntoViewIfNeeded ，就是兼容性太差

@shaodahong
Copy link
Member

用 setTimeout 试了下不行，300 ms 了，感觉可以做个兼容处理，supported scrollIntoViewIfNeeded 然后做降级

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 2, 2020 •
edited

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 d661a73:

Sandbox Source
antd reproduction template Configuration

@codecov
Copy link

codecov bot commented Sep 2, 2020 •
edited

Codecov Report

Merging #26538 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #26538   +/-   ##
=======================================
  Coverage   99.75%   99.75%           
=======================================
  Files         383      383           
  Lines        7343     7343           
  Branches     2045     2051    +6     
=======================================
  Hits         7325     7325           
  Misses         18       18

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 0097d34...d661a73. Read the comment docs.

@afc163
Copy link
Member Author

afc163 commented Sep 2, 2020

和 scrollIntoViewIfNeeded 没关系，这是调用 focus，不需要 scrollIntoView。

afc163
afc163 commented Sep 2, 2020
View reviewed changes
package.json
@@ -121,7 +121,7 @@
"rc-cascader": "~1.3.0",
"rc-checkbox": "~2.3.0",
"rc-collapse": "~2.0.0",
"rc-dialog": "~8.2.0",
"rc-dialog": "~8.2.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

react-component/dialog#202

@afc163 afc163 changed the title chore: 💄 Improve site experience chore: 💄 Improve site experience and fix #26528 Sep 2, 2020
@afc163
Copy link
Member Author

afc163 commented Sep 2, 2020 •
edited

好像和触发时机有关系，试试把 focusTriggerAfterClose 的行为放到滚动条重置之后再进行。

可行，https://preview-26538-ant-design.surge.sh 实测没问题了。

@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2020

Size Change: +6 B (0%)

Total Size: 799 kB

Filename Size Change
./dist/antd.compact.min.css 66.1 kB +2 B (0%)
./dist/antd.dark.min.css 67.4 kB +3 B (0%)
./dist/antd.min.css 66.1 kB +1 B
ℹ️ View Unchanged
Filename Size Change
./dist/antd-with-locales.min.js 317 kB 0 B
./dist/antd.min.js 283 kB 0 B

compressed-size-action

@afc163 afc163 merged commit e5fe9b9 into master Sep 2, 2020
@afc163 afc163 deleted the improve-site-experience branch September 2, 2020 14:37
@pr-triage pr-triage bot added the PR: merged label Sep 2, 2020
@afc163 afc163 mentioned this pull request Sep 6, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Modal close will roll up api 适配屏幕较小的电脑
4 participants
@afc163 @ant-design-bot @hengkx @shaodahong