LinkTo Transition Bug #16983
LinkTo Transition Bug #16983
Conversation
| &.active { | ||
| &.is-active { |
There was a problem hiding this comment.
While attempting to test that the nav drawer closed when clicking a link on smaller screens I noticed that it wasn't appearing on screen in the first place. Updating the class name to match the template fixed the issue.
|
Deployment failed with the following error: |
| @models={{this.link.models}} | ||
| @query={{this.query}} | ||
| @disabled={{@disabled}} | ||
| {{on "click" this.onLinkClick}} |
There was a problem hiding this comment.
This was only being used in the SecretVersionMenu component to close the menu but since a new route is transitioned to the menu is torn down making it unnecessary.
| @@ -113,7 +113,6 @@ | |||
| "ember-export-application-global": "^2.0.1", | |||
| "ember-fetch": "^8.1.1", | |||
| "ember-inflector": "4.0.2", | |||
| "ember-link-action": "^1.0.0", | |||
There was a problem hiding this comment.
This was no longer used as of 1.11
| } | ||
| }, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
This probably won't be used again but it might be good to keep all of the recast scripts around for examples to help with future codemods.
There was a problem hiding this comment.
This is great to see, as someone who's never written a codemod!
ui/app/templates/vault/cluster.hbs
Outdated
| @@ -12,48 +12,52 @@ | |||
| <NamespacePicker @class="navbar-item" @namespace={{this.namespaceQueryParam}} /> | |||
| </li> | |||
| {{/if}} | |||
| <li class={{if (is-active-route "vault.cluster.secrets") "is-active"}}> | |||
| <li class={{if (is-active-route "vault.cluster.secrets") "is-active"}} role="button" {{on "click" Nav.closeDrawer}}> | |||
There was a problem hiding this comment.
I think I like this pattern better, where we add the "side effects" click event on the event outside the element, which just has the @route link. I'm concerned that making these links technically buttons violates some A11y concerns, except of course in cases when the button is triggering a save or submit, and then redirecting.
There was a problem hiding this comment.
I wondered about that too and ultimately followed the suggestions from the linter. I think since the li is the parent element it's not actually the link so the role of button might be ok? Basically the li is acting as a button to close the nav drawer. The nested link functions as a link to transition routes.
| } | ||
| }, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
This is great to see, as someone who's never written a codemod!
I had some success using |
| @action | ||
| closeDropdown(dropdown) { | ||
| // strange issue where closing dropdown triggers full transition which redirects to auth screen in production builds | ||
| // closing dropdown in next tick of run loop fixes it |
There was a problem hiding this comment.
Haha yes! I noticed that a full page reload was happening when only a partial transition should have been taking place. I wonder if closing the dropdown in the same tick was throwing an error behind the scenes which caused issues with the transition?
It was discovered that in production builds of Ember, LinkTo elements that used the on click modifier were transitioning to the incorrect route. During the latest Ember upgrade a new lint rule was alerting of unsupported arguments being used on the LinkTo component. In this case,
invokeActionwas being provided by theember-link-actionaddon which was removed in favor of using theonmodifier.Since it works as expected in development builds it seems like it could be a bug in Ember, but it also seems like a link that has a click event is maybe not the best pattern. After attempting to find a way to make a click event work it was decided to use the
buttonelement instead along with the router service to transition inside the event handler.Migrations Details
A script was created for
ember-template-recastto identify any template files that hadLinkToelements with theonmodifier. The UI elements impacted were:Where feasible, the
LinkToelements were changed to buttons and the router service was used to transition. In other cases where a link was appropriate theonmodifier was added to the parent element.Update
In the interests of accessibility the updates to use the
buttonelement overLinkTowere reverted. After further testing, the issue was not with the on click modifier but specifically with closing the dropdown in the event handler. To work around this the dropdown is now being closed in the next tick of the run loop which allows for the transition to complete as expected.