Skip to content

[EuiIcon] Add new expand glyphs#7875

Merged
cee-chen merged 6 commits intoelastic:mainfrom
andreadelrio:expand-icons
Jul 24, 2024
Merged

[EuiIcon] Add new expand glyphs#7875
cee-chen merged 6 commits intoelastic:mainfrom
andreadelrio:expand-icons

Conversation

@andreadelrio
Copy link
Copy Markdown
Contributor

Summary

Added the following new icon glyphs for use in [Discover][DocViewer] Limit the height of long field values by default #183736:

  • minusInSquare
  • plusInSquare
image

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in Chrome, Safari, Edge, and Firefox
  • Docs site QA
  • Release checklist
    • A changelog entry exists and is marked appropriately.
  • Designer checklist
    • Updated the Figma library counterpart

@andreadelrio andreadelrio requested a review from a team as a code owner July 8, 2024 05:14
@cee-chen cee-chen requested a review from MichaelMarcialis July 8, 2024 18:01
@tkajtoch
Copy link
Copy Markdown
Member

tkajtoch commented Jul 9, 2024

Code changes LGTM

Copy link
Copy Markdown
Contributor

@MichaelMarcialis MichaelMarcialis left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together, @andreadelrio. The icons look great. Leaving some small comments/questions below for your review. Once those are addressed though, I think you're good to go. Approving now so I don't hold you up.


  • Pixel grid: The plus and minus vector shapes within the box don't rest perfectly on the pixel grid, which means they will be slightly blurry on displays without a high pixel density. I'm guessing this was done so that the icon itself can be perfectly centered in the viewport. Historically though, I think we've tended to give higher priority to the pixel grid rather than perfect alignment. Would offsetting the icon alignment by 1px be OK to achieve better grid alignment?

    CleanShot 2024-07-09 at 11 04 53

  • Guidelines for usage: Should we provide some guidance somewhere that instructs designers when/where/why to use these icons versus the arrow icons that we typically use for accordion-type expand/collapse behavior? Ideally, it would be good to proactively reduce the potential for inconsistency if possible.

cee-chen and others added 3 commits July 17, 2024 08:47
Co-authored-by: Michael Marcialis <michael.l.marcialis@gmail.com>
Co-authored-by: Michael Marcialis <michael.l.marcialis@gmail.com>
@andreadelrio
Copy link
Copy Markdown
Contributor Author

@MichaelMarcialis thanks Michael for the feedback, I've addressed it. @cee-chen I have pushed my changes but for some reason the new ML icons Joana recently added got their .tsx files modified. Can you please provide some guidance and/or help? Not sure if it's relevant but I'm on a brand new laptop.

@cee-chen
Copy link
Copy Markdown
Contributor

Should be fixed now!

@cee-chen cee-chen enabled auto-merge (squash) July 24, 2024 16:35
@cee-chen
Copy link
Copy Markdown
Contributor

buildkite test this

@cee-chen cee-chen merged commit 002ab58 into elastic:main Jul 24, 2024
@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

History

jbudz pushed a commit to elastic/kibana that referenced this pull request Aug 1, 2024
`v95.4.0` ⏩ `v95.5.0`

_[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_

---

## [`v95.5.0`](https://github.com/elastic/eui/releases/v95.5.0)

- Added `minusInSquare` and `plusInSquare` glyphs to `EuiIcon`.
([#7875](elastic/eui#7875))

**Bug fixes**

- Fixed `EuiSuperDatePicker` not correctly passing `refreshMinInterval`
from the quick select popover
([#7905](elastic/eui#7905))

**CSS-in-JS conversions**

- Converted `EuiSuperDatePicker`'s form control to Emotion;
([#7904](elastic/eui#7904))
  - Removed `$euiSuperDatePickerWidth`
  - Removed `$euiSuperDatePickerButtonWidth`
  - Removed `$euiSuperDatePickerNeedsUpdatingBackgroundColor`
  - Removed `$euiSuperDatePickerNeedsUpdatingTextColor`
  - Removed `@euiSuperDatePickerText` mixin
- Converted `EuiSuperDatePicker`'s date popover content to Emotion
([#7908](elastic/eui#7908))
- Converted `EuiSuperDatePicker`'s quick select to Emotion
([#7909](elastic/eui#7909))

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
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.

5 participants