ui: Improved main navigation #7673
Merged
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
kaxcode
force-pushed
the
ui/feature/improved-main-nav
branch
from
5b72dd3
to
a8a30bd
Compare
johncowen
force-pushed
the
ui/feature/improved-main-nav
branch
from
a8a30bd
to
5b72dd3
Compare
added 9 commits
April 21, 2020 08:47
johncowen
force-pushed
the
ui/feature/improved-main-nav
branch
from
5b72dd3
to
8c32ba9
Compare
kaxcode
approved these changes
Apr 21, 2020
| token: service('repository/token'), | ||
| type: service('data-source/protocols/http/blocking'), | ||
| source: function(src, configuration) { | ||
| const [, dc /*nspace*/, , model, ...rest] = src.split('/'); | ||
| const [, , /*nspace*/ dc, model, ...rest] = src.split('/'); |
There was a problem hiding this comment.
Did you leave an extra comma here?
There was a problem hiding this comment.
Oh no hehehe, I just tried to fix this about 4 times before I realised prettier is 'unfixing' it for me whenever I commit! 🤣
I've got a PR coming soon that uncomments this nspace so the problem will go away when I PR that.
johncowen
force-pushed
the
ui/feature/improved-main-nav
branch
3 times, most recently
from
bd11bc1
to
566e833
Compare
johncowen
added a commit
that referenced
this pull request
Apr 30, 2020
* Make datacenter queries use query vs findAll like the rest of the app * Make sure we have an element to pass to isInViewport * Make sure href-mut doesn't error even if the currentRoute === null * More post test cleanup and Safari fix (safari requires http:// URLs) * Reverse order of datasource nspace/dc's and add a namespace source * Rearrange routes/templates/controllers to only use HashicorpConsul once * Add datasources and correct token namespace detection/redirection * Remove old dc findAll adapter method * Add more comments around the 'child route/parent controller' vars
kaxcode
pushed a commit
that referenced
this pull request
May 11, 2020
* Make datacenter queries use query vs findAll like the rest of the app * Make sure we have an element to pass to isInViewport * Make sure href-mut doesn't error even if the currentRoute === null * More post test cleanup and Safari fix (safari requires http:// URLs) * Reverse order of datasource nspace/dc's and add a namespace source * Rearrange routes/templates/controllers to only use HashicorpConsul once * Add datasources and correct token namespace detection/redirection * Remove old dc findAll adapter method * Add more comments around the 'child route/parent controller' vars
johncowen
added a commit
that referenced
this pull request
May 12, 2020
* Make datacenter queries use query vs findAll like the rest of the app * Make sure we have an element to pass to isInViewport * Make sure href-mut doesn't error even if the currentRoute === null * More post test cleanup and Safari fix (safari requires http:// URLs) * Reverse order of datasource nspace/dc's and add a namespace source * Rearrange routes/templates/controllers to only use HashicorpConsul once * Add datasources and correct token namespace detection/redirection * Remove old dc findAll adapter method * Add more comments around the 'child route/parent controller' vars
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.
This PR reduces the individual places where we use our 'app chrome' component (
<HashicorpConsul />).Previously it individually wrapped the loading state, the app itself, the app error state and then individually the settings page. This was all due to not knowing important data required for the main navigation such as the current DC and current namespace until the user had entered the DC route.
We now get around this restriction by passing data up from the child DC route to the parent application controller (thanks @meirish for the suggestion!)
As a consequence of this we have moved the loading of Datacenters and Namespace up to the application route, which is almost never reloaded/refreshed. We therefore use our new
<Datasource />component to only reload datacenters and namespaces when the menus themselves are opened.<Datasource />s work just like<img />tags, so you can set theirloading=""to beloading="eager"orloading="lazy"for lazy loading, just like<img />tags.We also move our Datacenter adapter to use
querylike the rest of the adapters in the UI instead of findAll.There are a few other related changes here, such as trying to ensure that
href-mutwill always generated a URL even if therouterservice returns acurrentRoute === null, and a more sane way of checking whether a user needs redirecting from anspace.indexroute (big thanks to @randallmorey for the transition vs currentURL work here)There's probably more work to come here in reducing the error and loading states in the application route to other individual components.