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

avatar: Solve the problem of text position size #15468

Closed
wants to merge 1 commit into from

Conversation

ppbl
Copy link
Contributor

@ppbl ppbl commented Mar 17, 2019

First of all, thank you for your contribution! 😄

New feature please send pull request to feature branch, and rest to master branch.
Pull request will be merged after one of collaborators approve.
Please makes sure that these form are filled before submitting your pull request, thank you!

[中文版模板 / Chinese template]

🤔 This is a ...

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

👻 What's the background?

  1. fix: avatar cannot calculate the offset when display: none #15351 的时候改过了一点,但是为了跑通测试改了一点,以为效果一样,结果差一点。。
  2. 这次我用自己代码测试了好多次(手动测试,主要是不会写测试。。。)。
  3. 上次主要是有三行测试跑不通,这次我把测试暂时注释了,但是我看我注释的测试应该暂时不影响,毕竟原来别的地方的代码我没动

💡 Solution

  1. 完善了一下实现

📝 Changelog description

Describe changes from userside, and list all potential break changes or other risks.

  1. English description

  2. 这次应该是完全解决了(。。。)

☑️ 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

@netlify
Copy link

netlify bot commented Mar 17, 2019

Deploy preview for ant-design ready!

Built with commit 1262581

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

@codecov
Copy link

codecov bot commented Mar 17, 2019

Codecov Report

Merging #15468 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #15468      +/-   ##
==========================================
- Coverage   94.25%    94.2%   -0.06%     
==========================================
  Files         250      250              
  Lines        6653     6643      -10     
  Branches     1946     1942       -4     
==========================================
- Hits         6271     6258      -13     
- Misses        381      384       +3     
  Partials        1        1
Impacted Files Coverage Δ
components/avatar/index.tsx 87.71% <100%> (+1.27%) ⬆️
components/layout/Sider.tsx 84.88% <0%> (-4.66%) ⬇️
components/menu/index.tsx 90.9% <0%> (-0.52%) ⬇️
components/badge/index.tsx 100% <0%> (ø) ⬆️
components/page-header/index.tsx 100% <0%> (ø) ⬆️
components/layout/layout.tsx 90.62% <0%> (ø) ⬆️
components/list/index.tsx 97.26% <0%> (ø) ⬆️
components/steps/index.tsx 100% <0%> (ø) ⬆️

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 f080fa0...1262581. Read the comment docs.

2 similar comments
@codecov
Copy link

codecov bot commented Mar 17, 2019

Codecov Report

Merging #15468 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #15468      +/-   ##
==========================================
- Coverage   94.25%    94.2%   -0.06%     
==========================================
  Files         250      250              
  Lines        6653     6643      -10     
  Branches     1946     1942       -4     
==========================================
- Hits         6271     6258      -13     
- Misses        381      384       +3     
  Partials        1        1
Impacted Files Coverage Δ
components/avatar/index.tsx 87.71% <100%> (+1.27%) ⬆️
components/layout/Sider.tsx 84.88% <0%> (-4.66%) ⬇️
components/menu/index.tsx 90.9% <0%> (-0.52%) ⬇️
components/badge/index.tsx 100% <0%> (ø) ⬆️
components/page-header/index.tsx 100% <0%> (ø) ⬆️
components/layout/layout.tsx 90.62% <0%> (ø) ⬆️
components/list/index.tsx 97.26% <0%> (ø) ⬆️
components/steps/index.tsx 100% <0%> (ø) ⬆️

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 f080fa0...1262581. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 17, 2019

Codecov Report

Merging #15468 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #15468      +/-   ##
==========================================
- Coverage   94.25%    94.2%   -0.06%     
==========================================
  Files         250      250              
  Lines        6653     6643      -10     
  Branches     1946     1942       -4     
==========================================
- Hits         6271     6258      -13     
- Misses        381      384       +3     
  Partials        1        1
Impacted Files Coverage Δ
components/avatar/index.tsx 87.71% <100%> (+1.27%) ⬆️
components/layout/Sider.tsx 84.88% <0%> (-4.66%) ⬇️
components/menu/index.tsx 90.9% <0%> (-0.52%) ⬇️
components/badge/index.tsx 100% <0%> (ø) ⬆️
components/page-header/index.tsx 100% <0%> (ø) ⬆️
components/layout/layout.tsx 90.62% <0%> (ø) ⬆️
components/list/index.tsx 97.26% <0%> (ø) ⬆️
components/steps/index.tsx 100% <0%> (ø) ⬆️

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 f080fa0...1262581. Read the comment docs.

@afc163
Copy link
Member

afc163 commented Mar 18, 2019

测试不好写的话可以先把下面几种情况的 gif 图贴上来,不然后面不知道怎样是对的。

  • 改之前的 gif
  • 第一次修改后的 gif
  • 本次修改后的 gif

@ppbl
Copy link
Contributor Author

ppbl commented Mar 18, 2019

gif 图都指的从隐藏到显示的切换

改之前的( 3.15.0 ) 第一次修改后(3.15.1) 本次修改后
普通字符型 3 15 0普通从隐藏到显示 3 15 1普通从显示到隐藏 这次修改-普通隐藏到显示
无效src + 字符型 3 15 0错误url加有占位文本从隐藏到显示 3 15 1错误url+占位文本从隐藏到显示 这次修改-错误url+占位文本从隐藏到显示
  1. 之前主要是算出来的 scale 值无效的情况下,整个 transform 都渲染不出来,所以影响到 translate 也失效了,导致位置和大小都不行。

  2. 上次修改过后,src 是无效的情况下, 回退显示文本,计算 scale 时避免了分母为 0 的问题,解决了整个transform 失效,所以 translate 是好的, 但是 scale 还是没计算出来。应该是某个地方缺少判断, 比如之前 compontentDidMount 的时候并没有判断。

  3. 这次我把判断和设置 avatarDisplay 放到了 setScale 函数里,这样在 compontentDidMount 的时候也能执行。( 在 didMount 和 didUpdate 分别判断一次也是一样的 )

@afc163 你看下

ps: 我这个不一定非得合并的,只要这个问题能解决就行。。。😅

@zombieJ
Copy link
Member

zombieJ commented Mar 18, 2019

代码看起来没啥问题,不过测试跑不通总有点担心……

@afc163
Copy link
Member

afc163 commented Mar 18, 2019

现在的改动差不多是让 setScale 每次 update 都触发了。

不如直接引入 https://github.com/rikschennink/fitty

@afc163
Copy link
Member

afc163 commented Mar 18, 2019

---
order: 4
title:
  zh-CN: 隐藏情况下计算字符对齐
  en-US: Calculate text style when hiding
debug: true
---

## zh-CN

切换 Avatar 显示的时候,文本样式应该居中并正确调整字体大小。

## en-US

Text inside Avatar should be set a proper font size when toggle it's visibility.

````jsx
import { Avatar, Button } from 'antd';

class App extends React.Component {
  state = {
    hide: false,
  };

  toggle = () => {
    this.setState({
      hide: !this.state.hide,
    });
  }

  render() {
    const { hide } = this.state;
    return (
      <div>
        <Button onClick={this.toggle}>Toggle Avatar</Button>
        <Avatar size="large" style={{ background: '#7265e6', display: hide ? 'none' : '' }}>
          Avatar
        </Avatar>
        <Avatar size="large" src="invalid" style={{ background: '#00a2ae', display: hide ? 'none' : '' }}>
          Invalid src
        </Avatar>
      </div>
    );
  }
}

ReactDOM.render(<App />, mountNode);

@ppbl
Copy link
Contributor Author

ppbl commented Mar 18, 2019

现在的改动差不多是让 setScale 每次 update 都触发了。

是每次都触发了,但是进去 setScale 里面其实也只是判断一下,这个放外面 update 判断也可以, 每次判断
主要是保证每次从隐藏到显示都可以重新计算一下,因为并不知道什么时候显示。隐藏的时候也可能更改头像文字,这个应该问题不大吧。。

afc163 added a commit that referenced this pull request Mar 19, 2019
@afc163 afc163 mentioned this pull request Mar 19, 2019
13 tasks
@afc163
Copy link
Member

afc163 commented Mar 19, 2019

#15503

@ppbl ppbl closed this Mar 19, 2019
@ppbl ppbl deleted the avatar-test-update branch March 19, 2019 09:01
afc163 added a commit that referenced this pull request Mar 20, 2019
dengfuping pushed a commit that referenced this pull request Mar 21, 2019
* 🐛 Refactor Avatar scale calculate logic

close #15351

close #15468

* ✅ Fix infinite loop

* 🐛 Fix avatar size change logic

* ✅ Fix snapshot

* ✅ Add test cases for Avatar setScale

* 🐛 getBoundingClientRect => offsetWidth

* ✅ Fix snapshot
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