[EuiListGroupItem] Adding a `toolTipProps` spread operator to allow tooltip customization #7018

1Copenut · 2023-08-01T13:37:07Z

Summary

@Heenawter brought us an excellent feature request to spread toolTipProps into EuiListGroupItem for more customization. This will allow consumers more control over where tooltips appear and how they're styled.

PR closes #7000.

QA

All QA completed. CodeSandbox isn't showing the correct version yet. Thinking that'll change when the code is merged and published.

General checklist

kibanamachine · 2023-08-01T14:06:27Z

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

kibanamachine · 2023-08-01T14:10:44Z

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

Heenawter

This looks great - this should do exactly what we need!! Thanks so much for tackling this so quickly 🎉

src/components/list_group/list_group_item.tsx

src/components/list_group/list_group.test.tsx

src/components/list_group/list_group_item.tsx

src/components/list_group/list_group.test.tsx

src-docs/src/views/list_group/list_group_example.js

src-docs/src/views/list_group/list_group_extra.tsx

cee-chen · 2023-08-01T17:32:40Z

src/components/list_group/list_group_item.tsx

@@ -144,6 +144,11 @@ export type EuiListGroupItemProps = CommonProps &
     * By default the text will be same as the label text.
     */
    toolTipText?: string;


[not a change request, just me thinking out loud] I wonder if we should consider deprecating this prop in favor of telling consumers to use toolTipProps.content 🤔 Probably not since this is a nicer API, but wanted to throw that out there

I lean toward keeping it. Agree with you it's a nice API and saves a few keystrokes if folks want to just add a custom string the tooltip than string plus N number more change.

kibanamachine · 2023-08-02T14:11:50Z

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

kibanamachine · 2023-08-02T14:13:00Z

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

src/components/list_group/list_group_item.test.tsx

upcoming_changelogs/7018.md

src/components/list_group/list_group_item.tsx

src/components/list_group/list_group.test.tsx

src/components/list_group/list_group_item.test.tsx

Co-authored-by: Cee Chen <[email protected]>

kibanamachine · 2023-08-04T18:42:26Z

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

cee-chen

🚢

kibanamachine · 2023-08-04T18:46:15Z

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

kibanamachine · 2023-08-04T19:05:09Z

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

kibanamachine · 2023-08-04T19:06:30Z

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

cee-chen · 2023-08-04T19:16:53Z

@1Copenut it looks like the type check is now failing, which is honestly bizarre (EuiToolTip 100% accepts data-test-subj as a top level prop with no errors). But on the bright side, it's a good thing we added this test - if a consumer had wanted to pass a custom data-test-subj they would have run into the same issues as well 🤔

Can you try modifying EuiToolTipProps to extend HTMLAttributes<HTMLDivElement> & CommonProps? I think that should handle the type error 🤞

1Copenut · 2023-08-04T19:21:41Z

Can you try modifying EuiToolTipProps to extend HTMLAttributes<HTMLDivElement> & CommonProps? I think that should handle the type error 🤞

I used CommonProps to smooth out the TS error. The linting passed locally, so should run correct on CI now. :)

Will merge when it passes.

kibanamachine · 2023-08-04T19:43:35Z

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

kibanamachine · 2023-08-04T19:45:48Z

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

kibanamachine · 2023-08-04T20:51:52Z

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

kibanamachine · 2023-08-04T21:00:10Z

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

elasticmachine · 2023-08-04T21:06:13Z

💚 Build Succeeded

Buildkite Build
Commit: 15c4c7d

History

💚 Build #211 succeeded 0c98496
💔 Build #209 failed 3f9b318
💚 Build #164 succeeded e426c3a
💚 Build #139 succeeded 35aed30

cc @1Copenut

cee-chen · 2023-08-04T23:19:34Z

I used CommonProps to smooth out the TS error.

That's probably fine for now, but it might (still?) be too restrictive in the long run. HTMLAttributes<> will allow any valid attr that an element takes. So, e.g. if some consumer has their own custom data- attr, or they want to pass a custom aria attribute, or the hidden attribute, etc etc - those are all valid attributes to pass to the tooltip, but would cause annoying type errors if our typing is too restrictive.

Anyway, I suppose that's a problem for future us :) The props table looks fine and is correctly rendering the new common props, so I'm going to go ahead and approve/merge

`v86.0.0`⏩`v87.1.0` ⚠️ The biggest set of type changes in this PR come from the breaking change that makes `pageSize` and `pageSizeOptions` now optional props for `EuiBasicTable.pagination`, `EuiInMemoryTable.pagination` and `EuiDataGrid.pagination`. This caused several other components that were cloning EUI's pagination type to start throwing type warnings about `pageSize` being optional. Where I came across these errors, I modified the extended types to require `pageSize`. These types and their usages may end up changing again in any case once the Shared UX team looks into #56406. --- ## [`87.1.0`](https://github.com/elastic/eui/tree/v87.1.0) - Updated the underlying library powering `EuiAutoSizer`. This primarily affects typing around the `disableHeight` and `disableWidth` props ([#6798](elastic/eui#6798)) - Added new `EuiAutoSize`, `EuiAutoSizeHorizontal`, and `EuiAutoSizeVertical` types to support `EuiAutoSizer`'s now-stricter typing ([#6798](elastic/eui#6798)) - Updated `EuiDatePickerRange` to support `compressed` display ([#7058](elastic/eui#7058)) - Updated `EuiFlyoutBody` with a new `scrollableTabIndex` prop ([#7061](elastic/eui#7061)) - Added a new `panelMinWidth` prop to `EuiInputPopover` ([#7071](elastic/eui#7071)) - Added a new `inputPopoverProps` prop for `EuiRange`s and `EuiDualRange`s with `showInput="inputWithPopover"` set ([#7082](elastic/eui#7082)) **Bug fixes** - Fixed `EuiToolTip` overriding instead of merging its `aria-describedby` tooltip ID with any existing `aria-describedby`s ([#7055](elastic/eui#7055)) - Fixed `EuiSuperDatePicker`'s `compressed` display ([#7058](elastic/eui#7058)) - Fixed `EuiAccordion` to remove tabbable children from sequential keyboard navigation when the accordion is closed ([#7064](elastic/eui#7064)) - Fixed `EuiFlyout`s to accept custom `aria-describedby` IDs ([#7065](elastic/eui#7065)) **Accessibility** - Removed the default `dialog` role and `tabIndex` from push `EuiFlyout`s. Push flyouts, compared to overlay flyouts, require manual accessibility management. ([#7065](elastic/eui#7065)) ## [`87.0.0`](https://github.com/elastic/eui/tree/v87.0.0) - Added beta `componentDefaults` prop to `EuiProvider`, which will allow configuring certain default props globally. This list of components and defaults is still under consideration. ([#6923](elastic/eui#6923)) - `EuiPortal`'s `insert` prop can now be configured globally via `EuiProvider.componentDefaults` ([#6941](elastic/eui#6941)) - `EuiFocusTrap`'s `crossFrame` and `gapMode` props can now be configured globally via `EuiProvider.componentDefaults` ([#6942](elastic/eui#6942)) - `EuiTablePagination`'s `itemsPerPage`, `itemsPerPageOptions`, and `showPerPageOptions` props can now be configured globally via `EuiProvider.componentDefaults` ([#6951](elastic/eui#6951)) - `EuiBasicTable`, `EuiInMemoryTable`, and `EuiDataGrid` now allow `pagination.pageSize` to be undefined. If undefined, `pageSize` defaults to `EuiTablePagination`'s `itemsPerPage` component default. ([#6993](elastic/eui#6993)) - `EuiBasicTable`, `EuiInMemoryTable`, and `EuiDataGrid`'s `pagination.pageSizeOptions` will now fall back to `EuiTablePagination`'s `itemsPerPageOptions` component default. ([#6993](elastic/eui#6993)) - Updated `EuiHeaderLinks`'s `gutterSize` spacings ([#7005](elastic/eui#7005)) - Updated `EuiHeaderAlert`'s stacking styles ([#7005](elastic/eui#7005)) - Added `toolTipProps` to `EuiListGroupItem` that allows customizing item tooltips. ([#7018](elastic/eui#7018)) - Updated `EuiBreadcrumbs` to support breadcrumbs that toggle popovers via `popoverContent` and `popoverProps` ([#7031](elastic/eui#7031)) - Improved the contrast ratio of disabled titles within `EuiSteps` and `EuiStepsHorizontal` to meet WCAG AA guidelines. ([#7032](elastic/eui#7032)) - Updated `EuiSteps` and `EuiStepsHorizontal` to highlight and provide a more clear visual indication of the current step ([#7048](elastic/eui#7048)) **Bug fixes** - Single uses of `<EuiHeaderSectionItem side="right" />` now align right as expected without needing a previous `side="left"` sibling. ([#7005](elastic/eui#7005)) - `EuiPageTemplate` now correctly displays `panelled={true}` ([#7044](elastic/eui#7044)) **Breaking changes** - `EuiTablePagination`'s default `itemsPerPage` is now `10` (was previously `50`). This can be configured through `EuiProvider.componentDefaults`. ([#6993](elastic/eui#6993)) - `EuiTablePagination`'s default `itemsPerPageOptions` is now `[10, 25, 50]` (was previously `[10, 20, 50, 100]`). This can be configured through `EuiProvider.componentDefaults`. ([#6993](elastic/eui#6993)) - Removed `border` prop from `EuiHeaderSectionItem` (unused since Amsterdam theme) ([#7005](elastic/eui#7005)) - Removed `borders` object configuration from `EuiHeader.sections` ([#7005](elastic/eui#7005)) **CSS-in-JS conversions** - Converted `EuiHeaderAlert` to Emotion; Removed unused `.euiHeaderAlert__dismiss` CSS ([#7005](elastic/eui#7005)) - Converted `EuiHeaderSection`, `EuiHeaderSectionItem`, and `EuiHeaderSectionItemButton` to Emotion ([#7005](elastic/eui#7005)) - Converted `EuiHeaderLinks` and `EuiHeaderLink` to Emotion; Removed `$euiHeaderLinksGutterSizes` Sass variables ([#7005](elastic/eui#7005)) - Removed `$euiHeaderBackgroundColor` Sass variable; use `$euiColorEmptyShade` instead ([#7005](elastic/eui#7005)) - Removed `$euiHeaderChildSize` Sass variable; use `$euiSizeXXL` instead ([#7005](elastic/eui#7005)) --------- Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Patryk Kopyciński <[email protected]>

Spreading toolTipProps into EuiListGroupItem for more customization.

9b5d2ca

1Copenut added the feature request label Aug 1, 2023

1Copenut requested review from cee-chen and Heenawter August 1, 2023 13:37

1Copenut self-assigned this Aug 1, 2023

Adding CHANGELOG for iPR 7018.

35aed30

1Copenut marked this pull request as ready for review August 1, 2023 14:20

Heenawter approved these changes Aug 1, 2023

View reviewed changes

cee-chen suggested changes Aug 1, 2023

View reviewed changes

1Copenut added 2 commits August 1, 2023 16:20

Merge branch 'main' into feature/7000-euiListItem-propSpread

73fbd9a

Fixed className to point to child anchor class(es).

e426c3a

1Copenut requested a review from cee-chen August 2, 2023 13:56

cee-chen reviewed Aug 2, 2023

View reviewed changes

src/components/list_group/list_group_item.test.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Aug 2, 2023

View reviewed changes

upcoming_changelogs/7018.md Outdated Show resolved Hide resolved

cee-chen reviewed Aug 2, 2023

View reviewed changes

src/components/list_group/list_group_item.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Aug 2, 2023

View reviewed changes

src/components/list_group/list_group.test.tsx Outdated Show resolved Hide resolved

1Copenut added 2 commits August 4, 2023 12:26

Merge branch 'main' into feature/7000-euiListItem-propSpread

ba277cd

Refactoring tests to include a check for toolTipProps.

9c741d7

1Copenut requested a review from cee-chen August 4, 2023 18:31

cee-chen reviewed Aug 4, 2023

View reviewed changes

src/components/list_group/list_group_item.test.tsx Outdated Show resolved Hide resolved

cee-chen reviewed Aug 4, 2023

View reviewed changes

src/components/list_group/list_group_item.test.tsx Outdated Show resolved Hide resolved

1Copenut and others added 2 commits August 4, 2023 13:42

Update src/components/list_group/list_group_item.test.tsx

1c9d522

Co-authored-by: Cee Chen <[email protected]>

Update src/components/list_group/list_group_item.test.tsx

3f9b318

Co-authored-by: Cee Chen <[email protected]>

cee-chen approved these changes Aug 4, 2023

View reviewed changes

1Copenut enabled auto-merge (squash) August 4, 2023 18:47

Added data-test-subj to toolTipProps.

0c98496

1Copenut disabled auto-merge August 4, 2023 20:05

Making the EuiToolTipProps simpler extensions of CommonProps.

15c4c7d

1Copenut enabled auto-merge (squash) August 4, 2023 20:33

1Copenut disabled auto-merge August 4, 2023 21:00

cee-chen approved these changes Aug 4, 2023

View reviewed changes

cee-chen merged commit 304f35c into elastic:main Aug 4, 2023

cee-chen mentioned this pull request Aug 15, 2023

Upgrade EUI to v87.1.0 elastic/kibana#163961

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiListGroupItem] Adding a `toolTipProps` spread operator to allow tooltip customization #7018

[EuiListGroupItem] Adding a `toolTipProps` spread operator to allow tooltip customization #7018

1Copenut commented Aug 1, 2023 •

edited

Loading

kibanamachine commented Aug 1, 2023

kibanamachine commented Aug 1, 2023

Heenawter left a comment

cee-chen Aug 1, 2023

1Copenut Aug 2, 2023

kibanamachine commented Aug 2, 2023

kibanamachine commented Aug 2, 2023

kibanamachine commented Aug 4, 2023

cee-chen left a comment

kibanamachine commented Aug 4, 2023

kibanamachine commented Aug 4, 2023

kibanamachine commented Aug 4, 2023

cee-chen commented Aug 4, 2023

1Copenut commented Aug 4, 2023 •

edited

Loading

kibanamachine commented Aug 4, 2023

kibanamachine commented Aug 4, 2023

kibanamachine commented Aug 4, 2023

kibanamachine commented Aug 4, 2023

elasticmachine commented Aug 4, 2023

cee-chen commented Aug 4, 2023 •

edited

Loading

[EuiListGroupItem] Adding a toolTipProps spread operator to allow tooltip customization #7018

[EuiListGroupItem] Adding a toolTipProps spread operator to allow tooltip customization #7018

Conversation

1Copenut commented Aug 1, 2023 • edited Loading

Summary

QA

General checklist

kibanamachine commented Aug 1, 2023

kibanamachine commented Aug 1, 2023

Heenawter left a comment

Choose a reason for hiding this comment

cee-chen Aug 1, 2023

Choose a reason for hiding this comment

1Copenut Aug 2, 2023

Choose a reason for hiding this comment

kibanamachine commented Aug 2, 2023

kibanamachine commented Aug 2, 2023

kibanamachine commented Aug 4, 2023

cee-chen left a comment

Choose a reason for hiding this comment

kibanamachine commented Aug 4, 2023

kibanamachine commented Aug 4, 2023

kibanamachine commented Aug 4, 2023

cee-chen commented Aug 4, 2023

1Copenut commented Aug 4, 2023 • edited Loading

kibanamachine commented Aug 4, 2023

kibanamachine commented Aug 4, 2023

kibanamachine commented Aug 4, 2023

kibanamachine commented Aug 4, 2023

elasticmachine commented Aug 4, 2023

💚 Build Succeeded

History

cee-chen commented Aug 4, 2023 • edited Loading

[EuiListGroupItem] Adding a `toolTipProps` spread operator to allow tooltip customization #7018

[EuiListGroupItem] Adding a `toolTipProps` spread operator to allow tooltip customization #7018

1Copenut commented Aug 1, 2023 •

edited

Loading

1Copenut commented Aug 4, 2023 •

edited

Loading

cee-chen commented Aug 4, 2023 •

edited

Loading