[EuiBasicTable][EuiInMemoryTable][EuiDataGrid] Update to use EuiTablePagination component defaults#6993
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993/ |
1 similar comment
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993_buildkite/ |
ebd3bb3 to
4d2d7fa
Compare
e76c8da to
1a3a35e
Compare
4d2d7fa to
0a18b85
Compare
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993_buildkite/ |
1 similar comment
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993_buildkite/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993/ |
8c72ddd to
8b89730
Compare
1a3a35e to
92f1d8b
Compare
8b89730 to
f514d30
Compare
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993_buildkite/ |
2 similar comments
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993_buildkite/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993_buildkite/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993/ |
f514d30 to
6a0202e
Compare
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993_buildkite/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993/ |
|
@tkajtoch Sending this PR your way since you already have some context from the previous first part of this work/PR, but please let me know if you're busy and I can reassign to another dev! |
92f1d8b to
839b68d
Compare
6a0202e to
6f9c4e7
Compare
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993_buildkite/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993/ |
- will be required/used by tables & grids shortly - hook is an extra syntactical sugar bonus that *should* only be needed by this specific table pagination use case
… table's defaults - these higher level components are the ones actually using these defaults anyway; we should defer to current production usage
- requires some class-specific syntax/workarounds
- pagination now requires the correct context provider, which our RTL render provides by default but Enzyme doesn't - we might as well reduce our snapshot footprint and write more specific assertions while here (note: this isn't perfect/complete, but it's a good start)
+ update props documentation
- anything that gets the current visible row index (e.g. focus) needs pageSize propagated from defaults + update types to reflect that internal `pagination` references should have all required props
…e` / `pageSizeOptions` props - there's a few left, e.g. virtualization, and the focus example, where custom page sizes makes sense, but overall most do not need to set a custom page size
…ading `@default`s
839b68d to
8f11a68
Compare
6f9c4e7 to
8b2e9a0
Compare
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993_buildkite/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993/ |
|
jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6993/ |
|
@tkajtoch Alrighty, this feature is finally rebased against latest main / the React 18 work and ready for review once again 😄 I'm hoping to get it merged in by next Tuesday's release, so no immediate rush, but I'd really appreciate your eyes on this before this Friday. Please let me know if that's something you have time for! |
| import React from 'react'; | ||
| import { shallow, mount } from 'enzyme'; | ||
| import { fireEvent } from '@testing-library/react'; | ||
| import { render } from '../../../test/rtl'; |
| > | ||
| <span | ||
| class="euiContextMenu__itemLayout" | ||
| > | ||
| <span | ||
| class="euiContextMenu__icon" | ||
| color="inherit" | ||
| data-euiicon-type="check" | ||
| /> | ||
| <span | ||
| class="euiContextMenuItem__text" | ||
| > | ||
| 50 rows | ||
| </span> | ||
| </span> | ||
| </button> | ||
| <button | ||
| class="euiContextMenuItem" | ||
| data-test-subj="tablePagination-100-rows" | ||
| type="button" |
There was a problem hiding this comment.
Is it expected that now the snapshot doesn't have a checked element?
There was a problem hiding this comment.
oooo, awesome catch on this. This is indeed a regression that I'll need to fix
There was a problem hiding this comment.
Actually sorry, I spoke too soon - I just QA'd again locally and the checks are correctly appearing in the UI.
For this snapshot, the check element didn't go away, it's still there next to 10 rows:
tkajtoch
left a comment
There was a problem hiding this comment.
The changes look and work great! 🚢
|
Thanks Tomasz!! There'll be one final PR after this (the feature branch itself) that I'll send your way, but it'll hopefully be super minimal in review! :) |
…ePagination` component defaults (elastic#6993)
`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>
Summary
This PR is part 2 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).
This PR is also the last PR in the
provider/component-defaultsfeature branch - once this PR merges in, the final feature PR itself will be opened up against main 🚢As always, I strongly recommend following along by commit because there's a lot of RTL test tech debt as well as docs changes/tweaks in here.
QA
gh pr checkout 6993yarn && yarn start10, 25, 50page sizessrc-docs/src/views/app_context.js<EuiProvidercomponent, addcomponentDefaults={{ EuiTablePagination: { itemsPerPage: 20, itemsPerPageOptions: [10, 20, 0] }}}10, 20, show all rowsoptionsGeneral checklist
@defaultif default values are missing)and playground toggles- [ ] Added documentationand cypress tests- [ ] 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- [ ] Checked Code Sandbox works for any docs examples- [ ] Updated the Figma library counterpart