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

ui: Fix sticky namespace selector #11830

Merged
merged 2 commits into from Dec 15, 2021

Conversation

johncowen
Copy link
Contributor

When the application initially loads we request a list of namespaces from the backend within the application model hook in order to immediately populate the namespace menu in the top left of the UI.

We also use this application model namespace list to find 'valid' namespaces to mark as 'selected' in order to show the currently selected menu item. If we can't find the specified namespace (taken from the URL segment) then we default to just show the 'default' namespace instead.

The namespace menu is refreshed everytime it is opened (we fire off a request to get a refreshed response) - importantly this request doesn't refresh the model on the application model hook, which is the one we sometimes use to check for a 'valid' selection 😬 .

The application model hook is also not refreshed when you save a new namespace via the UI. Meaning that we end up defaulting to 'default' if you save a new namespace and then immediately try to select it with the same 'javascript execution session' (i.e. without manually hitting refresh)

All in all this can mean that once you have saved a namespace, whilst it possible to select it and navigate to it, it sometimes seems like the top menu doesn't want to mark it as selected.

The fix here is to just make sure to use the central data store instead of using the result of the application model hook which in Ember's case is the Ember Data store.

Lastly, 1.10 forwards uses a completely different approach for the entire UI so this is a 1.9 only fix (note the base branch) that will never be ported forwards and we will soonish drop support for this version. Oh I also changed to retrieve the list of dcs here also, which meant I could get rid of the app variable, and whilst there isn't a similar related bug there, I felt it was best to just access this directly from the store also.

Changelog coming shortly.

Fixes #11553

When adding a namespace via the UI, the app model which contains a list
of namespace isn't refreshed. This means when we use the same list after
adding a namespace to search for the namespace that is contained in the
URL, if that namespace in the URL is the new one, its is not found, and
therefore the menu does not get 'selected' properly, leaving a default
selection of 'default'

This commit instead uses peekAll of the Ember Data store to access the
entire list of cached namespaces instead of using the now stale
app.nspaces one.
@johncowen johncowen added the theme/ui Anything related to the UI label Dec 14, 2021
@johncowen johncowen requested review from jgwhite, amyrlam, a user and natmegs December 14, 2021 16:01
@vercel
Copy link

vercel bot commented Dec 14, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

consul-ui-staging – ./ui

🔍 Inspect: https://vercel.com/hashicorp/consul-ui-staging/9yvYvM9HxSMF1KsgktogqKmyhhjS
✅ Preview: Failed

[Deployment for a7df08c canceled]

@vercel vercel bot temporarily deployed to Preview – consul December 15, 2021 12:07 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging December 15, 2021 12:07 Inactive
@johncowen johncowen merged commit aafc02b into release/1.9.x Dec 15, 2021
@johncowen johncowen deleted the ui/bugfix/sticky-nspace-selector branch December 15, 2021 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants