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

[EuiIcon] Add move glyph #7789

Merged
merged 6 commits into from
Jun 10, 2024
Merged

[EuiIcon] Add move glyph #7789

merged 6 commits into from
Jun 10, 2024

Conversation

andreadelrio
Copy link
Contributor

Summary

This new icon is needed for an upcoming new feature in Dashboard where we add hover actions to panels. One of these actions will be "move panel".

image 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 mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist
    • Updated the Figma library counterpart

@andreadelrio andreadelrio requested a review from a team as a code owner May 24, 2024 03:50
@mgadewoll
Copy link
Contributor

Hei @andreadelrio thanks for this PR! There seems to an issue with missing files (added svg, generated icon component) which looks to be a small issue introduced when we updated to a monorepo in this PR.
I updated your PR to fix it and add the missing files. Hope you don't mind 🙂

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@andreadelrio
Copy link
Contributor Author

Hei @andreadelrio thanks for this PR! There seems to an issue with missing files (added svg, generated icon component) which looks to be a small issue introduced when we updated to a monorepo in this PR. I updated your PR to fix it and add the missing files. Hope you don't mind 🙂

Thanks Lene!

@cee-chen
Copy link
Contributor

cee-chen commented Jun 3, 2024

@MichaelMarcialis Ping for designer review on this - or should we just go ahead and merge it as-is?

MichaelMarcialis
MichaelMarcialis approved these changes Jun 3, 2024
View reviewed changes
Copy link
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 the ping. This icon looks nice, @andreadelrio. The only question I have is whether it makes sense for the intersecting lines to have a 2px stroke width (as they do currently), or if it would be more consistent for them to be 1px. Most (but not all) of our arrow-based icons tend to use a 1px stroke. However, if it was better for alignment or shrinking of the icon to deviate, I'm also not opposed to it. Just curious.

That said, approving now to not hold you up, assuming you had valid reasons for the line thickness.

@andreadelrio
Copy link
Contributor Author

Thanks for the ping. This icon looks nice, @andreadelrio. The only question I have is whether it makes sense for the intersecting lines to have a 2px stroke width (as they do currently), or if it would be more consistent for them to be 1px. Most (but not all) of our arrow-based icons tend to use a 1px stroke. However, if it was better for alignment or shrinking of the icon to deviate, I'm also not opposed to it. Just curious.

That said, approving now to not hold you up, assuming you had valid reasons for the line thickness.

@MichaelMarcialis Thanks for the review. The reasoning is that I'm trying to mimic the native move cursor that web browsers have and I feel like the 2px stroke width achieves a better result.

@cee-chen cee-chen enabled auto-merge (squash) June 10, 2024 16:34
@cee-chen cee-chen merged commit 971c76e into elastic:main Jun 10, 2024
5 checks passed
@tkajtoch tkajtoch mentioned this pull request Jun 11, 2024
Merged
cee-chen added a commit to elastic/kibana that referenced this pull request Jun 18, 2024
@tkajtoch @cee-chen @kibanamachine
Upgrade EUI to v95.0.0 (#185943)
946a255 
`v94.6.0` ⏩ `v95.0.0-backport.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.0.0-backport.0`](https://github.com/elastic/eui/releases/v95.0.0-backport.0)

**This is a backport release only intended for use by Kibana.**

- Updated `EuiSteps` to support a new `titleSize="xxs"` style, which
outputs the same title font size but smaller unnumbered step indicators
([#7813](elastic/eui#7813))
- Updated `EuiStepsHorizontal` to support a new `size="xs"` style, which
outputs smaller unnumbered step indicators
([#7813](elastic/eui#7813))
- Updated `EuiStepNumber` to support new `titleSize="none"` which omits
rendering step numbers, and will only render icons
([#7813](elastic/eui#7813))

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

- Added `move` glyph to `EuiIcon`
([#7789](elastic/eui#7789))
- Updated `EuiBasicTable` and `EuiInMemoryTable`s with `selection` - the
header row checkbox will now render an indeterminate state if some (but
not all) rows are selected
([#7817](elastic/eui#7817))

**Bug fixes**

- Fixed an `EuiDataGrid` visual bug when using `lineCount` row heights
where the clamped text was still visible for some font sizes
([#7793](elastic/eui#7793))
- Fixed `EuiSearchBar`'s filter configs to always respect `autoClose:
false` ([#7806](elastic/eui#7806))

**Breaking changes**

- Removed deprecated `EUI_CHARTS_THEME_DARK`, `EUI_CHARTS_THEME_LIGHT`
and `EUI_SPARKLINE_THEME_PARTIAL` exports
([#7682](elastic/eui#7682))
- Removed deprecated `euiPalettePositive` and `euiPaletteNegative`. Use
`euiPaletteGreen` and `euiPaletteRed` instead
([#7808](elastic/eui#7808))
- Removed `type="inList"` from `EuiCheckbox`. Simply omit passing a
`label` prop to render this style of checkbox
([#7814](elastic/eui#7814))
- Removed the unused `compressed` prop from `EuiCheckbox` and
`EuiRadio`. This prop was not doing anything on individual components.
([#7818](elastic/eui#7818))

**CSS-in-JS conversions**

- Converted `EuiCheckboxGroup` to Emotion
([#7818](elastic/eui#7818))
- Converted `EuiRadioGroup` to Emotion
([#7818](elastic/eui#7818))

---------

Co-authored-by: Cee Chen <constance.chen@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>
bhapas pushed a commit to bhapas/kibana that referenced this pull request Jun 18, 2024
@tkajtoch @cee-chen
@kibanamachine @bhapas
Upgrade EUI to v95.0.0 (elastic#185943)
49030bf 
`v94.6.0` ⏩ `v95.0.0-backport.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.0.0-backport.0`](https://github.com/elastic/eui/releases/v95.0.0-backport.0)

**This is a backport release only intended for use by Kibana.**

- Updated `EuiSteps` to support a new `titleSize="xxs"` style, which
outputs the same title font size but smaller unnumbered step indicators
([elastic#7813](elastic/eui#7813))
- Updated `EuiStepsHorizontal` to support a new `size="xs"` style, which
outputs smaller unnumbered step indicators
([elastic#7813](elastic/eui#7813))
- Updated `EuiStepNumber` to support new `titleSize="none"` which omits
rendering step numbers, and will only render icons
([elastic#7813](elastic/eui#7813))

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

- Added `move` glyph to `EuiIcon`
([elastic#7789](elastic/eui#7789))
- Updated `EuiBasicTable` and `EuiInMemoryTable`s with `selection` - the
header row checkbox will now render an indeterminate state if some (but
not all) rows are selected
([elastic#7817](elastic/eui#7817))

**Bug fixes**

- Fixed an `EuiDataGrid` visual bug when using `lineCount` row heights
where the clamped text was still visible for some font sizes
([elastic#7793](elastic/eui#7793))
- Fixed `EuiSearchBar`'s filter configs to always respect `autoClose:
false` ([elastic#7806](elastic/eui#7806))

**Breaking changes**

- Removed deprecated `EUI_CHARTS_THEME_DARK`, `EUI_CHARTS_THEME_LIGHT`
and `EUI_SPARKLINE_THEME_PARTIAL` exports
([elastic#7682](elastic/eui#7682))
- Removed deprecated `euiPalettePositive` and `euiPaletteNegative`. Use
`euiPaletteGreen` and `euiPaletteRed` instead
([elastic#7808](elastic/eui#7808))
- Removed `type="inList"` from `EuiCheckbox`. Simply omit passing a
`label` prop to render this style of checkbox
([elastic#7814](elastic/eui#7814))
- Removed the unused `compressed` prop from `EuiCheckbox` and
`EuiRadio`. This prop was not doing anything on individual components.
([elastic#7818](elastic/eui#7818))

**CSS-in-JS conversions**

- Converted `EuiCheckboxGroup` to Emotion
([elastic#7818](elastic/eui#7818))
- Converted `EuiRadioGroup` to Emotion
([elastic#7818](elastic/eui#7818))

---------

Co-authored-by: Cee Chen <constance.chen@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelMarcialis MichaelMarcialis MichaelMarcialis approved these changes

@cee-chen cee-chen cee-chen 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.
6 participants
@andreadelrio @mgadewoll @kibanamachine @elasticmachine @cee-chen @MichaelMarcialis