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

[EuiNotificationBadge] Add success color #6864

Merged
merged 9 commits into from
Jun 23, 2023
Merged

[EuiNotificationBadge] Add success color #6864

merged 9 commits into from
Jun 23, 2023

Conversation

andreadelrio
Copy link
Contributor

@andreadelrio andreadelrio commented Jun 21, 2023 •
edited
Loading

Summary

  • Add success as a new color in EuiNotificationBadge
  • Add a new prop to EuiFilterButton called badgeColor to support success in addition to accent.

The reasoning behind the new background color

  • This request originated from feedback from the Security Solution. They are using controls on their Alerts page and mentioned the dark pink color could be interpreted as something negative or alarming, which is not the intended case when displaying the number of selections in a control. They expressed a desire for a more neutral color. One suggestion that came up was to use subdued. However, we're already using that color when isDisabled is set to true. As an alternative, I suggested the success color, which they agreed with.

  • I am using the background color from the success EuiBadge as it seems to be a better fit than euiTheme.colors.successText.

Frame 148
Page where request originated

image (3)

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs (using @default if default values are missing) and playground toggles
  • Added documentation
    - [ ] Checked Code Sandbox works for any docs examples
  • Added or updated jest and cypress tests
    - [ ] Checked for breaking changes and labeled appropriately
    - [ ] Checked for accessibility including keyboard-only and screenreader modes
  • Updated the Figma library counterpart
  • A changelog entry exists and is marked appropriately

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@cee-chen
[PR feedback] DRY out badgeColor type
b2368bf
@cee-chen
[Cleanup] Create new color utilities file + set up a shared map of Em…
3070696 
…otion colors

+ convert basic EuiBadge to use emotion CSS as opposed to inline styles for named colors - color contrast is already handled automatically by our button utils

+ misc clean up/linting for various function naming, comments, syntax
@cee-chen
Update downstream snapshots
e124668
@cee-chen
Convert EuiBetaBadge and EuiNotificationBadge to use new `euiBadgeCol…
d136d62 
…ors` util

- this automatically shares (e.g.) `success` and `hollow` styles and makes colors more consistent across multiple badges

- this architecture matches how EuiButtons use a separate color map/util to determine color as opposed to each button component having its own color CSS
@cee-chen
[docs] Tweak EuiNotificationBadge example to show all colors
8faca28
@cee-chen
changelog for refactor
0298b26
cee-chen
cee-chen approved these changes Jun 22, 2023
View reviewed changes
Copy link
Member

@cee-chen cee-chen 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 chatting with me about the state of the various badge colors! The refactor I pushed up should significantly DRY out various color utilities/logic between all badge components. Feel free to give it a quick QA once staging is done updating and merge this in if everything looks good to you and you don't see any regressions on the badge page.

@kibanamachine
Copy link

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

@andreadelrio
Copy link
Contributor Author

Thanks for chatting with me about the state of the various badge colors! The refactor I pushed up should significantly DRY out various color utilities/logic between all badge components. Feel free to give it a quick QA once staging is done updating and merge this in if everything looks good to you and you don't see any regressions on the badge page.

@cee-chen Thank you for your review and the improvements you added! How can I re-trigger the CI check?

@cee-chen
Copy link
Member

jenkins test this

@cee-chen
Copy link
Member

That was a really bizarre CI failure! 😖

@kibanamachine
Copy link

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

@andreadelrio andreadelrio merged commit 746f3d7 into elastic:main Jun 23, 2023
@breehall breehall mentioned this pull request Jun 28, 2023
Merged
Ikuni17 pushed a commit to elastic/kibana that referenced this pull request Jul 6, 2023
@breehall @cee-chen
@jeramysoucy @kibanamachine
Upgrade EUI to 83.0.0 (#160813)
4cf630b 
`eui@82.1.0` ⏩ `83.0.0`

⚠️ The biggest change in this PR by far is the `EuiButtonEmpty` Emotion
conversion, which changes the DOM structure of the button slightly as
well as several CSS classes around it.

EUI has attempted to convert any custom EuiButtonEmpty CSS overrides
where possible, but would super appreciate it if CODEOWNERS checked
their touched files. If anything other than a snapshot or test was
touched, please double check the display of your button(s) and confirm
everything still looks shipshape. Feel free to ping us for advice if
not.

---

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

**Bug fixes**

- Fixed `EuiPaginationButton` styling affected by `EuiButtonEmpty`'s
Emotion conversion ([#6893](elastic/eui#6893))

**Breaking changes**

- Removed `isPlaceholder` prop from `EuiPaginationButton`
([#6893](elastic/eui#6893))

## [`82.2.1`](https://github.com/elastic/eui/tree/v82.2.1)

- Updated supported Node engine versions to allow Node 16, 18 and >=20
([#6884](elastic/eui#6884))

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

- Updated EUI's SVG icons library to use latest SVGO v3 optimization
([#6843](elastic/eui#6843))
- Added success color `EuiNotificationBadge`
([#6864](elastic/eui#6864))
- Added `badgeColor` prop to `EuiFilterButton`
([#6864](elastic/eui#6864))
- Updated `EuiBadge` to use CSS-in-JS for named colors instead of inline
styles. Custom colors will still use inline styles.
([#6864](elastic/eui#6864))

**CSS-in-JS conversions**

- Converted `EuiButtonGroup` and `EuiButtonGroupButton` to Emotion
([#6841](elastic/eui#6841))
- Converted `EuiButtonIcon` to Emotion
([#6844](elastic/eui#6844))
- Converted `EuiButtonEmpty` to Emotion
([#6863](elastic/eui#6863))
- Converted `EuiCollapsibleNav` and `EuiCollapsibleNavGroup` to Emotion
([#6865](elastic/eui#6865))
- Removed Sass variables `$euiCollapsibleNavGroupLightBackgroundColor`,
`$euiCollapsibleNavGroupDarkBackgroundColor`, and
`$euiCollapsibleNavGroupDarkHighContrastColor`
([#6865](elastic/eui#6865))

---------

Co-authored-by: Cee Chen <constance.chen@elastic.co>
Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@Heenawter Heenawter mentioned this pull request Jul 7, 2023
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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