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 legacy/undocumented alert->warning alias #6640

Merged
merged 3 commits into from
Mar 9, 2023
Merged

[EuiIcon] Add legacy/undocumented alert->warning alias #6640

merged 3 commits into from
Mar 9, 2023

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Mar 9, 2023 •
edited
Loading

Summary

After a conversation with the Elastic Charts team (@dej611 and @stratoula - thank you!) about our rename of the alert icon to warning (#6608), we've determined that this change unfortunately puts an undue onus on their team due to how their constants, saved objects, and rendering functions are tied together. A quick snippet from our conversations:

I've run thru the various editors and checked what's the impact of this new alias, and things are a bit more complicated than planned.
It's not just about rendering, but also configuration in the Lens editor. The ad hoc code has to be deployed into several places for both Annotations and Reference Lines. Also the expression requires both old and new alias to be available for validation.

An alias on EUI sounds as a better approach here, it will be quite messy to make these changes in kibana side

This PR will need to be included in a backport release to our current Kibana upgrade.

QA

  • Go to https://eui.elastic.co/pr_6640/#/display/icons
  • Confirm that the alert text is not mentioned or displayed anywhere in our docs, or in any props tables
  • Open the icon playground flyout
  • Type in alert into the type input
  • Confirm that the warning icon shows up as expected

General checklist

  • A changelog entry exists and is marked appropriately

@cee-chen cee-chen requested a review from breehall March 9, 2023 22:39
@cee-chen cee-chen changed the title [EuiIcon] Add undocumented alert->warning alias [EuiIcon] Add legacy/undocumented alert->warning alias Mar 9, 2023
breehall
breehall approved these changes Mar 9, 2023
View reviewed changes
Copy link
Contributor

@breehall breehall left a comment

Choose a reason for hiding this comment

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

Approved! Confirmed by pulling the branch locally and verifying that I can type alert to see the icon, but it is not present in any documentation. I can also type warning and see the documentation for this icon in the table of icons

@cee-chen
Copy link
Member Author

cee-chen commented Mar 9, 2023

Awesome - thanks a million Bree!

@cee-chen cee-chen enabled auto-merge (squash) March 9, 2023 22:57
@kibanamachine
Copy link

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

@cee-chen cee-chen merged commit 20eff9c into elastic:main Mar 9, 2023
@cee-chen cee-chen deleted the icon/warning-alert-alias branch March 9, 2023 23:19
@cee-chen cee-chen mentioned this pull request Mar 9, 2023
Merged
jbudz added a commit to elastic/kibana that referenced this pull request Mar 14, 2023
@JasonStoltz @daveyholler
@cee-chen @kibanamachine @jbudz
Upgraded EUI to v76.0.0 (#152506)
c339ca8 
👋 Hi all - the biggest breaking change of this PR is around two icon
type changes/renames.

1. ⚠️ **The  `alert` icon is now named `warning`**
- <img width="103" alt=""
src="https://user-images.githubusercontent.com/549407/223561599-8913e88c-676f-47cd-aaed-81b64783bd81.png"
align="middle">
- This change should have been automatically converted on your behalf by
the EUI team, **but if for some reason** we missed making this
conversion in this PR and your icon(s) are now broken, please ping us or
let us know in this PR (or fix yourself after this PR merges).
- In some cases, teams were using this icon for error messages,
alongside the `danger` color. In those cases, we opinionatedly changed
those icon usages to the new `error` icon instead of using the old
alert/warning icon.

2. 🛑 **The `crossInACircleFilled` icon has been removed, and a new
`error` icon added**
- <img width="84" alt=""
src="https://user-images.githubusercontent.com/549407/223561892-4406bdf6-1a55-49ac-85ad-3a11eb7c090d.png"
align="middle">
- The conversion for this breaking change was not straightforward. This
was the path we used to determine what to change `crossInACircleFilled`
usages to:
- If the icon was associated with errors or error messages, we changed
it to the new `error` icon.
- If a "delete" action was associated with this icon, we changed it to
the `trash` icon instead.
- If a "clear" action was associated with this icon, we changed it to
just the `cross` icon, or in some cases `minusInCircleFilled` (if used
alongside `plusInCircleFilled`).
- Again, if we made a mistake during this conversion or missed your
plugin, please feel free to ping us.

## Summary

`eui@75.1.2` ⏩ `eui@76.0.2`

## [`76.0.2`](https://github.com/elastic/eui/tree/v76.0.2)

**Bug fixes**

- Added a legacy `alert` alias for the `warning` `EuiIcon` type
([#6640](elastic/eui#6640))

## [`76.0.1`](https://github.com/elastic/eui/tree/v76.0.1)

**Bug fixes**

- Fixed broken icons on all `isInvalid` form controls
([#6629](elastic/eui#6629))

## [`76.0.0`](https://github.com/elastic/eui/tree/v76.0.0)

- Added `pivot` glyph to `EuiIcon`
([#6605](elastic/eui#6605))
- Added the `displayHeaderCellProps` API to `EuiDataGrid`'s columns,
which allows passing custom props directly to column header cells
([#6609](elastic/eui#6609))
- Added the new `headerCellProps`/`footerCellProps` APIs to
`EuiDataGrid`'s control columns, which allows passing custom props
directly to control column header or footer cells
([#6609](elastic/eui#6609))
- Added a new `footerCellRender` API to `EuiDataGrid`'s control columns,
which allows completely customizing control column rendering (previously
rendered an empty cell)
([#6609](elastic/eui#6609))
- Updated the styling of nested ordered lists in `EuiText` to align with
GitHub's list style, which is a popular format used in Markdown or MDX
formatting ([#6615](elastic/eui#6615))
- Added a margin-bottom property exclusively to the direct child `ul`
and `ol` elements of the `EuiText` component
([#6615](elastic/eui#6615))
- Fix issue with badges appearing within an `EuiBadgeGroup`, where the
CSS rule to override the `margin-inline-start` was not being applied
correctly due to the order of appearance in the CSS rules
([#6618](elastic/eui#6618))

**Bug fixes**

- Fixed `EuiDataGrid` footer control columns rendering with cell
expansion popovers when they should not have been
([#6609](elastic/eui#6609))
- Fixed an `EuiSkipLink` bug where main content loading in
progressively/dynamically after the skip link rendered was not being
correctly focused ([#6613](elastic/eui#6613))

**Breaking changes**

- Renamed `EuiIcon`'s `alert` to `warning`
([#6608](elastic/eui#6608))
- Removed `EuiIcon`'s `crossInACircleFilled` in favor of `error`
([#6608](elastic/eui#6608))

---------

Co-authored-by: Davey Holler <daveyholler@hey.com>
Co-authored-by: Constance Chen <constance.chen@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Jon <jon@elastic.co>
This was referenced Mar 20, 2023
Closed
Merged
1Copenut added a commit to elastic/kibana that referenced this pull request Mar 23, 2023
@1Copenut @mistic
Upgrade EUI to v76.3.0 (#153341)
6d55dd9 
## Summary

eui@76.0.2 ⏩ eui@76.3.0

## [`76.3.0`](https://github.com/elastic/eui/tree/v76.3.0)

- Updated `EuiSkipLink`'s `fallbackDestination` prop to support an array
of query selector strings
([#6646](elastic/eui#6646))

**Bug fixes**

- Fixed `EuiFlyout` to preserve body scrollbar width on open
([#6645](elastic/eui#6645))
- Fixed `EuiImage`'s full screen mode to not scroll jump & to preserve
body scrollbar width on open
([#6645](elastic/eui#6645))
- Fixed `EuiCodeBlock`'s full screen mode to not scroll jump & to
preserve body scrollbar width on open
([#6645](elastic/eui#6645))

## [`76.2.0`](https://github.com/elastic/eui/tree/v76.2.0)

- Added new `renderCustomGridBody` escape hatch rendering prop to
`EuiDataGrid` ([#6624](elastic/eui#6624))

**Bug fixes**

- Fixed visual listbox focus ring bug on non-searchable `EuiSelectable`s
([#6637](elastic/eui#6637))
- Added a legacy `alert` alias for the `warning` `EuiIcon` type
([#6640](elastic/eui#6640))
- Fixed a type definition incorrectly coming from a dev dependency,
which was causing issues for some consuming projects
([#6643](elastic/eui#6643))

## [`76.1.0`](https://github.com/elastic/eui/tree/v76.1.0)

- Added more detailed screen reader instructions to `EuiSelectable`,
`EuiSuggest`, `EuiSelectableTemplateSitewide`, `EuiRange`, and
`EuiDualRange`. ([#6589](elastic/eui#6589))
- Added new `placeholder` prop to `EuiSuperSelect`
([#6630](elastic/eui#6630))
- Added new `setCellPopoverProps` parameter callback to `EuiDataGrid`'s
`renderCellPopover` prop
([#6632](elastic/eui#6632))

**Bug fixes**

- Fixed an ARIA attribute in `EuiSelectableList`
([#6589](elastic/eui#6589))
- Fixed `EuiSelectable` to no longer show active selection state or
respond to the Up/Down arrow keys when focus is inside the selectable
container, but not on the searchbox or listbox.
([#6631](elastic/eui#6631))

---------

Co-authored-by: Tiago Costa <tiago.costa@elastic.co>
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Mar 24, 2023
@1Copenut @mistic @jennypavlova
Upgrade EUI to v76.3.0 (elastic#153341)
72cdbde 
## Summary

eui@76.0.2 ⏩ eui@76.3.0

## [`76.3.0`](https://github.com/elastic/eui/tree/v76.3.0)

- Updated `EuiSkipLink`'s `fallbackDestination` prop to support an array
of query selector strings
([elastic#6646](elastic/eui#6646))

**Bug fixes**

- Fixed `EuiFlyout` to preserve body scrollbar width on open
([elastic#6645](elastic/eui#6645))
- Fixed `EuiImage`'s full screen mode to not scroll jump & to preserve
body scrollbar width on open
([elastic#6645](elastic/eui#6645))
- Fixed `EuiCodeBlock`'s full screen mode to not scroll jump & to
preserve body scrollbar width on open
([elastic#6645](elastic/eui#6645))

## [`76.2.0`](https://github.com/elastic/eui/tree/v76.2.0)

- Added new `renderCustomGridBody` escape hatch rendering prop to
`EuiDataGrid` ([elastic#6624](elastic/eui#6624))

**Bug fixes**

- Fixed visual listbox focus ring bug on non-searchable `EuiSelectable`s
([elastic#6637](elastic/eui#6637))
- Added a legacy `alert` alias for the `warning` `EuiIcon` type
([elastic#6640](elastic/eui#6640))
- Fixed a type definition incorrectly coming from a dev dependency,
which was causing issues for some consuming projects
([elastic#6643](elastic/eui#6643))

## [`76.1.0`](https://github.com/elastic/eui/tree/v76.1.0)

- Added more detailed screen reader instructions to `EuiSelectable`,
`EuiSuggest`, `EuiSelectableTemplateSitewide`, `EuiRange`, and
`EuiDualRange`. ([elastic#6589](elastic/eui#6589))
- Added new `placeholder` prop to `EuiSuperSelect`
([elastic#6630](elastic/eui#6630))
- Added new `setCellPopoverProps` parameter callback to `EuiDataGrid`'s
`renderCellPopover` prop
([elastic#6632](elastic/eui#6632))

**Bug fixes**

- Fixed an ARIA attribute in `EuiSelectableList`
([elastic#6589](elastic/eui#6589))
- Fixed `EuiSelectable` to no longer show active selection state or
respond to the Up/Down arrow keys when focus is inside the selectable
container, but not on the searchbox or listbox.
([elastic#6631](elastic/eui#6631))

---------

Co-authored-by: Tiago Costa <tiago.costa@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@breehall breehall breehall 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.
3 participants
@cee-chen @kibanamachine @breehall