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

[EuiCheckbox][EuiRadio] Remove compressed props #7818

Merged
merged 4 commits into from
Jun 10, 2024
Merged

[EuiCheckbox][EuiRadio] Remove compressed props #7818

merged 4 commits into from
Jun 10, 2024

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Jun 7, 2024 •
edited by mgadewoll
Loading

Summary

Note

This PR is merging into main and not a feature branch because it contains a breaking change that I want to roll into June's deprecations.

Discussed in #7781 (comment)

The compressed prop by itself on EuiCheckbox and EuiRadio does nothing visually - it only has an impact on parent EuiCheckboxGroup/EuiRadioGroups. We should remove it from the individual components as it's misleading API, and have the parent groups handle spacing via flex and gap instead.

While here I also noticed that EuiCheckboxGroup and EuiRadioGroup are missing EUI classNames and added those as well 🤷

QA

  • EuiCheckboxGroup looks the same as production with no regressions (compressed & uncompressed)
  • EuiRadioGroup looks the same as production with no regressions (compressed & uncompressed)
  • EuiCheckbox and EuiRadio do not have compressed prop documentation any longer

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
      - [ ] Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • 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

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 7, 2024 •
edited by cee-chen
Loading

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:
    • Search through Kibana for the changed EUI selectors, e.g. .euiComponent (example search)
    • In the PR description or in a PR comment, include a count or list with the number of custom CSS overrides in Kibana that will need to be updated (if that amount is "none", include that information as well)
  • 🔍 Tip: When searching through Kibana, consider excluding **/target, **/*.snap, **/*.storyshot files to reduce noise and only look at source code usages
  • ⚠️ For extremely risky changes, the EUI team should potentially consider the following precautions:
    • Using a pre-release release candidate to test Kibana CI ahead of time
    • Using kibana-a-la-carte for manual QA, and to give other Kibana teams a staging server to quickly test against

@cee-chen
Copy link
Contributor Author

cee-chen commented Jun 7, 2024

Breaking changes review:

  • ✅ No usages of .euiCheckbox--compressed or .euiRadio--compressed in Kibana or cloud
  • ✅ No custom CSS that would be affected by margin to flex gap CSS change on .euiCheckboxGroup__item or .euiRadioGroup__item
    • ⚠️❓ 2 usages of .euiRadioGroup__item in Cloud that will need to be manually reviewed for visual regressions (should be fine, but it's unclear depending on their usage. Would have also broken in the Emotion conversion in any case)
  • 🔜 API changes - I didn't bother trying to regex for this as Typescript will catch compressed usages for us, and it's a very easy deletion with no visual impact if so

@cee-chen cee-chen marked this pull request as ready for review June 7, 2024 18:27
@cee-chen cee-chen requested a review from a team as a code owner June 7, 2024 18:27
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

mgadewoll
mgadewoll approved these changes Jun 10, 2024
View reviewed changes
Copy link
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 🐈‍⬛ It's definitely cleaner and clearer now to have compressed only on the groups, great call! 👍 The changes and output look good to me!

tkajtoch
tkajtoch approved these changes Jun 10, 2024
View reviewed changes
Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice cleanup 💯

@cee-chen
Copy link
Contributor Author

Thanks y'all! Tomasz, please do shout if you encounter any shenanigans during the upgrade with this!

@cee-chen cee-chen merged commit 7fac1ce into elastic:main Jun 10, 2024
8 checks passed
@cee-chen cee-chen deleted the checkbox-radio-compressed branch June 10, 2024 15:36
@tkajtoch tkajtoch mentioned this pull request Jun 11, 2024
Merged
cee-chen added a commit to elastic/kibana that referenced this pull request Jun 18, 2024
@tkajtoch @cee-chen @kibanamachine
Upgrade EUI to v95.0.0 (#185943)
946a255 
`v94.6.0` ⏩ `v95.0.0-backport.0`

_[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_

---

##
[`v95.0.0-backport.0`](https://github.com/elastic/eui/releases/v95.0.0-backport.0)

**This is a backport release only intended for use by Kibana.**

- Updated `EuiSteps` to support a new `titleSize="xxs"` style, which
outputs the same title font size but smaller unnumbered step indicators
([#7813](elastic/eui#7813))
- Updated `EuiStepsHorizontal` to support a new `size="xs"` style, which
outputs smaller unnumbered step indicators
([#7813](elastic/eui#7813))
- Updated `EuiStepNumber` to support new `titleSize="none"` which omits
rendering step numbers, and will only render icons
([#7813](elastic/eui#7813))

## [`v95.0.0`](https://github.com/elastic/eui/releases/v95.0.0)

- Added `move` glyph to `EuiIcon`
([#7789](elastic/eui#7789))
- Updated `EuiBasicTable` and `EuiInMemoryTable`s with `selection` - the
header row checkbox will now render an indeterminate state if some (but
not all) rows are selected
([#7817](elastic/eui#7817))

**Bug fixes**

- Fixed an `EuiDataGrid` visual bug when using `lineCount` row heights
where the clamped text was still visible for some font sizes
([#7793](elastic/eui#7793))
- Fixed `EuiSearchBar`'s filter configs to always respect `autoClose:
false` ([#7806](elastic/eui#7806))

**Breaking changes**

- Removed deprecated `EUI_CHARTS_THEME_DARK`, `EUI_CHARTS_THEME_LIGHT`
and `EUI_SPARKLINE_THEME_PARTIAL` exports
([#7682](elastic/eui#7682))
- Removed deprecated `euiPalettePositive` and `euiPaletteNegative`. Use
`euiPaletteGreen` and `euiPaletteRed` instead
([#7808](elastic/eui#7808))
- Removed `type="inList"` from `EuiCheckbox`. Simply omit passing a
`label` prop to render this style of checkbox
([#7814](elastic/eui#7814))
- Removed the unused `compressed` prop from `EuiCheckbox` and
`EuiRadio`. This prop was not doing anything on individual components.
([#7818](elastic/eui#7818))

**CSS-in-JS conversions**

- Converted `EuiCheckboxGroup` to Emotion
([#7818](elastic/eui#7818))
- Converted `EuiRadioGroup` to Emotion
([#7818](elastic/eui#7818))

---------

Co-authored-by: Cee Chen <constance.chen@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>
bhapas pushed a commit to bhapas/kibana that referenced this pull request Jun 18, 2024
@tkajtoch @cee-chen
@kibanamachine @bhapas
Upgrade EUI to v95.0.0 (elastic#185943)
49030bf 
`v94.6.0` ⏩ `v95.0.0-backport.0`

_[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_

---

##
[`v95.0.0-backport.0`](https://github.com/elastic/eui/releases/v95.0.0-backport.0)

**This is a backport release only intended for use by Kibana.**

- Updated `EuiSteps` to support a new `titleSize="xxs"` style, which
outputs the same title font size but smaller unnumbered step indicators
([elastic#7813](elastic/eui#7813))
- Updated `EuiStepsHorizontal` to support a new `size="xs"` style, which
outputs smaller unnumbered step indicators
([elastic#7813](elastic/eui#7813))
- Updated `EuiStepNumber` to support new `titleSize="none"` which omits
rendering step numbers, and will only render icons
([elastic#7813](elastic/eui#7813))

## [`v95.0.0`](https://github.com/elastic/eui/releases/v95.0.0)

- Added `move` glyph to `EuiIcon`
([elastic#7789](elastic/eui#7789))
- Updated `EuiBasicTable` and `EuiInMemoryTable`s with `selection` - the
header row checkbox will now render an indeterminate state if some (but
not all) rows are selected
([elastic#7817](elastic/eui#7817))

**Bug fixes**

- Fixed an `EuiDataGrid` visual bug when using `lineCount` row heights
where the clamped text was still visible for some font sizes
([elastic#7793](elastic/eui#7793))
- Fixed `EuiSearchBar`'s filter configs to always respect `autoClose:
false` ([elastic#7806](elastic/eui#7806))

**Breaking changes**

- Removed deprecated `EUI_CHARTS_THEME_DARK`, `EUI_CHARTS_THEME_LIGHT`
and `EUI_SPARKLINE_THEME_PARTIAL` exports
([elastic#7682](elastic/eui#7682))
- Removed deprecated `euiPalettePositive` and `euiPaletteNegative`. Use
`euiPaletteGreen` and `euiPaletteRed` instead
([elastic#7808](elastic/eui#7808))
- Removed `type="inList"` from `EuiCheckbox`. Simply omit passing a
`label` prop to render this style of checkbox
([elastic#7814](elastic/eui#7814))
- Removed the unused `compressed` prop from `EuiCheckbox` and
`EuiRadio`. This prop was not doing anything on individual components.
([elastic#7818](elastic/eui#7818))

**CSS-in-JS conversions**

- Converted `EuiCheckboxGroup` to Emotion
([elastic#7818](elastic/eui#7818))
- Converted `EuiRadioGroup` to Emotion
([elastic#7818](elastic/eui#7818))

---------

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

@mgadewoll mgadewoll mgadewoll approved these changes

@tkajtoch tkajtoch tkajtoch approved these changes

Assignees
No one assigned
Labels
breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cee-chen @kibanamachine @elasticmachine @tkajtoch @mgadewoll