[EuiButton][EuiButtonEmpty][EuiButtonIcon] Remove `color="ghost"` #7296

cee-chen · 2023-10-18T21:18:59Z

Summary

This PR removes color="ghost" only for EUI button components. It does not remove color="ghost" for EuiIcon, EuiText, or EuiLink.

This PR also closes #7287 (last component with .defaultProps). Removing the ghost logic allows us to correctly destructure default props in the original function params (see b7dc9b4).

Update paths

If the color="ghost" context is being used within EuiBottomBar or any other component that automatically sets a dark color mode, simply change the prop to color="text"
If not, create your own <EuiThemeProvider colorMode="dark"> wrapper around your button, and then change to color="text"

Downstream effects

This affects multiple usages in Kibana and other Elastic consumers. I went ahead and opened PRs with migrations to remove usages:

For more context, see #6820

QA

No UI should have changed or visually regressed as a result of this PR. Docs changes to QA:

https://eui.elastic.co/pr_7296/#/templates/page-template/examples#showing-a-bottom-bar - the "Share" button in the bottom right should be the correct color (white)
https://eui.elastic.co/pr_7296/#/layout/bottom-bar#positions - the "Scroll to see" text should be the correct color (white)
(Any page on staging) the Light/Dark mode theme switcher button in the top right hand corner of the site nav should be the same color as production (white)

General checklist

Browser QA
- Checked in both light and dark modes
- Checked in Chrome, Safari, Edge, and Firefox
  - [ ] Checked in mobile
  ~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~
Docs site QA
- Updated documentation
- Props have proper autodocs (using @default if default values are missing) and playground toggles
  ~~- [ ] Checked Code Sandbox works for any docs examples~~
Code quality checklist
- ~~Added or~~ updated jest and cypress tests
Release checklist
- A changelog entry exists and is marked appropriately.
- If applicable, added the breaking change issue label (and filled out the breaking change checklist)
Designer checklist
- Updated the Figma library counterpart

- This will fix our props tables and inferred `defaultProps` logic

- now throwing type errors from EuiButton ghost removal - worth noting: this component is slated for deprecation in 1 month

github-actions · 2023-10-18T21:19:16Z

This PR contains breaking changes. The opener of this pull request is asked to perform the following due diligence steps below, to assist EUI in our next Kibana upgrade:

If this PR contains prop/API changes:
- Search through Kibana for <EuiComponent usages (example search)
- In the PR description or in a PR comment, include a count or list with the number of component usages in Kibana that will need to be updated (if that amount is "none", include that information as well)
If this PR contains CSS changes: N/A

Log | Bot Usage

- has no actual user-facing effect

- appears to be because we're renaming it via destructure, doh

kibanamachine · 2023-10-18T23:50:22Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7296/
Storybook: https://eui.elastic.co/pr_7296/storybook

elasticmachine · 2023-10-18T23:50:25Z

💚 Build Succeeded

Buildkite Build
Commit: 6d6d79b

History

💚 Build #853 succeeded 5c871b2
💔 Build #852 failed d54b47b

JasonStoltz · 2023-10-19T13:26:43Z

src-docs/src/components/guide_theme_selector/guide_theme_selector.tsx

-    >
-      {isMobileSize ? 'Theme' : currentTheme.text}
-    </EuiButton>
+    <EuiThemeProvider colorMode="dark" wrapperProps={{ cloneElement: true }}>


Looking at the Update Paths, this would be a case where:

if not, create your own wrapper around your button, and then change to color="text"

So in this context, we want to maintain the "ghost" coloring of this button, which is why we wrap it in a theme provider set explicitly to "dark".

This essentially makes the button permanently and independently exist in a "dark mode" context, isolating it from any other dark/light mode changes in any higher-level theme contexts.

Does that all track? I'm just trying to understand.

Does this only impact color mode? Meaning, if other theme changes are set in a higher-level theme context, would they still flow through to this button?

This essentially makes the button permanently and independently exist in a "dark mode" context, isolating it from any other dark/light mode changes in any higher-level theme contexts.

Yeah, your understanding is totally correct!

Does this only impact color mode? Meaning, if other theme changes are set in a higher-level theme context, would they still flow through to this button?

Yes! Child theme providers will always inherit from their most recent parent. So you could even do something like this:

<EuiProvider> // App <EuiThemeProvider modify={{ size: { base: 20 } }}> // Some modified content <EuiThemeProvider colorMode="dark"> // Will inherit *both* the size override and dark color mode settings </EuiThemeProvider> </EuiThemeProvider> </EuiProvider>

Here's the source code for that behavior if you're curious (we grab the parent React context and then merge in any modifications and then pass that down as the new context):

eui/src/services/theme/provider.tsx

Lines 74 to 77 in b1c8582

const parentSystem = useContext(EuiSystemContext);

const parentModifications = useContext(EuiModificationsContext);

const parentColorMode = useContext(EuiColorModeContext);

const parentTheme = useContext(EuiThemeContext);

All the credit to Greg for creating that component with flexibility and future-proof-ness in mind!

JasonStoltz

@cee-chen These code changes all look good to me. I dropped a couple of clarifying questions but otherwise this LGTM.

JasonStoltz · 2023-10-19T13:44:02Z

src/components/button/button.tsx

      {...rest}
    />
  );
 };
-
-EuiButton.displayName = 'EuiButton';


I take it this was just redundant?

Yeah, unless we're using forwardRef or have some other reason for renaming components (e.g. HOCs and other shenanigans), we don't need to manually set a displayName.

JasonStoltz · 2023-10-19T13:46:39Z

src-docs/src/views/bottom_bar/bottom_bar_position.js

@@ -16,7 +16,7 @@ export default () => {
      <EuiSpacer size="xl" />
      <EuiSpacer size="xl" />
      <EuiBottomBar position="sticky" bottom={10}>
-        <EuiText color="ghost" textAlign="center">


Wouldn't we need color="text" here to maintain the ghost coloring?

EuiText defaults to color="text"! So we don't need to specify a color (unlike buttons which default to color="primary")

Edit to add: And EuiBottomBar already sets a dark mode context by default

JasonStoltz · 2023-10-19T13:47:29Z

src-docs/src/views/bottom_bar/bottom_bar_position.js


 export default () => {
  return (
-    <>
+    <div css={{ overflow: 'auto', blockSize: 200 }}>


This is just a bonus fix in addition to the ghost changes?

Yes! It's in its own separate commit as well 76577f7. I usually add a note to my PR descriptions recommending that folks follow along with changes by commit as I tend to throw in a lot of misc cleanup items in (but I try really hard to keep my git history clean so that optional cleanup items are separate from actual features or main functionality).

TBH I noticed a while back that the demo was broken on prod and was too lazy to fix it then, this PR was a good excuse 😅

cee-chen · 2023-10-19T16:55:52Z

FYI, I'm going to hold off on merging this into main until all the linked Kibana PRs are merged in. There's no point releasing with this until then; it'll just stall our next Kibana upgrade because this change will end up breaking/causing a bunch of type issues that we need team approvals/QA on in any case.

cee-chen · 2023-10-25T21:42:11Z

There's just 1 PR remaining in Kibana to remove usages, so I'm going to go ahead and merge this now to get it into next week's release. Worst comes to worst, I'll roll the open PR into our next Kibana upgrade which then justifies asking the KibanaOps team for an admin merge.

`v89.1.0`⏩`v90.0.0` The majority of changes in this PR come from: - **EuiContextMenu** being converted to Emotion (elastic/eui#7312). If your usage of `EuiContextMenu` was significantly affected, we recommend pulling down this PR and QAing it locally. - `defaultProps` being removed from some very widespread components, particularly **EuiButton**, in anticipation of React's upcoming deprecation. (elastic/eui@b7dc9b4) **NOTE**: This only affected Enzyme snapshots, and did not affect production behavior. [Commits](https://github.com/elastic/kibana/pull/170179/commits) have been broken up by component changes as well as types of changes. --- ## [`90.0.0`](https://github.com/elastic/eui/tree/v90.0.0) - Updated the `eventColor` prop on `EuiCommentEvent` to apply the color to the entire comment header. ([#7288](elastic/eui#7288)) - Updated `EuiBasicTable` and `EuiInMemoryTable` to support a new controlled selection API: `selection.selected` ([#7321](elastic/eui#7321)) **Bug fixes** - Fixed controlled `EuiFieldNumbers` not correctly updating native validity state ([#7291](elastic/eui#7291)) - Fixed `EuiListGroupItem` to pass `style` props to the wrapping `<li>` element alongside `className` and `css`. All other props will be passed to the underlying content. ([#7298](elastic/eui#7298)) - Fixed `EuiListGroupItem`'s non-transitioned transform on hover/focus ([#7298](elastic/eui#7298)) - Fixed `EuiDataGrid`s with `gridStyle.stripes` sometimes showing buggy row striping after being sorted ([#7301](elastic/eui#7301)) - Fixed `EuiDataGrid`'s `gridStyle.rowClasses` API to not conflict with `gridStyle.stripes` if dynamically updated ([#7301](elastic/eui#7301)) - Fixed `EuiDataGrid`'s `gridStyle.rowClasses` API to support multiple space-separated classes ([#7301](elastic/eui#7301)) - Fixed `EuiInputPopover` not calling `onPanelResize` callback prop ([#7305](elastic/eui#7305)) - Fixed `EuiDualRange` incorrectly positioning highlights when rendered with `showInput="inputWithPopover"` ([#7305](elastic/eui#7305)) - Fixed `EuiTabs` incorrectly wrapping text when it should instead either scroll or truncate ([#7309](elastic/eui#7309)) - `EuiContextMenu` now renders text colors correctly when used within an `EuiBottomBar` ([#7312](elastic/eui#7312)) - Fixed the width of `EuiSuperDatePicker`'s Absolute date picker ([#7313](elastic/eui#7313)) - Fixed `EuiDataGrid` cells visually cutting off overflowing content a little too quickly ([#7320](elastic/eui#7320)) **Deprecations** - Deprecated `EuiBasicTable` and `EuiInMemoryTable`'s ref `setSelection` API. Use the new `selection.selected` API instead. ([#7321](elastic/eui#7321)) **Breaking changes** - Removed `EuiPageTemplate_Deprecated`, `EuiPageSideBar_Deprecated`, and `EuiPageContent*_Deprecated` ([#7265](elastic/eui#7265)) - Removed the `ghost` color option from `EuiButton`, `EuiButtonEmpty`, and `EuiButtonIcon`. Use an `<EuiThemeProvider colorMode="dark">` wrapper and `color="text"` instead. ([#7296](elastic/eui#7296)) **Dependency updates** - Updated `refractor` to v3.6.0 ([#7127](elastic/eui#7127)) - Updated `rehype-raw` to v5.1.0 ([#7127](elastic/eui#7127)) - Updated `vfile` to v4.2.1 ([#7127](elastic/eui#7127)) **Accessibility** - `EuiContextMenu` now correctly respects reduced motion preferences ([#7312](elastic/eui#7312)) - `EuiAccordion`s no longer attempt to focus child content when the accordion is externally opened via `forceState`, but will continue to focus expanded content when users click the toggle button. ([#7314](elastic/eui#7314)) **CSS-in-JS conversions** - Converted `EuiContextMenu`, `EuiContextMenuPanel`, and `EuiContextMenuItem` to Emotion; Removed `$euiContextMenuWidth` ([#7312](elastic/eui#7312))

cee-chen added 6 commits October 18, 2023 13:47

Remove ghost color from EUI buttons

e7c29fd

Clean up props

b7dc9b4

- This will fix our props tables and inferred `defaultProps` logic

[docs] Remove ghost button documentation

b5e5a8c

[docs] Fix other color="ghost" button usages

be0c695

[docs] Fix broken sticky bottom bar demo

76577f7

[EuiControlBar] Remove ghost colors

b95716e

- now throwing type errors from EuiButton ghost removal - worth noting: this component is slated for deprecation in 1 month

cee-chen added the breaking change label Oct 18, 2023

cee-chen added 2 commits October 18, 2023 14:21

changelog

d54b47b

Update mounted snapshot changes caused by defaultProps removal

5c871b2

- has no actual user-facing effect

cee-chen marked this pull request as ready for review October 18, 2023 22:28

cee-chen requested a review from a team as a code owner October 18, 2023 22:28

[docs] Fix color prop not correctly showing the default value

6d6d79b

- appears to be because we're renaming it via destructure, doh

cee-chen mentioned this pull request Oct 18, 2023

[Tech debt] Remove defaultProps from function components #7297

Merged

3 tasks

JasonStoltz self-requested a review October 19, 2023 13:15

JasonStoltz reviewed Oct 19, 2023

View reviewed changes

JasonStoltz approved these changes Oct 19, 2023

View reviewed changes

cee-chen added the blocked label Oct 19, 2023

cee-chen removed the blocked label Oct 25, 2023

cee-chen merged commit c866299 into elastic:main Oct 25, 2023
10 checks passed

cee-chen deleted the button-ghost-props branch October 25, 2023 21:44

This was referenced Oct 30, 2023

Upgrade EUI to v90.0.0 elastic/kibana#170179

Merged

[EuiSelectableTemplateSitewide] Fix React wrappers breaking popoverButton behavior #7339

Merged

ryankeairns mentioned this pull request Feb 2, 2024

The contrast examples on the Theming>Colors page seem to be broken #7500

Closed

JasonStoltz mentioned this pull request Apr 15, 2024

"ghost" option for color in <Euibu #7684

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiButton][EuiButtonEmpty][EuiButtonIcon] Remove `color="ghost"` #7296

[EuiButton][EuiButtonEmpty][EuiButtonIcon] Remove `color="ghost"` #7296

cee-chen commented Oct 18, 2023 •

edited

Loading

github-actions bot commented Oct 18, 2023 •

edited by cee-chen

Loading

kibanamachine commented Oct 18, 2023

elasticmachine commented Oct 18, 2023

JasonStoltz Oct 19, 2023

cee-chen Oct 19, 2023 •

edited

Loading

JasonStoltz left a comment

JasonStoltz Oct 19, 2023

cee-chen Oct 19, 2023

JasonStoltz Oct 19, 2023

cee-chen Oct 19, 2023 •

edited

Loading

JasonStoltz Oct 19, 2023

cee-chen Oct 19, 2023

cee-chen commented Oct 19, 2023 •

edited

Loading

cee-chen commented Oct 25, 2023 •

edited

Loading

	const parentSystem = useContext(EuiSystemContext);
	const parentModifications = useContext(EuiModificationsContext);
	const parentColorMode = useContext(EuiColorModeContext);
	const parentTheme = useContext(EuiThemeContext);

[EuiButton][EuiButtonEmpty][EuiButtonIcon] Remove color="ghost" #7296

[EuiButton][EuiButtonEmpty][EuiButtonIcon] Remove color="ghost" #7296

Conversation

cee-chen commented Oct 18, 2023 • edited Loading

Summary

Update paths

Downstream effects

QA

General checklist

github-actions bot commented Oct 18, 2023 • edited by cee-chen Loading

kibanamachine commented Oct 18, 2023

elasticmachine commented Oct 18, 2023

💚 Build Succeeded

History

JasonStoltz Oct 19, 2023

Choose a reason for hiding this comment

cee-chen Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

JasonStoltz left a comment

Choose a reason for hiding this comment

JasonStoltz Oct 19, 2023

Choose a reason for hiding this comment

cee-chen Oct 19, 2023

Choose a reason for hiding this comment

JasonStoltz Oct 19, 2023

Choose a reason for hiding this comment

cee-chen Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

JasonStoltz Oct 19, 2023

Choose a reason for hiding this comment

cee-chen Oct 19, 2023

Choose a reason for hiding this comment

cee-chen commented Oct 19, 2023 • edited Loading

cee-chen commented Oct 25, 2023 • edited Loading

[EuiButton][EuiButtonEmpty][EuiButtonIcon] Remove `color="ghost"` #7296

[EuiButton][EuiButtonEmpty][EuiButtonIcon] Remove `color="ghost"` #7296

cee-chen commented Oct 18, 2023 •

edited

Loading

github-actions bot commented Oct 18, 2023 •

edited by cee-chen

Loading

cee-chen Oct 19, 2023 •

edited

Loading

cee-chen Oct 19, 2023 •

edited

Loading

cee-chen commented Oct 19, 2023 •

edited

Loading

cee-chen commented Oct 25, 2023 •

edited

Loading