[EuiTablePagination] Allow page size props to be configured via `EuiProvider.componentProps` #6951

cee-chen · 2023-07-15T01:53:11Z

Summary

This PR is part 1 of 2 of the work required to allow all EUI tables and grids to have their pagination page size configurable by consumer defaults (see elastic/kibana#56406 for more context).

The previous components configured to use the new defaults context were less complex to set up because they were undefined by default; this is the first component we're setting up where we want to fall back to an actual value.

The remaining work after this involves updating EuiBasicTable/EuiInMemoryTable/EuiDataGrid themselves to use the new default context, which you can preview here if you're super gung ho, but the diff was starting to get really large and unwieldy and I wanted to discuss some of the more general architectural choices without it muddying the waters.

I recommend following along by commit, as this PR contains a good amount of extra RTL test conversions/cleanup.

QA

General checklist

Props have proper autodocs (using @default if default values are missing) and playground toggles
Added or updated jest ~~and cypress tests~~
A changelog entry exists and is marked appropriately

- we're going to need this extra specificity shortly for when we add internal context defaults

- I played around with it, but unfortunately it doesn't makse sense to define the defaults in the component itself still. You either run into dependency issues (if you try to import them) or you have a lot of extra `??` repeated code if you try to make an extra layer of fallbacks

…ults

- convert tests to RTL - remove unnecessary `() => {}` callback default (since prop is not required), use optional chaining instead

- default props must be manually declared using jsdocs now, since react docgen can no longer use `=` fallbacks to figure them out - note: setting `EuiTablePagination.defaultProps` unfortunately does not work, React will automatically populate them and they don't come in as undefined

- PaginationBar should not be testing props that are basic pass-throughs - EuiTablePagination should (the whole point of unit tests) + add test for PaginationBar logic that's actually there (onPageChange) - Move said missing tests to EuiTablePagination and improve tests to use specific assertions instead of noisy snapshots

cee-chen · 2023-07-15T02:16:37Z

src/components/table/table_pagination/table_pagination.tsx

+    onChangePage,
+    pageCount,
+    ...rest
+  } = props;


Just want to quickly note that moving the = default parameters out of the initial function component means that our props table docgen can no longer automatically infer the default values, and we need to use manual jsdoc @default comments in our props documentation now like so:

eui/src/components/table/table_pagination/table_pagination.tsx

Lines 40 to 47 in 37ff1c3

* @default 50

*/

itemsPerPage?: number;

/**

* Custom array of options for "Rows per page".

* Pass `0` as one of the options to create a "Show all" option.

*

* @default [10, 20, 50, 100]

I'm not sure there's any way around this without coding up our own plugin or logic into our props doc generator - which I totally think is worth doing, but we should file that as a separate enhancement / a nice to have for another day. In the meanwhile, I think a little less DRYness for the sake of better docs/developer experience is worth it for now.

Quick note to self: prop defaults docgen will remain as yet unresolved in this PR/feature branch

kibanamachine · 2023-07-15T02:22:23Z

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

src/components/provider/component_defaults/component_defaults.tsx

src/components/table/table_pagination/table_pagination.tsx

src/components/provider/component_defaults/component_defaults.tsx

1Copenut

This approach feels right on the money to me. I expanded my thoughts in thread.

Will change this to approved after other folks have had a chance to weigh in.

src/components/provider/component_defaults/component_defaults.tsx

kibanamachine · 2023-07-20T17:38:33Z

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

kibanamachine · 2023-07-24T21:11:17Z

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

- to make it (maybe??) a bit clearer that component defaults in this context are 100% from the consumer and 0% from EUI at this point

via the magic of an inferred generic

kibanamachine · 2023-07-25T04:33:14Z

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

cee-chen · 2023-07-25T04:56:34Z

@tkajtoch Alrighty, I think this is ready for a final review if you don't mind taking another pass. Apologies for the more chaotic git history than normal, since we ended up going a bit back and forth on architecture 😅

I made some minor naming/copy tweaks, but the major change/cleanup item other than writing tests was I was able to move away from manually typing passed props somehow! 28523ea ✨

cee-chen · 2023-07-25T04:59:28Z

src/components/table/table_pagination/table_pagination.tsx

+    itemsPerPage = 50,
+    itemsPerPageOptions = [10, 20, 50, 100],
+    showPerPageOptions = true,


One quick note on this: I'm going to end up moving these defaults to a separate euiTablePaginationDefaults const/file in the next PR, but this is only because said defaults will end up being reused/referenced by EUI tables/datagrids, which is going to be a fairly unusual use-case for most components.

I would anticipate 99% of components w/ defaults to set them like the above, hence why I kept this as-is for this PR.

kibanamachine · 2023-07-25T05:27:54Z

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

tkajtoch

So excited about this one! The changes are looking great

…rovider.componentProps` (elastic#6951) Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

…rovider.componentProps` (#6951) Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

…rovider.componentProps` (elastic#6951) Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

`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 <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Patryk Kopyciński <contact@patrykkopycinski.com>

[setup] Misc EuiPortal cleanup

c4bd1b1

- we're going to need this extra specificity shortly for when we add internal context defaults

cee-chen changed the title ~~[EuiTablePagination] Set up inherited EUI defaults~~ [EuiTablePagination] Allow itemsPerPage, itemsPerPageOptions, and showPerPageOptions to be configurabled via EuiProvider.componentProps Jul 15, 2023

cee-chen changed the title ~~[EuiTablePagination] Allow itemsPerPage, itemsPerPageOptions, and showPerPageOptions to be configurabled via EuiProvider.componentProps~~ [EuiTablePagination] Allow page size props to be configurabled via EuiProvider.componentProps Jul 15, 2023

cee-chen added 5 commits July 14, 2023 18:58

Add logic for merging/overriding consumer defaults on top of EUI defa…

b21664a

…ults

[setup] Misc EuiTablePaginationCleanup

b1fda8c

- convert tests to RTL - remove unnecessary `() => {}` callback default (since prop is not required), use optional chaining instead

cee-chen force-pushed the provider/table-pagination-1 branch from 23065c7 to 37ff1c3 Compare July 15, 2023 01:59

cee-chen changed the title ~~[EuiTablePagination] Allow page size props to be configurabled via EuiProvider.componentProps~~ [EuiTablePagination] Allow page size props to be configured via EuiProvider.componentProps Jul 15, 2023

cee-chen requested review from a team and tkajtoch July 15, 2023 02:12

cee-chen marked this pull request as ready for review July 15, 2023 02:12

cee-chen commented Jul 15, 2023

View reviewed changes

src/components/provider/component_defaults/component_defaults.tsx Outdated Show resolved Hide resolved

cee-chen commented Jul 15, 2023

View reviewed changes

src/components/table/table_pagination/table_pagination.tsx Outdated Show resolved Hide resolved

cee-chen commented Jul 15, 2023

View reviewed changes

src/components/provider/component_defaults/component_defaults.tsx Outdated Show resolved Hide resolved

1Copenut reviewed Jul 17, 2023

View reviewed changes

src/components/provider/component_defaults/component_defaults.tsx Outdated Show resolved Hide resolved

src/components/provider/component_defaults/component_defaults.tsx Outdated Show resolved Hide resolved

feat: add usePropsWithComponentDefaults

8695bb3

cee-chen added 5 commits July 24, 2023 21:09

[PR feedback feedback] Prop naming tweaks, order

8dcf924

Revert EUI_COMPONENT_DEFAULTS const/merging logic

3a44a79

Rename useEuiComponentDefaults to just useComponentDefaults

4f8a621

- to make it (maybe??) a bit clearer that component defaults in this context are 100% from the consumer and 0% from EUI at this point

Add tests for usePropsWithComponentDefaults

4f5f225

Remove need for EuiComponentDefaultsSupportedComponents

28523ea

via the magic of an inferred generic

cee-chen force-pushed the provider/table-pagination-1 branch from 0e45ed7 to 28523ea Compare July 25, 2023 04:09

cee-chen commented Jul 25, 2023

View reviewed changes

changelog

f303f96

tkajtoch approved these changes Jul 25, 2023

View reviewed changes

cee-chen merged commit b46e557 into elastic:feature/provider/component-defaults Jul 25, 2023

cee-chen deleted the provider/table-pagination-1 branch July 25, 2023 20:56

cee-chen added a commit to cee-chen/eui that referenced this pull request Jul 25, 2023

[EuiTablePagination] Allow page size props to be configured via `EuiP…

e76c8da

…rovider.componentProps` (elastic#6951) Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

cee-chen added a commit to cee-chen/eui that referenced this pull request Jul 25, 2023

[EuiTablePagination] Allow page size props to be configured via `EuiP…

1a3a35e

…rovider.componentProps` (elastic#6951) Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

cee-chen added a commit to cee-chen/eui that referenced this pull request Jul 26, 2023

[EuiTablePagination] Allow page size props to be configured via `EuiP…

92f1d8b

…rovider.componentProps` (elastic#6951) Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

cee-chen added a commit that referenced this pull request Aug 1, 2023

[EuiTablePagination] Allow page size props to be configured via `EuiP…

839b68d

…rovider.componentProps` (#6951) Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

cee-chen added a commit to cee-chen/eui that referenced this pull request Aug 1, 2023

[EuiTablePagination] Allow page size props to be configured via `EuiP…

60f85ec

…rovider.componentProps` (elastic#6951) Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

cee-chen mentioned this pull request Aug 3, 2023

[EuiProvider] Add new componentDefaults configuration #6995

Merged

cee-chen added a commit to cee-chen/eui that referenced this pull request Aug 3, 2023

[EuiTablePagination] Allow page size props to be configured via `EuiP…

c83abe1

…rovider.componentProps` (elastic#6951) Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>

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

[EuiTablePagination] Allow page size props to be configured via `EuiProvider.componentProps` #6951

[EuiTablePagination] Allow page size props to be configured via `EuiProvider.componentProps` #6951

cee-chen commented Jul 15, 2023 •

edited

Loading

cee-chen Jul 15, 2023

cee-chen Jul 25, 2023 •

edited

Loading

kibanamachine commented Jul 15, 2023

1Copenut left a comment

kibanamachine commented Jul 20, 2023

kibanamachine commented Jul 24, 2023

kibanamachine commented Jul 25, 2023

cee-chen commented Jul 25, 2023 •

edited

Loading

cee-chen Jul 25, 2023

kibanamachine commented Jul 25, 2023

tkajtoch left a comment

	* @default 50
	*/
	itemsPerPage?: number;
	/**
	* Custom array of options for "Rows per page".
	* Pass `0` as one of the options to create a "Show all" option.
	*
	* @default [10, 20, 50, 100]

[EuiTablePagination] Allow page size props to be configured via EuiProvider.componentProps #6951

[EuiTablePagination] Allow page size props to be configured via EuiProvider.componentProps #6951

Conversation

cee-chen commented Jul 15, 2023 • edited Loading

Summary

QA

General checklist

cee-chen Jul 15, 2023

Choose a reason for hiding this comment

cee-chen Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

kibanamachine commented Jul 15, 2023

1Copenut left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 20, 2023

kibanamachine commented Jul 24, 2023

kibanamachine commented Jul 25, 2023

cee-chen commented Jul 25, 2023 • edited Loading

cee-chen Jul 25, 2023

Choose a reason for hiding this comment

kibanamachine commented Jul 25, 2023

tkajtoch left a comment

Choose a reason for hiding this comment

[EuiTablePagination] Allow page size props to be configured via `EuiProvider.componentProps` #6951

[EuiTablePagination] Allow page size props to be configured via `EuiProvider.componentProps` #6951

cee-chen commented Jul 15, 2023 •

edited

Loading

cee-chen Jul 25, 2023 •

edited

Loading

cee-chen commented Jul 25, 2023 •

edited

Loading