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: support table sort by header click position with sortByCursor … #21631

Merged
merged 1 commit into from Feb 29, 2020

Conversation

AshoneA
Copy link
Contributor

@AshoneA AshoneA commented Feb 27, 2020

…props

🤔 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

💡 Background and solution

以前table排序是点击上下箭头icon排序的时候能够明确用户想要的排序是根据上还是下排序,现在table中的排序做成点击整块表头只能是根据预设规则一步一步排序, 可能会给用户带来迷惑性.。还有就是比如用户预设规则是先升序再降序, 如果用户想要直接降序排序,必须得先经过一次升序排序(依次点击效果: 升序 -> 降序)。 如果想要查看降序之后再升序,也必须得先取消一次排序(点击排序效果: 取消排序 -> 升序)。所以为了不破坏现有的排序逻辑额外加了个sortByCursor属性,能够达到用户根据点击表头中上半部分还是下半部分给用户明确的排序预期效果。

📝 Changelog

Language Changelog
🇺🇸 English Table sorter adding hint tooltip and a new prop showSorterTooltip.
🇨🇳 Chinese Taple 排序增加下次排序的提,并增加 showSorterTooltip 属性开关。

☑️ 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/index.en-US.md
View rendered components/table/index.zh-CN.md

Copy link

@tests-checker tests-checker bot left a comment

Could you please add tests to make sure this change works as expected?

@ant-design-bot
Copy link
Contributor

@ant-design-bot ant-design-bot commented Feb 27, 2020

@AshoneA AshoneA force-pushed the feat/table-sort-by-cursor branch from 0707de7 to fe7b4e5 Compare Feb 27, 2020
@codesandbox-ci
Copy link

@codesandbox-ci codesandbox-ci bot commented Feb 27, 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 52926d5:

Sandbox Source
antd reproduction template Configuration

@codecov
Copy link

@codecov codecov bot commented Feb 27, 2020

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff             @@
##           feature   #21631      +/-   ##
===========================================
+ Coverage    97.88%   97.89%   +<.01%     
===========================================
  Files          305      305              
  Lines         6998     7015      +17     
  Branches      1887     1943      +56     
===========================================
+ Hits          6850     6867      +17     
  Misses         148      148
Impacted Files Coverage Δ
components/table/hooks/useSorter.tsx 93.28% <100%> (+0.8%) ⬆️
components/table/Table.tsx 96.69% <100%> (+0.02%) ⬆️

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 994c6b8...52926d5. Read the comment docs.

@afc163
Copy link
Member

@afc163 afc163 commented Feb 27, 2020

现在的交互是参考世面上大多数的表头交互方式,上下区域的交互虽然操作路径缩短了,但是一方面是比较难点到,另一方面是视觉细节复杂(需要有样式区分点击区域)。最大的问题是容易挑战用户在各个系统内形成的交互习惯,破坏统一性。我觉得改这个还是要谨慎一点。

@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Feb 27, 2020

@afc163 嗯,我看了下MaterialUI只用了一个箭头表示排序,可能给用户感觉更清晰一点,知道下次点击的排序应该是升序还是降序(但是好像就失去了默认排序逻辑), 现在antd的交互有点像一盒巧克力,用户不知道下次点击的是升序,降序还是取消排序。因为考虑到不破坏统一性,所以采取额外加一个属性的方式来做这个配置不对现有的交互造成破坏。这个配置也是点击整个表头呀,就是判断了下点击表头时的位置,所以也不会难以点到。样式区分区域这块之前我也考虑到过,可能做成比如hover到整个表头下方的时候下箭头变成primary色?(ps: 谨慎是对的)

@afc163
Copy link
Member

@afc163 afc163 commented Feb 27, 2020

不破坏统一性 的含义是不希望开放这个配置,避免在一个表格里是这个交互在别的表格里是另一个交互,或者一个应用里是这个交互在另一个应用里是别的交互。这样来保证用户体验的连贯性。

MUI 做的一点比较好是从 未排序排序 有个半透明的箭头提示终点。也就是我们的问题 用户不知道下次点击的是升序,降序还是取消排序,我觉得我们可以只针对这点做小优化。比如未排序状态用户 hover 表头,上箭头有个轻微的高亮(浅蓝或者是比当前更灰一点);升序排序状态下 hover 表头,下箭头有个轻微的高亮(浅蓝或者是比当前更灰一点)。这样你看如何?

@AshoneA AshoneA force-pushed the feat/table-sort-by-cursor branch from fe7b4e5 to 9a8691f Compare Feb 27, 2020
@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Feb 27, 2020

@afc163 试着做了一下重新推了下代码。现在是hover时候置灰上次的排序箭头,给下次的排序箭头一个浅蓝色。你觉得需要置灰掉上次箭头吗?因为不置灰的话,如果下一次是取消排序,hover的时候提现不出来下次的排序状态。还有就是这个交互感觉用户点击之后,因为鼠标还是在表头上,所以显示的是再下次的排序箭头变浅蓝感觉给用户体验也挺奇怪的。

@AshoneA AshoneA force-pushed the feat/table-sort-by-cursor branch from 9a8691f to 14c1c90 Compare Feb 27, 2020
@afc163
Copy link
Member

@afc163 afc163 commented Feb 27, 2020

可以试试看看效果。

@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Feb 27, 2020

@afc163
Copy link
Member

@afc163 afc163 commented Feb 27, 2020

你觉得需要置灰掉上次箭头吗

还是不要变色比较好,否则点一次直接是向下箭头了。

@AshoneA AshoneA force-pushed the feat/table-sort-by-cursor branch from 14c1c90 to 1231eee Compare Feb 28, 2020
@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Feb 28, 2020

@afc163 改动了下,变成不置灰了,我也感觉这样好一点。但如果用户下次是取消排序的话,那其实hover上去没有提示了,其实没有提示下次排序是什么好像也是一种提示,因为如果下次有其他排序的话就有淡蓝色箭头激活了

@afc163
Copy link
Member

@afc163 afc163 commented Feb 28, 2020

我再想想。

@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Feb 28, 2020

@afc163 或者我还有一个idea,在鼠标hover的时候给个tooltip,比如说点击降序之类的。

@afc163
Copy link
Member

@afc163 afc163 commented Feb 28, 2020

@afc163 或者我还有一个idea,在鼠标hover的时候给个tooltip,比如说点击降序之类的。

这个不错,不过就是文案容易太长?『点击升序』『当前升序,点击降序』『当前降序,回到未排序』

@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Feb 28, 2020

@afc163 或者我还有一个idea,在鼠标hover的时候给个tooltip,比如说点击降序之类的。

这个不错,不过就是文案容易太长?『点击升序』『当前升序,点击降序』『当前降序,回到未排序』

我觉得只要三个文案就好了, 『点击升序』, 『点击降序』,『取消排序』,用户关心的应该是下次排序,已经存在的排序箭头的蓝色已经指示了。

@afc163
Copy link
Member

@afc163 afc163 commented Feb 28, 2020

那就按这个来,记得加上英文。

@AshoneA AshoneA force-pushed the feat/table-sort-by-cursor branch from 1231eee to b52a46e Compare Feb 28, 2020
@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Feb 28, 2020

修改了点css达到了hover整个header时候出来tooltip,我检查了下应该没有什么break。

@afc163
Copy link
Member

@afc163 afc163 commented Feb 28, 2020

加个开关 showSorterTooltip,默认为 true 吧,我担心用户在页头加了业务的 tooltip 两个会重叠,给一个配置能关。

@AshoneA AshoneA force-pushed the feat/table-sort-by-cursor branch from b52a46e to e641252 Compare Feb 28, 2020
@AshoneA AshoneA force-pushed the feat/table-sort-by-cursor branch from e641252 to 52926d5 Compare Feb 29, 2020
@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Feb 29, 2020

tablecolumn都加了showSorterTooltipcolumn配置优先级高于tableshowSorterTooltip,默认为true,测试用例已添加。

@afc163
Copy link
Member

@afc163 afc163 commented Feb 29, 2020

发到 feature 分支吧,预计会在 4.1.0 版本上。

@AshoneA AshoneA changed the base branch from master to feature Feb 29, 2020
@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Feb 29, 2020

我把这个base改成feature就 好了吗,我更新了下changelog,因为跟最初的changelog已经不一样了。

afc163
afc163 approved these changes Feb 29, 2020
@afc163 afc163 merged commit e262d14 into ant-design:feature Feb 29, 2020
20 checks passed
@pr-triage pr-triage bot added the PR: merged label Feb 29, 2020
@zombieJ zombieJ mentioned this pull request Mar 29, 2020
10 tasks
@leon9665
Copy link

@leon9665 leon9665 commented Jun 29, 2021

tablecolumn都加了showSorterTooltipcolumn配置优先级高于tableshowSorterTooltip,默认为true,测试用例已添加。

为啥要默认开启 本来就不是必需的功能 怎么全局关闭

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

4 participants