Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discover] Fix navigating back when changing index pattern #84061

Conversation

kertal
Copy link
Member

@kertal kertal commented Nov 23, 2020

Summary

This PR restores the functionality of Discover to navigate back and forth selected index patterns using the browser back and forward button.

How to test

  1. Go to Discover
  2. Change index pattern
  3. Click browser back button -> now the previous index pattern should be selected

Fixes #79872

Checklist

@kertal kertal added the Feature:Discover Discover Application label Nov 23, 2020
@kertal kertal self-assigned this Nov 23, 2020
@@ -301,8 +302,7 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise
$scope.state.sort,
config.get(MODIFY_COLUMNS_ON_SWITCH)
);
await replaceUrlAppState(nextAppState);
$route.reload();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so switching the index pattern is now pushing to the browser history

@@ -264,7 +264,8 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise
if (!_.isEqual(newStatePartial, oldStatePartial)) {
$scope.$evalAsync(async () => {
if (oldStatePartial.index !== newStatePartial.index) {
//in case of index switch the route has currently to be reloaded, legacy
//in case of index pattern switch the route has currently to be reloaded, legacy
$route.reload();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when there is a change of index pattern in URL, the route is reloaded, so the new index pattern is selected ... reload is necessary because of other legacy dependencies, and will be no longer necessary in the future, when moving to react router

@kertal kertal added Team:Visualizations Visualization editors, elastic-charts and infrastructure release_note:fix v7.11.0 v8.0.0 labels Nov 24, 2020
@kertal kertal marked this pull request as ready for review November 24, 2020 09:41
@kertal kertal requested a review from a team November 24, 2020 09:41
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@kertal kertal requested a review from majagrubic November 25, 2020 11:40
@kertal
Copy link
Member Author

kertal commented Nov 26, 2020

@elasticmachine merge upstream

@majagrubic
Copy link
Contributor

When I tested this in Chrome on Mac OS, I found that after re-selecting an index pattern and navigating back, I'm directed back to home, not a previous index pattern in Discover:
discover_back

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
discover 430.2KB 430.2KB -14.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@majagrubic majagrubic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After doing yarn kbn clean it works fine for me. Tested in Chrome and Firefox on Mac OS.

@kertal kertal merged commit f0192c1 into elastic:master Nov 26, 2020
kertal added a commit to kertal/kibana that referenced this pull request Nov 26, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 30, 2020
* master:
  [Security Solution] Exceptions Cypress tests (elastic#81759)
  [ML] Fix spaces job ID check (elastic#84404)
  [Security Solution][Detections] Handle dupes when processing threshold rules (elastic#83062)
  skip flaky suite (elastic#84440)
  skip flaky suite (elastic#84445)
  [APM] Fix missing `service.node.name` (elastic#84269)
  Upgrade fp-ts to 2.8.6 (elastic#83866)
  Added data streams privileges to better control delete actions in UI (elastic#83573)
  Improve short-url redirect validation (elastic#84366)
  TSVB offsets (elastic#83051)
  [Discover] Fix navigating back when changing index pattern (elastic#84061)
  [Logs UI] Polish the UI for the log entry examples in the anomaly table (elastic#82139)
  [Logs UI] Limit the height of the "view in context" container (elastic#83178)
  [Application Usage] Update `schema` with new `fleet` rename (elastic#84327)
  fix identation in list (elastic#84301)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigate back in Discover does not work for index pattern changes from 7.7
4 participants