Skip to content

Conversation

@MichaelMarcialis
Copy link
Contributor

@MichaelMarcialis MichaelMarcialis commented Feb 11, 2022

Summary

Added new lineDashed, lineDotted, and lineSolid icons to EUI.

image

CCing @mbondyra, as elastic/kibana#124970 mentions needing these icons as part of the proposed design updates to Lens flyouts.

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • Checked in Chrome, Safari, Edge, and Firefox
  • Added documentation
  • Added or updated jest and cypress tests
  • A changelog entry exists and is marked appropriately

@MichaelMarcialis MichaelMarcialis changed the title Line icons Line icons addition Feb 11, 2022
@kibanamachine
Copy link

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

@cchaos cchaos requested a review from elizabetdev February 14, 2022 18:27
Copy link
Contributor

@elizabetdev elizabetdev left a comment

Choose a reason for hiding this comment

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

Thanks, @MichaelMarcialis for adding this new great set of icons to EUI! 🎉

The icons look good to me. But I noticed that the lineDotted looks thinner when compared with the other two.

Screenshot 2022-02-14 at 19 25 41

What do you think of instead of making a circle of 2px x 2px making it instead with 3px x 3px. This change makes the circles bigger with more space between them, and it's easier to notice that they are dots.

Screenshot 2022-02-14 at 19 25 35

I guess one of the reasons you went with the 2px x 2px circles version is that they get better centered aligned. The 3px x 3px doesn't get centered properly but when is side by side with the other icons it doesn't make that much difference.

Screenshot 2022-02-14 at 19 29 22

Let me know what do you think of the proposed changes?

@MichaelMarcialis
Copy link
Contributor Author

What do you think of instead of making a circle of 2px x 2px making it instead with 3px x 3px.

Thanks for the feedback, @miukimiu! I've updated the dottedLine icon to use 3x3px dots. Updated screenshot in the original description above.

@kibanamachine
Copy link

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

Copy link
Contributor

@elizabetdev elizabetdev left a comment

Choose a reason for hiding this comment

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

Thanks, @MichaelMarcialis.

Tested in Safari, Firefox, and Chrome. Also tested in low resolution and LGTM! 🎉

@MichaelMarcialis MichaelMarcialis merged commit 5c552ad into elastic:main Feb 15, 2022
@MichaelMarcialis MichaelMarcialis deleted the line-icons branch February 15, 2022 19:26
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