Skip to content

[EuiAvatar] Adding support for iconType, iconSize, and iconColor#4620

Merged
cchaos merged 13 commits intoelastic:masterfrom
cchaos:feature/avatar_icon
Mar 18, 2021
Merged

[EuiAvatar] Adding support for iconType, iconSize, and iconColor#4620
cchaos merged 13 commits intoelastic:masterfrom
cchaos:feature/avatar_icon

Conversation

@cchaos
Copy link
Contributor

@cchaos cchaos commented Mar 6, 2021

EuiAvatar used to only display either initials or a static background image. This PR adds support for displaying an EuiIcon.

It updates the props to the exclusion type of either initials, imageUrl OR iconType. The avatar will continue to color and size the icon based on the color and size props of the avatar but you can override this with adjacent props of iconSize and iconColor specifically.

Screen Shot 2021-03-08 at 14 18 56 PM

Screen Shot 2021-03-08 at 14 19 10 PM

Disabled:

Screen Shot 2021-03-08 at 14 17 39 PM

It also adds one named color plain matching the name of EuiPanel's for simply a white background, and the ability to completely remove the background color with null which would allow consumer's to more easily create dark/light theme colors with classNames and SASS.

This PR also breaks down the docs page a little better with more sections detailing specifically the types and disabled.

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs and playground toggles
  • Added documentation
  • Checked Code Sandbox works for the any docs examples
  • Added or updated jest tests
  • [ ] Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

cchaos added 6 commits March 4, 2021 15:11
…rl`, or `iconType`

Also updated layout to use inline-flex, and made background-image `undefined` instead of `none`
Updating all docs sections and snippets
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4620/

@cchaos cchaos changed the title [Draft][EuiAvatar] Adding support for iconType, iconSize, and iconColor [EuiAvatar] Adding support for iconType, iconSize, and iconColor Mar 8, 2021
@cchaos cchaos requested review from elizabetdev and thompsongl March 8, 2021 19:58
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4620/

@thompsongl
Copy link
Contributor

Code changes looking good. The test failure is due to a valid(?) axe a11y issue:

14:12:45 Errors on tp://localhost:9999/#/display/avatar
14:13:44 [heading-order]: Ensures the order of headings is semantically correct
14:13:44 Help: https://dequeuniversity.com/rules/axe/4.1/heading-order?application=axe-puppeteer
14:13:44 Elements:
14:13:44 - #types > div:nth-child(2) > div:nth-child(2) > h4
14:13:44 1 accessibility errors

"Spaces" h4 is the one in question.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4620/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4620/

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

LGTM with just some minor type tightening.

The imageUrl change is less impactful right now, but requiring iconType when that config section is in use will prevent setting other icon-related props when no icon is set.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4620/

@cchaos
Copy link
Contributor Author

cchaos commented Mar 18, 2021

I'm going to merge this in once it all goes green.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4620/

@cchaos cchaos merged commit 766a78b into elastic:master Mar 18, 2021
@cchaos cchaos deleted the feature/avatar_icon branch March 18, 2021 22:43
@GerardoPM GerardoPM mentioned this pull request Mar 1, 2022
5 tasks
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.

3 participants