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

chore: remember last child page visited for kubernetes and preferences pages #9451

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

SoniaSandler
Copy link
Contributor

@SoniaSandler SoniaSandler commented Oct 17, 2024

What does this PR do?

This PR adds stores to keep track of the last sections visited in submenu pages, so that when leaving and coming back, the user will come back to the last section they visited in the same submenu (can check with Kubernetes for now)

Screenshot / video of UI

Screencast.from.2024-10-21.00-38-16.webm

What issues does this PR fix or reference?

Closes #9057

How to test this PR?

Enter the Kubernetes (nodes) page, go to one of the other pages in Kubernetes (e.g. Deployments), exit to some other page in PD that is not in Kubernetes, and then go back to Kubernetes. You should be back in your last visited section page (e.g. Deployments)

  • Tests are covering the bug fix or the new feature

Signed-off-by: Sonia Sandler <ssandler@redhat.com>
Signed-off-by: Sonia Sandler <ssandler@redhat.com>
@SoniaSandler SoniaSandler requested review from benoitf and a team as code owners October 17, 2024 22:11
@SoniaSandler SoniaSandler requested review from cdrage, axel7083 and gastoner and removed request for a team October 17, 2024 22:11
@SoniaSandler
Copy link
Contributor Author

@deboer-tim I wasn't sure how specific we want the last visited page to be, so for now it is the very last one (could be summary/details page in kubernetes, or create new pages in preferences), but if needed, I can change it to just keep track of the last general section visited (Nodes, Deployments, Services, Preferences, CLI Tools, etc.)

Copy link
Contributor

@gastoner gastoner left a comment

Choose a reason for hiding this comment

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

LGTM, works well!

Copy link
Contributor

@deboer-tim deboer-tim left a comment

Choose a reason for hiding this comment

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

This change seems very hard-coded, i.e. every time we add or change a submenu we also need to update App, Route, breadcrumb, and associated tests. I haven't looked at the code, but is there something we could do that is more generic / based on NavigationRegistryEntry submenus? Settings likely isn't a submenu today but could be (and that part could be a separate PR)

I was hoping for something along the lines of:

  • enter submenu for first time -> pick first page
  • navigate within submenu -> store path for next entry

Failing tests need to be fixed.

@SoniaSandler SoniaSandler marked this pull request as draft October 18, 2024 14:18
@SoniaSandler
Copy link
Contributor Author

@deboer-tim since settings is not yet a submenu, should I keep the changes for it or remove them and just update the submenu?

Signed-off-by: Sonia Sandler <ssandler@redhat.com>
@deboer-tim
Copy link
Contributor

@deboer-tim since settings is not yet a submenu, should I keep the changes for it or remove them and just update the submenu?

I'd suggest checking if we can do something simpler/less hard-coded for submenus first. If so then yes I'd break it up, otherwise we can re-evaluate.

Signed-off-by: Sonia Sandler <ssandler@redhat.com>
Signed-off-by: Sonia Sandler <ssandler@redhat.com>
@SoniaSandler
Copy link
Contributor Author

@deboer-tim I've changes the PR to work for submenus and removed the changes for the preferences section, but now the last page visited is less specific. For example, when coming back to the nodes page, it will go to the node list page, even if the last visited Kubernetes page was a specific node, but I can modify it to be more specific if needed

@SoniaSandler SoniaSandler marked this pull request as ready for review October 21, 2024 05:18
@cdrage
Copy link
Contributor

cdrage commented Oct 21, 2024

Silly question, but with this PR, unsure if this should be done within this PR or as a follow-up, but what about remembering sections in extensions that use SubmenuNavigation imported? (ex bootc extension)

@SoniaSandler
Copy link
Contributor Author

SoniaSandler commented Oct 21, 2024

@cdrage I looked in the bootc repo (same for ai-lab), and it looks like it only uses SettingsNavItem from this PR. I think that for extensions we'll have to update their navigation files manually to also remember last child page.
TBH, I'm not sure if there is a way for me to generalize it more so that it would also automatically include extensions' submenus.
For now, this PR should work for all submenus created/rendered with SubmenuNavigation.svelte, but I can explore the topic of the extension submenus more if needed.

Signed-off-by: Sonia Sandler <ssandler@redhat.com>
Signed-off-by: Sonia Sandler <ssandler@redhat.com>
Signed-off-by: Sonia Sandler <ssandler@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Child navigation should return to previous page
5 participants