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

Error & Warning Icons #6550

Merged
merged 2 commits into from
Jan 26, 2023

Conversation

MichaelMarcialis
Copy link
Contributor

@MichaelMarcialis MichaelMarcialis commented Jan 25, 2023

Summary

I've added a new error icon and modified the existing alert icon, as part of some planned updates to how we present and distinguish between errors and warnings on Kibana visualization. Currently, we tend to use the alert icon for both warnings and errors across Kibana. The inclusion of a new error icon is to help remedy that.

Additionally, the existing alert icon was part of a subset of icons that utilized a 1.5px stroke, rather than a 1px stroke like most of our other icons. I've corrected that as part of the update to the alert icon.

As a follow-up to this PR, I think it may be wise to consider the following:

  • Consider deprecating/removing the crossInACircleFilled icon for two reasons: 1) It's very close to the new error icon, and 2) it perpetuates a problematic pattern where some EUI icons offer regular/circled/filled variants, which often lead to a lot of interface inconsistencies. I imagine it can be replaced with either a regular cross or error icon, as the context dictates.
  • Consider changing the current alert icon's name to warning. Doing so will make it clearer to developers that we have specific icons to use for error and warning cases.

If ya'll agree with those considerations, let me know and I'd be happy to write up a separate issue for them.

CCing @drewdaemon, for iconography in elastic/kibana#149262.

image

General checklist

  • Checked in both light and dark modes
  • Checked in Chrome, Safari, Edge, and Firefox
  • Added documentation
  • Added or updated jest and cypress tests
  • Updated the Figma library counterpart
  • A changelog entry exists and is marked appropriately

@MichaelMarcialis MichaelMarcialis marked this pull request as ready for review January 25, 2023 20:05
@kibanamachine
Copy link

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

@elizabetdev elizabetdev self-requested a review January 26, 2023 14:54
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. LGTM! 🎉

Don't forget to add the alert icon updates and the new error icon to the Figma library.

If ya'll agree with those considerations, let me know and I'd be happy to write up a separate issue for them.

An issue would be welcome. So we can come up with a plan to deprecate crossInACircleFilled and to evaluate if we should rename or not the alert icon to warning.

@MichaelMarcialis MichaelMarcialis merged commit e26a131 into elastic:main Jan 26, 2023
@MichaelMarcialis MichaelMarcialis deleted the icons-error-warning branch January 26, 2023 18:31
@kibanamachine
Copy link

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

@drewdaemon
Copy link

@miukimiu I'm assuming this will be available in EUI version 74.0.2. Is that right?

cee-chen pushed a commit to elastic/kibana that referenced this pull request Feb 9, 2023
## Summary

`eui@74.0.1` ⏩ `eui@74.1.0`

___

## [`74.1.0`](https://github.com/elastic/eui/releases/tag/v74.1.0)

- Added new `EuiSkeletonText`, `EuiSkeletonTitle`, `EuiSkeletonCircle`,
and `EuiSkeletonRectangle` components
([#6502](elastic/eui#6502))
- Updated `EuiSuperSelect` screen reader instructions to be more
specific ([#6549](elastic/eui#6549))
- Added `error` and updated `alert` glyphs to `EuiIcon`
([#6550](elastic/eui#6550))
- All `EuiSkeleton` components now accept an `isLoading` flag and
`children`, which automatically handles conditionally rendering loading
skeletons vs. loaded content (`children`)
([#6562](elastic/eui#6562))
- All `EuiSkeleton` components now accept a `contentAriaLabel` prop,
which more meaningfully describes the loaded content to screen readers
([#6562](elastic/eui#6562))
- Updated `EuiPopover` screen reader instructions for mobile and click
behaviors ([#6567](elastic/eui#6567))

**Bug fixes**

- Fixed `EuiCard` to ensure `onClick` method only runs once when `title`
contains a React node
([#6551](elastic/eui#6551))

**Deprecations**

- Deprecated `EuiLoadingContent` - use `EuiSkeletonText` instead
([#6557](elastic/eui#6557))

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@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.

4 participants