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.

Sign up for GitHub

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

Jump to bottom

feat: add support to responsive sizes in Avatar component #26244

Merged
merged 1 commit into from
Aug 29, 2020
Merged

feat: add support to responsive sizes in Avatar component #26244

merged 1 commit into from
Aug 29, 2020

Conversation

willamesoares
Copy link
Contributor

@willamesoares willamesoares commented Aug 17, 2020 •
edited by markdownify bot
Loading

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

Resolves #17738

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English Support list of screen sizes to be passed in Avatar size prop
🇨🇳 Chinese 支持在 Avatar size 道具中传递的屏幕尺寸列表

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • 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/avatar/demo/responsive.md
View rendered components/avatar/index.en-US.md
View rendered components/avatar/index.zh-CN.md

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Aug 17, 2020 •
edited
Loading

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Aug 17, 2020 •
edited
Loading

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 17, 2020 •
edited
Loading

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 724bff6:

Sandbox Source
antd reproduction template Configuration

@willamesoares
Copy link
Contributor Author

Need help with zh-CN translations

@codecov
Copy link

codecov bot commented Aug 17, 2020 •
edited
Loading

Codecov Report

Merging #26244 into feature will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           feature   #26244   +/-   ##
========================================
  Coverage    99.57%   99.57%           
========================================
  Files          382      382           
  Lines         7349     7359   +10     
  Branches      2006     2011    +5     
========================================
+ Hits          7318     7328   +10     
  Misses          31       31
Impacted Files Coverage Δ
components/_util/responsiveObserve.ts 100.00% <ø> (ø)
components/avatar/avatar.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 949f1f4...724bff6. Read the comment docs.

@willamesoares
Copy link
Contributor Author

@07akioni I'm not sure why that test is failing, everything runs just fine for me locally. Can you maybe re-run the pipeline?

@willamesoares willamesoares reopened this Aug 18, 2020
@afc163
Copy link
Member

afc163 commented Aug 19, 2020

/rebase

07akioni
07akioni reviewed Aug 21, 2020
View reviewed changes
components/avatar/avatar.tsx Outdated Show resolved Hide resolved
components/avatar/avatar.tsx Outdated Show resolved Hide resolved
components/_util/responsiveObserve.ts Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Aug 23, 2020

This pull request introduces 6 alerts when merging fd5e9da into 949f1f4 - view on LGTM.com

new alerts:

  • 6 for Property access on null or undefined

@07akioni
Copy link
Contributor

may be descendingResponsiveArray?

Sorry for making a breaking suggestion😂.

responsiveArray is used by other components so its name can't be changed. It will make CI broken. Revert it then check the CI status.

image

@willamesoares
Copy link
Contributor Author

@07akioni made the updates, let me know if there's anything else.

07akioni
07akioni reviewed Aug 23, 2020
View reviewed changes
components/avatar/style/index.tsx
@@ -2,4 +2,5 @@ import '../../style/index.less';
import './index.less';

// style dependencies
// deps-lint-skip: grid
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this line for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to the fact I'm using useBreakpoint, which comes from grid, the linter fails since it requires the style for grid to be imported within avatar folder as well, however we're not using anything that depends on style from grid so I guess we're fine with just skipping it

@willamesoares
feat: add support to responsive sizes in Avatar component
724bff6 
fix lint and test failures

update avatar demo card title

update ts type check for undefined
@afc163 afc163 merged commit 9101f36 into ant-design:feature Aug 29, 2020
@afc163 afc163 mentioned this pull request Oct 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xrkffgg xrkffgg xrkffgg left review comments

@07akioni 07akioni 07akioni approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@willamesoares @ant-design-bot @afc163 @07akioni @xrkffgg