Merged
Conversation
added 2 commits
January 25, 2023 14:29
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6550/ |
elizabetdev
approved these changes
Jan 26, 2023
Contributor
elizabetdev
left a comment
There was a problem hiding this comment.
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.
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6550/ |
|
@miukimiu I'm assuming this will be available in EUI version 74.0.2. Is that right? |
This was referenced Jan 27, 2023
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
I've added a new
erroricon and modified the existingalerticon, as part of some planned updates to how we present and distinguish between errors and warnings on Kibana visualization. Currently, we tend to use thealerticon for both warnings and errors across Kibana. The inclusion of a newerroricon is to help remedy that.Additionally, the existing
alerticon 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 thealerticon.As a follow-up to this PR, I think it may be wise to consider the following:
crossInACircleFilledicon for two reasons: 1) It's very close to the newerroricon, 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 regularcrossorerroricon, as the context dictates.alerticon's name towarning. 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.
General checklist