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: Typography add suffix #20224

Merged
merged 15 commits into from Dec 24, 2019
Merged

feat: Typography add suffix #20224

merged 15 commits into from Dec 24, 2019

Conversation

zouxiaomingya
Copy link
Contributor

@zouxiaomingya zouxiaomingya commented Dec 12, 2019

🤔 This is a ...

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

🔗 Related issue link

💡 Background and solution

  1. Title bug Example
  2. Add middle ellipsis, Typography component adds suffix attribute

📝 Changelog

Language Changelog
🇺🇸 English Typography component adds suffix attribute, Fix title bug
🇨🇳 Chinese Typography 组件增加 suffix 属性, 修复 title 的 bug

☑️ 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/typography/demo/suffix.md
View rendered components/typography/index.en-US.md
View rendered components/typography/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?

@netlify
Copy link

netlify bot commented Dec 12, 2019

Deploy preview for ant-design ready!

Built with commit b342944

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 12, 2019

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

Sandbox Source
antd reproduction template Configuration

@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

Merging #20224 into 4.0-prepare will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@               Coverage Diff               @@
##           4.0-prepare   #20224      +/-   ##
===============================================
+ Coverage        97.56%   97.56%   +<.01%     
===============================================
  Files              295      295              
  Lines             6852     6853       +1     
  Branches          1842     1873      +31     
===============================================
+ Hits              6685     6686       +1     
  Misses             167      167
Impacted Files Coverage Δ
components/typography/Base.tsx 100% <100%> (ø) ⬆️

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 e095f09...b342944. Read the comment docs.

@zouxiaomingya zouxiaomingya changed the title feat:Typography add suffix feat: Typography add suffix Dec 13, 2019
<Paragraph ellipsis={{ rows, suffix: songForDrinking }} title={poetry + songForDrinking}>
<Text code strong>
将进酒
</Text>
Copy link
Member

@zombieJ zombieJ Dec 17, 2019

Choose a reason for hiding this comment

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

留一个英文的例子就行了

</Text>
{poetry}
</Paragraph>
<Paragraph ellipsis={{ rows, expandable: true, suffix: hamlet }} title={article + hamlet}>
Copy link
Member

@zombieJ zombieJ Dec 17, 2019

Choose a reason for hiding this comment

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

suffix: hamlet 直接写到这里,不用经过 state。让例子尽可能简单,用户易于理解。

@zombieJ
Copy link
Member

zombieJ commented Dec 17, 2019

为何 snapshot 会变?

{(ellipsisStr + suffix)
.split('')
.reverse()
.join('')}
Copy link
Member

@zombieJ zombieJ Dec 20, 2019

Choose a reason for hiding this comment

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

这段逻辑原因是什么?

Copy link
Contributor Author

@zouxiaomingya zouxiaomingya Dec 20, 2019

Choose a reason for hiding this comment

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

如果后缀都溢出
那么后缀也需要开始省略,但是后缀省略是需要往前面省略的。
这样倒过来还是可以复用 measureText 方法

// We move full content to outer element to avoid repeat read the content by accessibility
textNode = (
<span title={String(children)} aria-hidden="true">
<span title={title} aria-hidden="true">
Copy link
Member

@zombieJ zombieJ Dec 22, 2019

Choose a reason for hiding this comment

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

这里有 title 用 title,如果没有但是 children 是 string | number 则用 children。

const { children } = this.props;
if (!rows || rows < 0 || !this.content || expanded) return;

// Do not measure if css already support ellipsis
if (this.canUseCSSEllipsis()) return;
if (this.canUseCSSEllipsis() && !suffix) return;
Copy link
Member

@zombieJ zombieJ Dec 23, 2019

Choose a reason for hiding this comment

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

suffix 移入 canUseCSSEllipsis 方法内。

// We move full content to outer element to avoid repeat read the content by accessibility
textNode = (
<span title={String(children)} aria-hidden="true">
<span title={title || String(children)} aria-hidden="true">
Copy link
Member

@zombieJ zombieJ Dec 23, 2019

Choose a reason for hiding this comment

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

这里和上面的 ariaLabel 已经一模一样了,直接放进来不用重复执行

@zombieJ zombieJ merged commit 1ebf5fa into ant-design:4.0-prepare Dec 24, 2019
@linxianxi
Copy link
Contributor

linxianxi commented Apr 13, 2020

希望后缀是 ReactNode 而不只是string

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

3 participants