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.

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

hotfix: correct EuiFlexGroup and EuiFlexItem ref prop type and other small adjustments #7724

Merged
merged 6 commits into from
May 7, 2024

Conversation

tkajtoch
Copy link
Member

@tkajtoch tkajtoch commented May 1, 2024

Summary

This PR fixes ref typing bug in the recently merged #7612, adds CommonProps and PropsWithChildren types to EuiFlexGroup props to get it aligned with EuiFlexItem (it's unnecessary after the fixes added to kibana EUI upgrade PR but I kept it just in case), and allows defining displayName on the component with no type casts necessary.

QA

  • Test that EuiFlexGroup and EuiFlexItem ref prop doesn't raise any type errors
    const Test = () => {
      const spanRef = useRef<HTMLSpanElement>();
      return <EuiFlexGroup component="span" ref={spanRef}>asd</EuiFlexGroup>;
    };

General checklist

  • Release checklist
    • A changelog entry exists and is marked appropriately.

@tkajtoch tkajtoch marked this pull request as ready for review May 1, 2024 13:31
@tkajtoch tkajtoch requested a review from a team as a code owner May 1, 2024 13:31
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

Changes LGTM. I followed your suggested QA pattern by puling your branch and adding the const Test = () => Component block into the docs pages and running yarn lint. I then reverted to main with the changes and ran yarn lint again.

The linting failed both times on main and passed both times on your feature branch.

@tkajtoch tkajtoch merged commit 88832eb into elastic:main May 7, 2024
7 checks passed
jbudz pushed a commit to elastic/kibana that referenced this pull request May 10, 2024
`v94.2.1-backport.0` ⏩ `v94.3.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)_

---

## [`v94.3.0`](https://github.com/elastic/eui/releases/v94.3.0)

- Updated `launch` glyph for `EuiIcon`
([#7670](elastic/eui#7670))
- Updated `EuiComboBox`'s `options` to support including tooltip details
for selectable options. Use `toolTipContent` to render tooltip
information, and `toolTipProps` to optionally customize the tooltip
rendering behavior ([#7700](elastic/eui#7700))
- Updated the following existing glyphs in `EuiIcon`:
([#7727](elastic/eui#7727))
  - `error` (now an outlined version instead of filled) 
  - `tokenMetricCounter`
  - `tokenMetricGauge` 
- Added the following new glyphs to `EuiIcon`:
([#7727](elastic/eui#7727))
  - `tokenDimension`
  - `clickLeft`
  - `clickRight`
  - `clockCounter`
  - `errorFilled` (the previous `error` glyph design)
  - `warningFilled`

**Bug fixes**

- Fixed a visual layout bug for `EuiComboBox` with `isLoading` in mobile
views ([#7700](elastic/eui#7700))
- Fixed missing styles on header cells of `EuiDataGrid` that prevented
content text alignment styles to apply
([#7720](elastic/eui#7720))
- Fixed `EuiFlexGroup` and `EuiFlexItem` `ref` prop typing to support
refs of the same type as the passed `component` type and allow
`displayName` to be defined for easy component naming when using
component wrappers like `styled()`
([#7724](elastic/eui#7724))

---

Most of the code changes you'll see in this PR are caused by the recent
EuiFlex* changes making it generic. This, unfortunately, is something
that `styled()` doesn't always like. I replaced the failing usages of
`styled(EuiFlexGroup)` and `styled(EuiFlexItem)` to use `component` and
other native EuiFlex* props, resulting in the same output but being
better typed.

We plan to add more props to EuiFlex* components giving developers
control over properties like `flex-grow` and `flex-shring`, and reducing
the need for writing any custom CSS when using these components. This
should reduce the number of `styled()` wrappers needed even further

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants