Skip to content

[EuiFilePicker] Use alert icon when isInvalid#6678

Merged
cee-chen merged 4 commits intoelastic:mainfrom
cee-chen:filepicker/invalid
Apr 5, 2023
Merged

[EuiFilePicker] Use alert icon when isInvalid#6678
cee-chen merged 4 commits intoelastic:mainfrom
cee-chen:filepicker/invalid

Conversation

@cee-chen
Copy link
Copy Markdown
Contributor

@cee-chen cee-chen commented Apr 4, 2023 •
edited
Loading

Summary

As part of my on-call work this week, I'm finishing up some older backlog work, in this case #2017.

Before

After

Non-large:

QA

General checklist

  • Checked in both light and dark modes
  • Checked in Chrome, Safari, Edge, and Firefox
  • A changelog entry exists and is marked appropriately

- [ ] Added or updated jest and cypress tests There aren't really any for EuiFilePicker. We should remedy this when we switch to Storybook

  • Updated the Figma library counterpart - I need to do this later once I figure out Figma.

@cee-chen cee-chen changed the title [EuiFilePicker] Add alert icon for isInvalid state [EuiFilePicker] Add alert icon when isInvalid Apr 4, 2023
@cee-chen cee-chen changed the title [EuiFilePicker] Add alert icon when isInvalid [EuiFilePicker] Use alert icon when isInvalid Apr 4, 2023
@cee-chen cee-chen requested a review from daveyholler April 4, 2023 00:48
@daveyholler
Copy link
Copy Markdown
Contributor

daveyholler commented Apr 4, 2023

I'm gonna be a little pedantic… Does it make more sense here to use an error icon with the danger color?

Just thinking about the guidelines we gave to folks in that PR from a couple weeks ago that the alert icon should be used when when the alert can be safely ignored, and the error icon should be used when alert is blocking. And also how those 2 icons map to respective colors in the palette.

Though maybe that error icon would just feel out of place here?

@MichaelMarcialis tagging you on this question as well. I'll roll with what y'all think is best.

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Apr 4, 2023

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

@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Apr 4, 2023 •
edited
Loading

Every other form icon on https://elastic.github.io/eui/#/forms/form-controls uses the alert icon for isInvalid state - I would be against changing this as a one-off for EuiFilePicker. If we want to evaluate every single component at once to use error instead of alert, we should open a separate issue for that.

@cee-chen cee-chen force-pushed the filepicker/invalid branch from 597c4d1 to a164a12 Compare April 4, 2023 03:17
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Apr 4, 2023

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

@cee-chen cee-chen requested review from JasonStoltz and breehall and removed request for daveyholler April 4, 2023 16:22
@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Apr 5, 2023

@breehall do you mind reviewing this PR sometime today? Thanks!

breehall
breehall approved these changes Apr 5, 2023
View reviewed changes
Copy link
Copy Markdown
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. I used different combinations of props within the Display Toggles for EuiFilePicker in the PR preview to confirm these changes.

@cee-chen cee-chen merged commit 34748f9 into elastic:main Apr 5, 2023
@cee-chen cee-chen deleted the filepicker/invalid branch April 5, 2023 16:40
@breehall breehall mentioned this pull request Apr 12, 2023
Merged
jbudz added a commit to elastic/kibana that referenced this pull request Apr 18, 2023
@breehall @kibanamachine @jbudz
Upgrade EUI to v77.1.1 (#154838)
3259647 
EUI `77.0.0` ➡️ `77.1.1`

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

- Updated `EuiDatePicker` to display a warning icon and correctly set
`aria-invalid` when `isInvalid` is passed
([#6677](elastic/eui#6677))
- Updated `EuiFilePicker` to display an alert icon when `isInvalid`
([#6678](elastic/eui#6678))
- Updated `EuiTextArea` to display an alert icon when `isInvalid`
([#6679](elastic/eui#6679))
- Updated `EuiTextArea` to support the `isLoading` prop
([#6679](elastic/eui#6679))
- Updated `EuiComboBox` to display a warning icon and correctly set
`aria-invalid` when `isInvalid` is passed
([#6680](elastic/eui#6680))

**Bug fixes**

- Fixed `EuiAccordion` to not set an `aria-expanded` attribute on
non-interactive `buttonElement`s
([#6694](elastic/eui#6694))
- Fixed an `EuiPopoverFooter` bug causing nested popovers within
popovers (note: not a recommended use-case) to unintentionally override
its panel padding size inherited from context
([#6698](elastic/eui#6698))
- Fixed `EuiComboBox` to only delete the last selected item on backspace
if the input caret is present
([#6699](elastic/eui#6699))

---------

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

Reviewers

@JasonStoltz JasonStoltz Awaiting requested review from JasonStoltz

1 more reviewer

@breehall breehall breehall approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cee-chen @daveyholler @kibanamachine @breehall