[EuiPagination] Remove Unneeded Styling Post `EuiButtonEmpty` Emotion Conversion #6893

breehall · 2023-06-29T14:46:26Z

Summary

When EuiPagination was converted to Emotion, a style hack was set and put in place in anticipation of the EuiButtonEmpty conversion. Now that the conversion is complete we no longer need the specificity hack.

With both styles in place, the numbers within EuiPagination aren't spaced properly.

QA

Find the incorrect styling in production and confirm that the numbers are too close together within EuiPagination
Compare staging and the PR preview before the EuiButtonEmpty conversion. Visually, they should be the same.

General checklist

Checked in Chrome, Safari, Edge, and Firefox
Checked for breaking changes and labeled appropriately
Checked for accessibility including keyboard-only and screenreader modes
A changelog entry exists and is marked appropriately

…uttonEmpty has been converted to Emotion and the hack is no longer needed.

kibanamachine · 2023-06-29T15:10:30Z

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

src/components/pagination/pagination_button.styles.ts

Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

cee-chen · 2023-06-29T16:24:41Z

Going to hang tight until staging finishes updating, and will approve after it looks good visually!

src/components/pagination/pagination_button.styles.ts

- uses the exact same styles as the default EuiPaginationButton now, so might as well reuse it + update snapshots

kibanamachine · 2023-06-29T16:36:38Z

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

+ move `cursor: default` to top-level button, now that padding is set there instead of on `__content`

- `align-items` only applies to `display: flex` items, so it's not doing anything here. We want `align-self` for children of flex groups - with the correct baseline alignment, the vertical padding is now unnecessary - the `line-height` coming from the font-size mixin is causing a surprising amount of height offset and IMO, removing it makes the entire pagination component look more visually balanced. This appears to have been the case even pre-Emotion conversions, but I think this is a useful change to make while we're here

cee-chen · 2023-06-29T16:57:30Z

src/components/pagination/pagination.styles.ts

@@ -30,8 +30,7 @@ export const euiPaginationStyles = (euiThemeContext: UseEuiTheme) => {
    euiPagination__compressedText: css`
      display: inline-flex;
      align-items: center;
-      /* Override EuiText line-height */
-      line-height: 1 !important; /* stylelint-disable-line declaration-no-important */
+      line-height: 1; /* Overrides EuiText line-height */


I forgot to document this in the commit message, but I noticed this while doing DOM/devtools QA and figured I'd throw it in here while doing other CSS cleanup - the !important isn't needed anymore since EuiText is on Emotion and everything is flat.

cee-chen · 2023-06-29T17:00:10Z

@breehall this is an actual bugfix and needs a changelog, especially if we're doing a patch release. Otherwise we're releasing a new version without a changelog, which is a no-go (our release script literally won't let us do it)

cee-chen

🚢

kibanamachine · 2023-06-29T17:32:38Z

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

- without an `eui` prefix, we're not getting the benefits of our Emotion snapshot serializer - there are no usages of `<EuiPaginationButton isPlaceholder>` anywhere in EUI or Kibana so we might as well completely remove the prop, and move its styles to the top level pagination component (where it's actually being used) - top level pagination cleanup - rename `paginationStyles`->`styles` to match all other components + remove unnecessary array which is causing extra Emotion concatenation - remove unnecessary `align-self` property from ellipsis styling - the `__list` parent is already setting it for all children

kibanamachine · 2023-06-29T18:35:39Z

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

cee-chen · 2023-06-29T18:39:19Z

@breehall FYI, technically the cleanup I pushed up is a breaking change (removes a prop) so the new release will be a major. In terms of impact, there's none to EUI or Kibana (we weren't using the prop anywhere), and likely very little elsewhere - most consumers are probably using EuiPagination only and not EuiPaginationButton. But it's the right move to mark the release as a major in any case, so just wanted to highlight that.

breehall · 2023-06-29T19:01:54Z

@cee-chen Is this ok to merge? Just wanted to confirm before pressing the button :)

cee-chen · 2023-06-29T19:09:37Z

Merge away!

`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>

[EuiPagination] Removed style hack that targeted EuiButtonEmpty. EuiB…

b1b3648

…uttonEmpty has been converted to Emotion and the hack is no longer needed.

breehall added the skip-changelog label Jun 29, 2023

breehall marked this pull request as ready for review June 29, 2023 14:58

breehall requested a review from a team June 29, 2023 15:13

cee-chen reviewed Jun 29, 2023

View reviewed changes

src/components/pagination/pagination_button.styles.ts Outdated Show resolved Hide resolved

Update src/components/pagination/pagination_button.styles.ts

0ff368a

Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>

cee-chen reviewed Jun 29, 2023

View reviewed changes

src/components/pagination/pagination_button.styles.ts Show resolved Hide resolved

DRY out pagination button arrow styles

d65b74c

- uses the exact same styles as the default EuiPaginationButton now, so might as well reuse it + update snapshots

cee-chen added 2 commits June 29, 2023 09:48

Remove all now-unnecessary && selectors

0bffcab

+ move `cursor: default` to top-level button, now that padding is set there instead of on `__content`

cee-chen reviewed Jun 29, 2023

View reviewed changes

cee-chen removed the skip-changelog label Jun 29, 2023

changelog

77d60e7

cee-chen approved these changes Jun 29, 2023

View reviewed changes

cee-chen added 2 commits June 29, 2023 10:39

update snapshots

4fe0262

breehall added the breaking change label Jun 29, 2023

breehall merged commit b0c98a6 into elastic:main Jun 29, 2023

breehall mentioned this pull request Jun 30, 2023

Upgrade EUI to 83.0.0 elastic/kibana#160813

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiPagination] Remove Unneeded Styling Post `EuiButtonEmpty` Emotion Conversion #6893

[EuiPagination] Remove Unneeded Styling Post `EuiButtonEmpty` Emotion Conversion #6893

breehall commented Jun 29, 2023 •

edited by cee-chen

Loading

kibanamachine commented Jun 29, 2023

cee-chen commented Jun 29, 2023

kibanamachine commented Jun 29, 2023

cee-chen Jun 29, 2023 •

edited

Loading

cee-chen commented Jun 29, 2023 •

edited

Loading

cee-chen left a comment

kibanamachine commented Jun 29, 2023

kibanamachine commented Jun 29, 2023

cee-chen commented Jun 29, 2023 •

edited

Loading

breehall commented Jun 29, 2023

cee-chen commented Jun 29, 2023

[EuiPagination] Remove Unneeded Styling Post EuiButtonEmpty Emotion Conversion #6893

[EuiPagination] Remove Unneeded Styling Post EuiButtonEmpty Emotion Conversion #6893

Conversation

breehall commented Jun 29, 2023 • edited by cee-chen Loading

Summary

QA

General checklist

kibanamachine commented Jun 29, 2023

cee-chen commented Jun 29, 2023

kibanamachine commented Jun 29, 2023

cee-chen Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

cee-chen commented Jun 29, 2023 • edited Loading

cee-chen left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 29, 2023

kibanamachine commented Jun 29, 2023

cee-chen commented Jun 29, 2023 • edited Loading

breehall commented Jun 29, 2023

cee-chen commented Jun 29, 2023

[EuiPagination] Remove Unneeded Styling Post `EuiButtonEmpty` Emotion Conversion #6893

[EuiPagination] Remove Unneeded Styling Post `EuiButtonEmpty` Emotion Conversion #6893

breehall commented Jun 29, 2023 •

edited by cee-chen

Loading

cee-chen Jun 29, 2023 •

edited

Loading

cee-chen commented Jun 29, 2023 •

edited

Loading

cee-chen commented Jun 29, 2023 •

edited

Loading