[Index Management] Keep the filters value in the url for the indices list #174515
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yuliacech
added
release_note:fix
Feature:Index Management
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
Pinging @elastic/platform-deployment-management (Team:Deployment Management) |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
ElenaStoeva
approved these changes
Jan 18, 2024
There was a problem hiding this comment.
Thanks for patching this up @yuliacech! Tested locally, code changes LGTM 👍
kibanamachine
added
v8.13.0
backport:skip
10 tasks
yuliacech
added a commit
that referenced
this pull request
Jan 25, 2024
## Summary This PR fixes a regression from #174515: When any tab on the index details page is clicked, then subsequent tab changes are not possible. To reproduce: 1. Open any index on the index details page 2. Click "Mappings" tab to navigate to that tab 3. Click "Settings" to navigate to another tab but nothing happens. The bug is caused by the logic to keep url parameters for the indices list table, but those parameters already include `tab` which should be reset before any further navigation is possible. ### Checklist Delete any items that are not applicable to this PR. - [ ] 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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] 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 renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### Risk Matrix Delete this section if it is not applicable to this PR. Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | | [See more potential risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
yuliacech
added
the
backport:prev-minor
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
yuliacech
added a commit
to yuliacech/kibana
that referenced
this pull request
Jan 26, 2024
…list (elastic#174515) ## Summary Fixes elastic#173637 This PR fixes a UX regression introduced in 8.11 by adding a new index details page: the search value is lost when the user clicks the index name and the index details page is opened. When they click the "back to all indices" button, the search in the indices list is cleared. To fix this issue, we need to store the data of the users' search, toggles and pagination before opening the index details page so that when going back to the indices list, these parameters could be restored in the table. This PR adds these parameters to the url when the index page is opened and when navigating back, the indices list is initialized with this data. Since the toggles of the indices list can be added dynamically via the extensions service, there logic for storing this data needs to be dynamic as well. For a cleaner solution, I decided to move the "include hidden indices" toggle to the extensions service as well, since the logic of this toggle is exactly the same as the one for "include rollup indices" toggle for example. This caused a minor change in the UI: the rollup and hidden indices toggles are now displayed in the reversed order. ### Screenshot <img width="916" alt="Screenshot 2024-01-11 at 14 38 22" src="https://github.com/elastic/kibana/assets/6585477/e7174909-0a68-4041-94a2-2d1646eae0fe"> ### Checklist Delete any items that are not applicable to this PR. - [ ] 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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] 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 renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: kibanamachine <[email protected]> (cherry picked from commit 953c92f) # Conflicts: # x-pack/plugins/index_management/public/application/sections/home/index_list/details_page/details_page.tsx # x-pack/plugins/index_management/public/application/sections/home/index_list/index_table/index_table.js # x-pack/plugins/index_management/public/services/extensions_service.ts
kibanamachine
added
the
backport missing
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
2 similar comments
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
|
Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync. |
yuliacech
removed
backport missing
CoenWarmer
pushed a commit
to CoenWarmer/kibana
that referenced
this pull request
Feb 15, 2024
…list (elastic#174515) ## Summary Fixes elastic#173637 This PR fixes a UX regression introduced in 8.11 by adding a new index details page: the search value is lost when the user clicks the index name and the index details page is opened. When they click the "back to all indices" button, the search in the indices list is cleared. To fix this issue, we need to store the data of the users' search, toggles and pagination before opening the index details page so that when going back to the indices list, these parameters could be restored in the table. This PR adds these parameters to the url when the index page is opened and when navigating back, the indices list is initialized with this data. Since the toggles of the indices list can be added dynamically via the extensions service, there logic for storing this data needs to be dynamic as well. For a cleaner solution, I decided to move the "include hidden indices" toggle to the extensions service as well, since the logic of this toggle is exactly the same as the one for "include rollup indices" toggle for example. This caused a minor change in the UI: the rollup and hidden indices toggles are now displayed in the reversed order. ### Screenshot <img width="916" alt="Screenshot 2024-01-11 at 14 38 22" src="https://github.com/elastic/kibana/assets/6585477/e7174909-0a68-4041-94a2-2d1646eae0fe"> ### Checklist Delete any items that are not applicable to this PR. - [ ] 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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] 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 renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: kibanamachine <[email protected]>
CoenWarmer
pushed a commit
to CoenWarmer/kibana
that referenced
this pull request
Feb 15, 2024
## Summary This PR fixes a regression from elastic#174515: When any tab on the index details page is clicked, then subsequent tab changes are not possible. To reproduce: 1. Open any index on the index details page 2. Click "Mappings" tab to navigate to that tab 3. Click "Settings" to navigate to another tab but nothing happens. The bug is caused by the logic to keep url parameters for the indices list table, but those parameters already include `tab` which should be reset before any further navigation is possible. ### Checklist Delete any items that are not applicable to this PR. - [ ] 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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] 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 renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### Risk Matrix Delete this section if it is not applicable to this PR. Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | | [See more potential risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #173637
This PR fixes a UX regression introduced in 8.11 by adding a new index details page: the search value is lost when the user clicks the index name and the index details page is opened. When they click the "back to all indices" button, the search in the indices list is cleared.
To fix this issue, we need to store the data of the users' search, toggles and pagination before opening the index details page so that when going back to the indices list, these parameters could be restored in the table. This PR adds these parameters to the url when the index page is opened and when navigating back, the indices list is initialized with this data.
Since the toggles of the indices list can be added dynamically via the extensions service, there logic for storing this data needs to be dynamic as well. For a cleaner solution, I decided to move the "include hidden indices" toggle to the extensions service as well, since the logic of this toggle is exactly the same as the one for "include rollup indices" toggle for example. This caused a minor change in the UI: the rollup and hidden indices toggles are now displayed in the reversed order.
Screenshot
Checklist
Delete any items that are not applicable to this PR.