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

[EuiToken] Updated tokenTag design #5553

Merged
merged 7 commits into from
Feb 7, 2022

Conversation

elizabetdev
Copy link
Contributor

@elizabetdev elizabetdev commented Jan 24, 2022

Summary

This PR updated the tokenTag design to look more like a tag. Closes #5520.

The tokenTag is currently not being used in our products so this gives us an opportunity to update the design without causing any harm.

After a few iterations, trying to design a new token icon to represent a semantic version we decided to use the tokenTag. But without the right context, the old design didn't look like a tag. So we decided to update it.

Design

For the design, I basically used the old token. But now it has a more rectangular shape to look more like a tag. Also, I removed the inner rectangle and replaced it with a circle.

Screenshot 2022-01-24 at 17 03 37

Light and dark themes

preview-light-dark-tokenTag@2x

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 any docs examples
  • Added or updated jest and cypress 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

@kibanamachine
Copy link

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

@elizabetdev elizabetdev requested a review from cchaos February 3, 2022 14:00
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

This PR LGTM 👍

I just noticed one thing while checking across the themes. It looks like Amsterdam has a higher border-radius than the legacy theme. I think this higher border radius makes it harder to distinguish between square and circle outer shapes. Would you mind bumping this radius down to be the same for both themes?

Amsterdam:
Screen Shot 2022-02-03 at 14 34 55 PM

Legacy:
Screen Shot 2022-02-03 at 14 34 59 PM

@kibanamachine
Copy link

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

@elizabetdev
Copy link
Contributor Author

Thanks @cchaos, the border radius is now the same in both themes. I updated the screenshots on the PR description to reflect these changes. 👍🏽

@elizabetdev elizabetdev requested a review from cchaos February 4, 2022 14:57
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Thank you! You might want to add a CL item for the lower radius as well. I think that now that Amsterdam is default, we don't need to put Amsterdam specific style updates under a specific header anymore.

@kibanamachine
Copy link

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

@elizabetdev elizabetdev merged commit ec9aee6 into elastic:main Feb 7, 2022
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.

[EuiIcon] Add tokenVersion
3 participants