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.

Sign up for GitHub

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

Jump to bottom

[Logs] Remember tab choice between Logs Explorer and Discover #193321

Closed
Kerry350 opened this issue Sep 18, 2024 · 1 comment · Fixed by #194930
Closed

[Logs] Remember tab choice between Logs Explorer and Discover #193321

Kerry350 opened this issue Sep 18, 2024 · 1 comment · Fixed by #194930
Assignees
@awahab07
Labels
Team:obs-ux-logs Observability Logs User Experience Team

Comments

@Kerry350
Copy link
Contributor

Kerry350 commented Sep 18, 2024 •
edited
Loading

Summary

For tabbed Logs Explorer / Discover (in serverless), the choice of the tab should be remembered per user and made automatically the next time the user clicks the “Discover” nav item. Initially, the Logs Explorer tab will be rendered.

Acceptance Criteria

  • The user's last clicked tab should be stored in local storage.
  • When navigating using the navigation item the value should be read from local storage, and the correct tab used.
  • The Logs Explorer tab should be used as the default.

Technical concerns and approach

There is no direct platform functionality to assist here.

The navigationTree that renders the navigation items only accepts a string (technically an app id or deep link id) for link, therefore we cannot perform any logic (reading local storage etc) for the navigation tree itself, href can also only be a string. There is an onClick prop which takes precedence, but we wouldn't have access here to the resources we need (locators, local storage utils etc).

Therefore I would propose that we add a new route (either under Discover or Logs Explorer) which acts purely as a redirection route. It would perform the logic we need, and then navigate to the correct tab. This then gives us a link that we can actually provide to the navigation tree. We should just be careful of redundant history entries.
@Kerry350 Kerry350 added the Team:obs-ux-logs Observability Logs User Experience Team label Sep 18, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

@awahab07 awahab07 self-assigned this Oct 2, 2024
@awahab07 awahab07 mentioned this issue Oct 4, 2024
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 14, 2024
@awahab07
Remember tab choice between logs explorer and discover (elastic#194930)
dc5eeb3 
Closes elastic#193321

## Summary

The PR adds the redirection point when "Discover" menu item is clicked
on the sidenav in serverless (or solution nav on stateful). Based on
what tab between "Discover" or "Logs Explorer" the user clicked
recently, "Discover" will point to that app/tab. Previously, "Discover"
would always point to "Logs Explorer" on serverless and to "Discover" on
stateful.

In order to implement this, a temporary app `last-used-logs-viewer` is
registered in `observability-logs-explorer` plugin whose only job is to
read the last stored value in local storage and perform the redirection.

Doing the redirection from a temporary app should help prevent
triggering unnecessary telemetry and history entries. And it should be
fairly easy to undo once context aware redirection is in place.

~With this implementation, only the behavior of user clicking "Discover"
on the sidenav and clicking the tabs is affected and any deeplinks from
other apps or direct links should work as is.~ The tab choice will be
updated even if the apps are visited via url.

https://github.com/user-attachments/assets/8a0308db-9ddb-47b6-b1a5-8ed70662040d

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit fed9a19)
kibanamachine added a commit that referenced this issue Oct 14, 2024
@kibanamachine @awahab07
[8.x] Remember tab choice between logs explorer and discover (#194930) (
ad83fc7 
#196068)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Remember tab choice between logs explorer and discover
(#194930)](#194930)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Abdul Wahab
Zahid","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-14T09:26:24Z","message":"Remember
tab choice between logs explorer and discover (#194930)\n\nCloses
#193321 \r\n\r\n## Summary\r\n\r\nThe PR adds the redirection point when
\"Discover\" menu item is clicked\r\non the sidenav in serverless (or
solution nav on stateful). Based on\r\nwhat tab between \"Discover\" or
\"Logs Explorer\" the user clicked\r\nrecently, \"Discover\" will point
to that app/tab. Previously, \"Discover\"\r\nwould always point to
\"Logs Explorer\" on serverless and to \"Discover\"
on\r\nstateful.\r\n\r\nIn order to implement this, a temporary app
`last-used-logs-viewer` is\r\nregistered in
`observability-logs-explorer` plugin whose only job is to\r\nread the
last stored value in local storage and perform the
redirection.\r\n\r\nDoing the redirection from a temporary app should
help prevent\r\ntriggering unnecessary telemetry and history entries.
And it should be\r\nfairly easy to undo once context aware redirection
is in place.\r\n\r\n~With this implementation, only the behavior of user
clicking \"Discover\"\r\non the sidenav and clicking the tabs is
affected and any deeplinks from\r\nother apps or direct links should
work as is.~ The tab choice will be\r\nupdated even if the apps are
visited via
url.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/8a0308db-9ddb-47b6-b1a5-8ed70662040d\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"fed9a193869739c2fae0bd7a49087fa1e216f5ac","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-logs","Team:obs-ux-management"],"title":"Remember
tab choice between logs explorer and
discover","number":194930,"url":"https://github.com/elastic/kibana/pull/194930","mergeCommit":{"message":"Remember
tab choice between logs explorer and discover (#194930)\n\nCloses
#193321 \r\n\r\n## Summary\r\n\r\nThe PR adds the redirection point when
\"Discover\" menu item is clicked\r\non the sidenav in serverless (or
solution nav on stateful). Based on\r\nwhat tab between \"Discover\" or
\"Logs Explorer\" the user clicked\r\nrecently, \"Discover\" will point
to that app/tab. Previously, \"Discover\"\r\nwould always point to
\"Logs Explorer\" on serverless and to \"Discover\"
on\r\nstateful.\r\n\r\nIn order to implement this, a temporary app
`last-used-logs-viewer` is\r\nregistered in
`observability-logs-explorer` plugin whose only job is to\r\nread the
last stored value in local storage and perform the
redirection.\r\n\r\nDoing the redirection from a temporary app should
help prevent\r\ntriggering unnecessary telemetry and history entries.
And it should be\r\nfairly easy to undo once context aware redirection
is in place.\r\n\r\n~With this implementation, only the behavior of user
clicking \"Discover\"\r\non the sidenav and clicking the tabs is
affected and any deeplinks from\r\nother apps or direct links should
work as is.~ The tab choice will be\r\nupdated even if the apps are
visited via
url.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/8a0308db-9ddb-47b6-b1a5-8ed70662040d\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"fed9a193869739c2fae0bd7a49087fa1e216f5ac"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194930","number":194930,"mergeCommit":{"message":"Remember
tab choice between logs explorer and discover (#194930)\n\nCloses
#193321 \r\n\r\n## Summary\r\n\r\nThe PR adds the redirection point when
\"Discover\" menu item is clicked\r\non the sidenav in serverless (or
solution nav on stateful). Based on\r\nwhat tab between \"Discover\" or
\"Logs Explorer\" the user clicked\r\nrecently, \"Discover\" will point
to that app/tab. Previously, \"Discover\"\r\nwould always point to
\"Logs Explorer\" on serverless and to \"Discover\"
on\r\nstateful.\r\n\r\nIn order to implement this, a temporary app
`last-used-logs-viewer` is\r\nregistered in
`observability-logs-explorer` plugin whose only job is to\r\nread the
last stored value in local storage and perform the
redirection.\r\n\r\nDoing the redirection from a temporary app should
help prevent\r\ntriggering unnecessary telemetry and history entries.
And it should be\r\nfairly easy to undo once context aware redirection
is in place.\r\n\r\n~With this implementation, only the behavior of user
clicking \"Discover\"\r\non the sidenav and clicking the tabs is
affected and any deeplinks from\r\nother apps or direct links should
work as is.~ The tab choice will be\r\nupdated even if the apps are
visited via
url.\r\n\r\n\r\n\r\nhttps://github.com/user-attachments/assets/8a0308db-9ddb-47b6-b1a5-8ed70662040d\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"fed9a193869739c2fae0bd7a49087fa1e216f5ac"}}]}]
BACKPORT-->

Co-authored-by: Abdul Wahab Zahid <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@awahab07 awahab07

Labels
Team:obs-ux-logs Observability Logs User Experience Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remember tab choice between logs explorer and discover awahab07/kibana
3 participants
@Kerry350 @awahab07 @elasticmachine