[DataGrid] Grid style updates don't cause re-render #7962
Comments
lukasolson
added
bug
⚠️ needs validation
|
@lukasolson I'll try to fix it |
JasonStoltz
removed
the
⚠️ needs validation
|
Thanks ya'll! We've added this to our backlog. We would be super happy to receive a PR for this -- otherwise we'll lump this into some larger Data Grid work we're planning for this quarter. |
4 tasks
8 tasks
7 tasks
davismcphee
added a commit
to elastic/kibana
that referenced
this issue
Dec 2, 2024
## Summary When the data grid density setting was added, an issue was found with resetting the density state: #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)
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this issue
Dec 2, 2024
…#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) (cherry picked from commit 281269f)
hop-dev
pushed a commit
to hop-dev/kibana
that referenced
this issue
Dec 5, 2024
…#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)
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this issue
Dec 9, 2024
…#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)
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this issue
Dec 12, 2024
…#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)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
CodeSandbox here: https://codesandbox.io/s/quirky-fire-cpj3v7?file=/demo.js
When the
DataGridaccepts agridStyleprop, updates to that prop don't cause the data grid to re-render.In the above example, if you use the data grid controls to change the density, you'll see the grid re-render in response. However, if you click the button at the top "Reset to Compact", which explicitly sets the grid style to compact, it doesn't re-render (even though you can see the console logging the new grid style):
Screen.Recording.2024-08-13.at.2.32.23.PM.mov
Impact and severity
This isn't currently a big issue, but it was found as part of elastic/kibana#188495 (comment). We will have a workaround for that PR, but it would be nice if this could be fixed so the workaround can be removed.
The text was updated successfully, but these errors were encountered: