-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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: Add Route component / routlet service #9813
Conversation
🤔 Double check that this PR does not require a changelog entry in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ~whoops, accidentally requested changes.
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/334115. |
This PR uses the excellent a11y-dialog to implement our modal functionality across the UI. This package covers all our a11y needs - overlay click and ESC to close, controlling aria-* attributes, focus trap and restore. It's also very small (1.6kb) and has good DOM and JS APIs and also seems to be widely used and well tested. There is one downside to using this, and that is: We made use of a very handy characteristic of the relationship between HTML labels and inputs in order to implement our modals previously. Adding a for="id" attribute to a label meant you can control an <input id="id" /> from anywhere else in the page without having to pass javascript objects around. It's just based on using the same string for the for attribute and the id attribute. This allowed us to easily open our login dialog with CSS from anywhere within the UI without having to manage passing around a javascript object/function/method in order to open the dialog. We've PRed #9813 which includes an approach which would make passing around JS modal object easier to do. But in the meantime we've added a little 'hack' here using an additional <input /> element and a change listener which allows us to keep this label/input characteristic of our old modals. I'd originally thought this would be a temporary amend in order to wait on #9813 but the more I think about it, the more I think its quite a nice thing to keep - so longer term we may/may not keep this.
Back in #8779 we added a new
<Outlet />
component which we use to automatically figure out the loading states for the hierarchy of templates/outlets as well as help us to provide unique CSS selectors to target individual outlets/routes in order to apply those tiny individual styling tweaks that are sometimes needed.This PR builds upon that work by providing a new
<Route>
component and a newroutlet
service which ties both the<Outlet />
and<Route />
components together to provide various different pieces of functionality. All in all this combined work gives us:Please see #8779 for an explanation of points 1 and 2.
This PR adds the
Route
component and only uses it in one area of the catalog area of the UI just for usage reference for this PR, but the idea is that every single template will have a top levelRoute
component, just as every single template with an outlet has anOutlet
component.Data down
Once we have this we can achieve the following:
We currently use ember
Routes
andmodelFor
to achieve this functionality but this new 'routlet' approach has several advantages:this.modelFor('dc')
from an emberRoute
is essentially 'reaching up' through the route hierarchy to retrieve data rather than passing 'data down' as is usually recommended.this.modelFor('dc')
from an emberRoute
means theRoute
has access to anything it likes. Passing the required data down through the 'routlet' gives a little more control - the nested template only has access to the data that has been passed to it by its parent template.This means we can reduce the amount of code/Routes that only exist in order to 'reach up' to the parent route in order to make data available in the template.
For the times when we still need to access this data from an ember Route, we also provide a
@service('routlet')
so you can access the same data from an ember Route just as you would using the normalmodelFor
method:Visual transitions
Whilst in this PR we don't add any visual transitions when navigating between routes, we add the machinery here to allow us to do this easily.
<Outlet>
s now have atransitionend
event listener applied to them. So, only if they have a CSS transition duration, they will resolve a promise that is shared across the 'routlet'. This promise can be accessed via theroutlet
service:We still need to decide whether this is something we actually want to do, so we don't use this here yet, but the machinery is there to be able to do this.
A small tangent to this was changing our
is-href
helper to 'activate' buttons/links when you click them rather than when the route has loaded. We achieve this using an event listener on the emberrouter
service rather than an old style observer, which overall makes the UI feel a lot more responsive now that buttons stay highlighted when you click on them.A11y route navigation
Once every page/tempalte has a wrapping
<Route />
this means we can centralize our page title functionality, possibly like:This (hopefully) means we can potentially implement page navigation announcements in a centralized place in order to improve the a11y of the UI. This feature seems to be the number one priority for improving a11y generally for ember apps.
There is already quite a lot of functionality all surrounding this 'routlet' concept, so we've left this for a future PR.
One final tiny little detail, I've found the spelling/typing of
routlet
vsroutelet
to be a tiny-teeny bit awkward sometimes, but I eventually decided on theroutlet
spelling for if you ever need to search across the project forroute
as opposed toroutlet
. Using theroutlet
spelling meansroutlet
will be excluded from a search forroute
.