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

Keyboard navigation is broken #1439

Open
gkappen opened this issue May 12, 2023 · 1 comment
Open

Keyboard navigation is broken #1439

gkappen opened this issue May 12, 2023 · 1 comment
Labels

Comments

@gkappen
Copy link

gkappen commented May 12, 2023

Keyboard navigation (using ArrowLeft and ArrowRight to go to the previous or next topic) is broken on any site created using this addon. You can check it out by going to any topic on this addons own docs and pressing one of the arrow keys.

The resulting error is something like this:

Uncaught Error:  More context objects were passed than there are dynamic segments for the route: docs.usage
    at I.applyToHandlers (vendor-907f1d49045ce1b9b3f2f6d948b40f39.js:46691:15)
    at I.applyToState (vendor-907f1d49045ce1b9b3f2f6d948b40f39.js:46628:19)
    at s.applyIntent (vendor-907f1d49045ce1b9b3f2f6d948b40f39.js:47510:21)
    at R (vendor-907f1d49045ce1b9b3f2f6d948b40f39.js:28701:45)
    at r._prepareQueryParams (vendor-907f1d49045ce1b9b3f2f6d948b40f39.js:28260:19)
    at r._doTransition (vendor-907f1d49045ce1b9b3f2f6d948b40f39.js:28219:12)
    at h.transitionTo (vendor-907f1d49045ce1b9b3f2f6d948b40f39.js:27027:37)
    at i.nextPage (vendor-907f1d49045ce1b9b3f2f6d948b40f39.js:81107:23)
    at chunk.21.0381709c1e24f2a2857d.js:2:117294
    at Array.forEach (<anonymous>)

The offending code seems to be at https://github.com/ember-learn/ember-cli-addon-docs/blob/daeaf9f909328ba92b106031b992bb085b96c23d/addon/components/docs-viewer/index.js#L55..L56 and https://github.com/ember-learn/ember-cli-addon-docs/blob/daeaf9f909328ba92b106031b992bb085b96c23d/addon/components/docs-viewer/index.js#L66..L67.

The fix seems fairly easy; If the model on a route is undefined the transitionTo method should not be called with the model argument.

Changing the code to the following fixes it, but I don't know if this might have side effects

const { route, model } = this.docsRoutes.previous;
if (model) {
  this.router.transitionTo(route, model);
} else {
  this.router.transitionTo(route);
}
@RobbieTheWagner
Copy link
Member

@gkappen sorry for the huge delay here! This fix seems reasonable to me. Would you like to open a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants