Melsumner/issue 378 #379
Melsumner/issue 378 #379
Conversation
|
@deanmarano can you check this and see if this is what you would expect? TYSM! |
| @@ -173,5 +173,77 @@ module('Integration | Component | navigation-narrator', function (hooks) { | |||
|
|
|||
| assert.dom('#ember-a11y-refocus-nav-message').isFocused(); | |||
| }); | |||
|
|
|||
| test('excludeAllQueryParams is true, queryParams do not exist but are added, do not manage focus', async function (assert) { | |||
There was a problem hiding this comment.
This test is meant to cover a scenario like this:
myurl.com/someRoute --> myurl.com/someRoute?someQueryParam=foo
| assert.dom('#ember-a11y-refocus-nav-message').isNotFocused(); | ||
| }); | ||
|
|
||
| test('excludeAllQueryParams is true, queryParams exist on the route but then are emptied, still do not manage focus', async function (assert) { |
There was a problem hiding this comment.
This test is meant to cover a scenario like this:
myurl.com/someRoute?someQueryParam=foo --> myurl.com/someRoute
| assert.dom('#ember-a11y-refocus-nav-message').isNotFocused(); | ||
| }); | ||
|
|
||
| test('excludeAllQueryParams is true, queryParams exist on the route, user navigates to new route without any query params, manage focus', async function (assert) { |
There was a problem hiding this comment.
This test is meant to cover a scenario like this:
myurl.com/someRoute?someQueryParam=foo --> myurl.com/someOtherRoute
| let hasSameRoute = this.transition.from.name === this.transition.to.name; | ||
|
|
||
| if (this.excludeAllQueryParams && this.hasQueryParams && hasSameRoute) { |
There was a problem hiding this comment.
This makes the failing test pass, but I also wonder if there's a more elegant way to do this. Suggestions welcome.
There was a problem hiding this comment.
The this.hasQueryParams might not be needed with the hasSameRoute.
| await render(hbs`<NavigationNarrator @excludeAllQueryParams={{true}} />`); | ||
| router.trigger( | ||
| 'routeDidChange', | ||
| new MockTransition({ |
There was a problem hiding this comment.
I used the same format found here, with a query param added to the from:
If merged, this PR resolves the issue described in #378
Details
hasQueryParamscheck so that it checks for more combinationshasQueryParamscheck to be more terseonRouteChangeaction to compare routesAfter review/any changes/approval, I'll squash/merge and do a patch release.