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(table): remove unnecessary tooltip #1907

Merged
merged 7 commits into from Feb 23, 2021
Merged

feat(table): remove unnecessary tooltip #1907

merged 7 commits into from Feb 23, 2021

Conversation

beichensky
Copy link
Contributor

issue:ProTable Column 中未进行缩略的内容不需要展示 Tooltip #1906

@chenshuai2144
Copy link
Contributor

写的有点复杂,我有些看不懂

@beichensky
Copy link
Contributor Author

我描述的可能有点不太清晰。
简单来说就是,目前在 ProTable 中设置了 ellipsis 属性之后,无论内容是否被缩略,鼠标悬浮之后都会出现 Tooltip 提示。
现在是希望内容溢出,出现省略符号的时候再展示 Tooltip;内容没有溢出的话不需要展示 Tooltip。

@codecov
Copy link

codecov bot commented Feb 15, 2021

Codecov Report

Merging #1907 (adb8fa1) into master (00fba77) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1907      +/-   ##
==========================================
- Coverage   99.04%   99.04%   -0.01%     
==========================================
  Files         159      159              
  Lines        4723     4718       -5     
  Branches     1701     1699       -2     
==========================================
- Hits         4678     4673       -5     
  Misses         44       44              
  Partials        1        1              
Impacted Files Coverage Δ
packages/table/src/utils.tsx 100.00% <100.00%> (ø)

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 00fba77...adb8fa1. Read the comment docs.

@beichensky
Copy link
Contributor Author

修改的代码是利用了 Typography.Text 组件的 ellipsis 属性展示对应的 tooltip,没有再额外使用 Tooltip 组件进行包裹。

@chenshuai2144
Copy link
Contributor

Merge下主分支吧

@beichensky
Copy link
Contributor Author

嗯,已经 Merge 好了

@chenshuai2144
Copy link
Contributor

测试覆盖率低了一点,搞个测试用例覆盖一下吧

@beichensky
Copy link
Contributor Author

修改了一下,之前测试覆盖率低是因为没有覆盖到 genEllipsis 函数。
该函数目前用不到了,所以进行了删除。

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

Successfully merging this pull request may close these issues.

None yet

2 participants