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

feat: Flipping table to automatically scroll to the top #18726

Merged
merged 2 commits into from Sep 26, 2019

Conversation

MrHeer
Copy link
Contributor

@MrHeer MrHeer commented Sep 8, 2019

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / document update
  • Component style update
  • TypeScript definition update
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

close #18623

💡 Background and solution

call scrollTo when handlePageChange

📝 Changelog

Language Changelog
🇺🇸 English ✨ Flipping table to automatically scroll to the top
🇨🇳 Chinese ✨ 表格翻页时自动滚动到顶部

☑️ Self Check before Merge

  • 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

View rendered components/table/demo/fixed-header.md
View rendered components/table/index.en-US.md
View rendered components/table/index.zh-CN.md

@netlify
Copy link

netlify bot commented Sep 8, 2019

Deploy preview for ant-design ready!

Built with commit 2745c06

https://deploy-preview-18726--ant-design.netlify.com

@buildsize
Copy link

buildsize bot commented Sep 8, 2019

File name Previous Size New Size Change
package-lock.json 851.16 KB 848.34 KB -2.82 KB (0%)

@yoyo837
Copy link
Contributor

yoyo837 commented Sep 8, 2019

It should also need to do this when sorting.

@yoyo837
Copy link
Contributor

yoyo837 commented Sep 8, 2019

I think it should be turned on by default,feature.

@MrHeer
Copy link
Contributor Author

MrHeer commented Sep 8, 2019

I think it should be turned on by default,feature.

Don't need to consider compatibility?

@yoyo837
Copy link
Contributor

yoyo837 commented Sep 8, 2019

Is MINOR version suitable? @afc163

@afc163
Copy link
Member

afc163 commented Sep 8, 2019

Default true is accepted, it is a new feature which could be put in minor version.

@codecov
Copy link

codecov bot commented Sep 8, 2019

Codecov Report

Merging #18726 into feature will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           feature   #18726      +/-   ##
===========================================
- Coverage    96.78%   96.74%   -0.04%     
===========================================
  Files          281      281              
  Lines         7548     7553       +5     
  Branches      2102     2106       +4     
===========================================
+ Hits          7305     7307       +2     
- Misses         241      244       +3     
  Partials         2        2
Impacted Files Coverage Δ
components/table/Table.tsx 95.43% <100%> (+0.04%) ⬆️
components/_util/wave.tsx 85.57% <0%> (-2.89%) ⬇️

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 f8af697...836b4b9. Read the comment docs.

@codecov
Copy link

codecov bot commented Sep 8, 2019

Codecov Report

Merging #18726 into feature will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           feature   #18726      +/-   ##
===========================================
+ Coverage    96.96%   96.96%   +<.01%     
===========================================
  Files          281      281              
  Lines         7601     7609       +8     
  Branches      2136     2137       +1     
===========================================
+ Hits          7370     7378       +8     
  Misses         229      229              
  Partials         2        2
Impacted Files Coverage Δ
components/table/Table.tsx 95.45% <100%> (+0.06%) ⬆️

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 53b3c5a...2745c06. Read the comment docs.

@yoyo837
Copy link
Contributor

yoyo837 commented Sep 9, 2019

https://github.com/ant-design/ant-design/blob/master/components/table/Table.tsx#L508

We should do the same thing here, I think.

components/table/Table.tsx Outdated Show resolved Hide resolved
@MrHeer
Copy link
Contributor Author

MrHeer commented Sep 9, 2019

https://github.com/ant-design/ant-design/blob/master/components/table/Table.tsx#L508

We should do the same thing here, I think.

Maybe apply it in handleFilter and toggleSortOrder?

components/table/Table.tsx Outdated Show resolved Hide resolved
components/table/Table.tsx Outdated Show resolved Hide resolved
components/table/Table.tsx Outdated Show resolved Hide resolved
components/table/Table.tsx Outdated Show resolved Hide resolved
yoyo837
yoyo837 previously approved these changes Sep 17, 2019
components/table/Table.tsx Outdated Show resolved Hide resolved
@afc163
Copy link
Member

afc163 commented Sep 24, 2019

components/table/Table.tsx Outdated Show resolved Hide resolved
@afc163
Copy link
Member

afc163 commented Sep 24, 2019

覆盖率不满足需求,或许需要加用例。

@MrHeer
Copy link
Contributor Author

MrHeer commented Sep 24, 2019

感觉最近GitHub变卡了,打开都要半天

@MrHeer
Copy link
Contributor Author

MrHeer commented Sep 25, 2019

我之前没用过Enzyme,找了半天也没找到测试滚动位置的办法。有谁能指点一下吗?

@afc163
Copy link
Member

afc163 commented Sep 25, 2019

滚动位置要靠 mock,这种很难精确测试。没关系,测试能跑到相关代码就可以了。

@MrHeer
Copy link
Contributor Author

MrHeer commented Sep 25, 2019

好的,我晚上下班后试一下

@todo
Copy link

todo bot commented Sep 25, 2019

scroll the table body then test

https://github.com/ant-design/ant-design/blob/d629ba6aa7f03645f41f6d02270858daa29a47af/components/table/__tests__/Table.pagination.test.js#L76-L81


This comment was generated by todo based on a TODO comment in d629ba6aa7f03645f41f6d02270858daa29a47af in #18726. cc @MrHeer.

👌 apply it in handleFilter and toggleSortOrder
👌 ref instead of findDOMNode
✅ update test for scroll to first row

close ant-design#18623
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.

None yet

5 participants