[EuiDataGrid] Display selector / toolbar control enhancements by cee-chen · Pull Request #8080 · elastic/eui

cee-chen · 2024-10-16T18:45:52Z

Summary

This PR adds 3 separate enhancements (listed with screenshots below). As always, I strongly recommend following along by commit, as I tried to split out several code refactors out to make following the actual new features easier.

Redesigned row height form controls (`493e2e0`)

Per @MichaelMarcialis's designs in #7835 (comment) (although these changes do not close the issue,

Single/Default	Custom `lineCount`	Auto fit

Completely custom control over rendering the display selector (`92bc73e`, `81b5e25`) (closes #7951)

Consumers can now pass a toolbarVisibility.showDisplaySelector.customRender render prop/function that passes back the default density/row height/reset button as optional nodes to render. This allows consumers to reorder and otherwise completely customize the display selector.

const customDisplaySelectorRender = ({ densityControl }) => (
  <div>Custom display selector {densityControl}</div>
);

<EuiDataGrid
  // ... required props
  toolbarVisibility={{
    showDisplaySelector: {
      customRender: customDisplaySelectorRender
    }
  }}
/>

`gridStyle` updates after users have changed grid styles via the display selector (`9cf4a4d`) (closes #7962)

Previously, EuiDataGrid would always defer to the user's row height/density selection from the display selector controls, and if consumers tried to pass in new gridStyle updates after, or control them from an external button, those updates would be ignored.

You can reproduce this on production by changing the density in the toolbar display selector and then trying to change fontSize or cellPadding in the Storybook controls - nothing happens at that point.

This PR changes this behavior so that new gridStyle updates are reflected in the datagrid (see QA steps below).

QA

New row height UI should be testable on any datagrid:
- https://eui.elastic.co/pr_8080/#/tabular-content/data-grid
Custom render testing:
- https://eui.elastic.co/pr_8080/#/tabular-content/data-grid-toolbar#completely-custom-toolbar-rendering
gridStyles updates should be testable on Storybook:
- Go to https://eui.elastic.co/pr_8080/storybook/?path=/story/tabular-content-euidatagrid-gridstyle-prop--playground&args=fontSize:s
- Go to the display selector in the toolbar and change, e.g. density to Expanded
- In the Storybook controls, change fontSize to s and confirm that the datagrid correctly updates to this setting

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
- [ ] Added documentation
- Props have proper autodocs (using @default if default values are missing) and playground toggles
  ~~- [ ] Checked Code Sandbox works for any docs examples~~
Code quality checklist
- Added or updated jest ~~and cypress tests~~
- Updated visual regression tests
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
- If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

…n subcomponents - we'll need this in order to pass it back a render prop version of this popover - also feels much cleaner code wise! and lets us not render some extra state if controls have been configured to be off

+ remove unnecessary flex item wrapper + reorder buttonLabel to be closer to displaySelector

- not flagging it in the main docs as it doesn't seem important enough, let me know if you disagree - it is in the snippets and props examples however + fix bad indentation and syntax on some existing snippets

…have been made - requires updating how reset button behaves, and storing initial display settings in refs on mount + reorder/organize code with comment blocks / by concern - modify deep equal to potentially not cost as much / re-run isEqual as often if the object *is* correctly memoized

- works when run alone, not entirely sure what's going on here

...erence/chrome_mobile_Tabular_Content_EuiDataGrid_toolbarVisibility_prop_Display_Selector.png

packages/eui/src/components/datagrid/controls/display_selector.tsx

- indentation nit - fix object equality issue when rowHeightsOptions is initialized with `{ lineCount }`

mgadewoll

🚢 🐈‍⬛ Those are awesome improvements! The fixes/updates work as expected and the code looks good to me. 👍
I left two small recommendations but they're not blocking.

mgadewoll · 2024-10-21T16:04:15Z

packages/eui/src/components/datagrid/controls/display_selector.tsx

    return 'expanded';
  return '';
 };
+const DensityControl = ({


Considering this file is quite large and since we're creating standalone parts now anyway maybe we could move those sub components to their own files? 🤔

This is definitely subjective / more of an art than a science, but to me in its current state the file would be more annoying to have it broken up than in one (slightly larger) file since I'd have to be referring back and forth to separate files to see how it all works together, particularly with the reset button.

My 2c is that it's not egregious right now since it's still below ~500 lines (especially considering comments etc) and concerns one major UI element (the display settings popover and its contents).

packages/eui/src/components/datagrid/controls/display_selector.tsx

kibanamachine · 2024-10-23T16:27:47Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_8080/
Storybook: https://eui.elastic.co/pr_8080/storybook
New docs: https://eui.elastic.co/pr_8080/new-docs

elasticmachine · 2024-10-23T16:30:42Z

💚 Build Succeeded

Buildkite Build
Commit: 508d0f1

History

💚 Build #2904 succeeded 04b8c02
💔 Build #2903 failed 493e2e0

`v97.2.0`⏩`v97.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)_ --- ## [`v97.3.0`](https://github.com/elastic/eui/releases/v97.3.0) - `EuiDataGrid` now supports a new `toolbarVisibility.showDisplaySelector.customRender` function that allows completely customizing the rendering of the display selector popover ([#8080](elastic/eui#8080)) - `EuiDataGrid`'s row height/lines per row setting has been streamlined in both UI and UX ([#8080](elastic/eui#8080)) - `EuiDataGrid` now accepts consumer-passed display setting updates even after users have changed their display preferences via UI ([#8080](elastic/eui#8080)) - Updated `EuiDataGrid` to vertically center all `toolbarVisibility.additionalControls` nodes ([#8085](elastic/eui#8085)) - Updated `EuiDataGrid` with a beta `rowHeightsOptions.autoBelowLineCount` feature flag ([#8096](elastic/eui#8096)) - Updated `EuiContextMenuPanel` to allow disabling initial focus via `initialFocusedItemIndex={-1}` ([#8101](elastic/eui#8101)) **Bug fixes** - Fixed `EuiComment`'s typing to correctly reflect all accepted props ([#8089](elastic/eui#8089)) - Fixed `EuiSelectableTemplateSitewide`s within dark-themed `EuiHeader`s missing input borders ([#8100](elastic/eui#8100))

`v97.2.0`⏩`v97.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)_ --- ## [`v97.3.0`](https://github.com/elastic/eui/releases/v97.3.0) - `EuiDataGrid` now supports a new `toolbarVisibility.showDisplaySelector.customRender` function that allows completely customizing the rendering of the display selector popover ([elastic#8080](elastic/eui#8080)) - `EuiDataGrid`'s row height/lines per row setting has been streamlined in both UI and UX ([elastic#8080](elastic/eui#8080)) - `EuiDataGrid` now accepts consumer-passed display setting updates even after users have changed their display preferences via UI ([elastic#8080](elastic/eui#8080)) - Updated `EuiDataGrid` to vertically center all `toolbarVisibility.additionalControls` nodes ([elastic#8085](elastic/eui#8085)) - Updated `EuiDataGrid` with a beta `rowHeightsOptions.autoBelowLineCount` feature flag ([elastic#8096](elastic/eui#8096)) - Updated `EuiContextMenuPanel` to allow disabling initial focus via `initialFocusedItemIndex={-1}` ([elastic#8101](elastic/eui#8101)) **Bug fixes** - Fixed `EuiComment`'s typing to correctly reflect all accepted props ([elastic#8089](elastic/eui#8089)) - Fixed `EuiSelectableTemplateSitewide`s within dark-themed `EuiHeader`s missing input borders ([elastic#8100](elastic/eui#8100)) (cherry picked from commit 4e7d43a)

cee-chen added 7 commits October 15, 2024 16:46

[refactor] Pull out reset button as well

8ea7bd0

+ remove unnecessary flex item wrapper + reorder buttonLabel to be closer to displaySelector

[7951] Add customRender API to showDisplaySelector

92bc73e

[docs] Add new customRender examples/props docs

81b5e25

- not flagging it in the main docs as it doesn't seem important enough, let me know if you disagree - it is in the snippets and props examples however + fix bad indentation and syntax on some existing snippets

[refactor] minor code organization

693dae7

[7835] Simplify row height selector UI to a single line

493e2e0

cee-chen added the data grid label Oct 16, 2024

cee-chen added 2 commits October 16, 2024 12:00

changelog

84f81a4

Skip React 16/17 test

04b8c02

- works when run alone, not entirely sure what's going on here

cee-chen commented Oct 17, 2024

View reviewed changes

...erence/chrome_mobile_Tabular_Content_EuiDataGrid_toolbarVisibility_prop_Display_Selector.png Show resolved Hide resolved

cee-chen commented Oct 17, 2024

View reviewed changes

packages/eui/src/components/datagrid/controls/display_selector.tsx Outdated Show resolved Hide resolved

cee-chen marked this pull request as ready for review October 17, 2024 16:50

cee-chen requested a review from a team as a code owner October 17, 2024 16:50

Minor fixes

bf312e6

- indentation nit - fix object equality issue when rowHeightsOptions is initialized with `{ lineCount }`

mgadewoll approved these changes Oct 22, 2024

View reviewed changes

[PR feedback] Use var/const instead of string

508d0f1

cee-chen enabled auto-merge (squash) October 23, 2024 15:47

cee-chen merged commit 07a6a6f into elastic:main Oct 23, 2024

cee-chen deleted the datagrid/display-selector branch October 23, 2024 16:30

This was referenced Oct 23, 2024

[DataGrid] Ability to rearrange display selector components and/or render custom display selector #7951

Closed

Upgrade EUI to v97.3.0 elastic/kibana#198087

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiDataGrid] Display selector / toolbar control enhancements#8080

[EuiDataGrid] Display selector / toolbar control enhancements#8080
cee-chen merged 11 commits intoelastic:mainfrom
cee-chen:datagrid/display-selector

cee-chen commented Oct 16, 2024 •

edited by mgadewoll

Loading

Uh oh!

Uh oh!

Uh oh!

mgadewoll left a comment

Uh oh!

mgadewoll Oct 21, 2024

Uh oh!

cee-chen Oct 23, 2024

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Oct 23, 2024

Uh oh!

elasticmachine commented Oct 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

cee-chen commented Oct 16, 2024 • edited by mgadewoll Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Redesigned row height form controls (493e2e0)

Completely custom control over rendering the display selector (92bc73e, 81b5e25) (closes #7951)

gridStyle updates after users have changed grid styles via the display selector (9cf4a4d) (closes #7962)

QA

General checklist

Uh oh!

Uh oh!

Uh oh!

mgadewoll left a comment

Choose a reason for hiding this comment

Uh oh!

mgadewoll Oct 21, 2024

Choose a reason for hiding this comment

Uh oh!

cee-chen Oct 23, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Oct 23, 2024

Uh oh!

elasticmachine commented Oct 23, 2024

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cee-chen commented Oct 16, 2024 •

edited by mgadewoll

Loading

Redesigned row height form controls (`493e2e0`)

Completely custom control over rendering the display selector (`92bc73e`, `81b5e25`) (closes #7951)

`gridStyle` updates after users have changed grid styles via the display selector (`9cf4a4d`) (closes #7962)