Skip to content

Commit

Permalink
[Unified Data Table] Stop passing data grid density as key (elastic…
Browse files Browse the repository at this point in the history
…#202390)

## Summary

When the data grid density setting was added, an issue was found with
resetting the density state:
elastic#188495 (comment). It
was caused by an EUI bug: elastic/eui#7962.
The EUI bug has been fixed and this is no longer an issue, so we can
remove the workaround we were using by passing the data grid density in
the `key` prop to the data grid to force a re-render. Besides removing a
small piece of tech debt, this change improves performance when
switching densities, and no longer closes the popover when changing the
density (like other settings).

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
davismcphee authored and CAWilson94 committed Dec 9, 2024
1 parent 26d51aa commit 0328518
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 6 deletions.
2 changes: 0 additions & 2 deletions packages/kbn-unified-data-table/src/components/data_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1155,8 +1155,6 @@ export const UnifiedDataTable = ({
/>
) : (
<EuiDataGridMemoized
// Using this as the `key` is a workaround for https://github.com/elastic/eui/issues/7962. This forces a re-render if the density is changed.
key={dataGridDensity}
id={dataGridId}
aria-describedby={randomId}
aria-labelledby={ariaLabelledBy}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await dataGrid.changeDensityValue('Normal');

// toggle the popover
// Right now changing the density closes the popover (see packages/kbn-unified-data-table/src/components/data_table.tsx:1144)
// When that workaround is removed we will need to uncomment this next line
// await dataGrid.clickGridSettings();
await dataGrid.clickGridSettings();
await dataGrid.clickGridSettings();
expect(await dataGrid.getCurrentDensityValue()).to.be('Normal');
});

it('should persist the density selection after reloading the page', async () => {
await dataGrid.clickGridSettings();
await dataGrid.changeDensityValue('Expanded');
await dataGrid.clickGridSettings();
expect(await dataGrid.getCurrentDensityValue()).to.be('Expanded');

await browser.refresh();
Expand Down

0 comments on commit 0328518

Please sign in to comment.