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

updates to router.js causing problems with our pageIncompletePrompt extension #1472

Closed
moloko opened this issue Mar 16, 2017 · 2 comments
Closed
Assignees

Comments

@moloko
Copy link
Contributor

moloko commented Mar 16, 2017

PR #1460 has unfortunately stopped our pageIncompletePrompt extension from working

The problem's down to this change in that _isCircularNavigationInProgress never actually gets set to false - it either doesn't exist, or it does but it's set to true. So the if block never gets executed any more.

The quick fix would be to revert this line back to what it was... @brian-learningpool was there any technical reason for this change or did it just seem a more robust check to you?

I'm happy to look at ensuring _isCircularNavigationInProgress is properly defined and always set to either true or false, this probably requires a fair amount more testing though...

@moloko
Copy link
Contributor Author

moloko commented Mar 16, 2017

I also noticed a couple of instances in handleRoute where arguments is being used instead of args - should all instances be updated to args?

@brian-learningpool
Copy link
Member

Hi @moloko, the _isCircularNavigationInProgress was causing problems with the plugin route. I think _isCircularNavigationInProgress, if it should be a boolean, for clarity should be be true or false. I should have ensured that in the original PR.

The other calls in handleRoute referring to arguments could also use args instead.

brian-learningpool added a commit that referenced this issue Mar 17, 2017
Converted _isCircularNavigationInProgress to a boolean on the router object.

Added the pruneArguments() function and tidied up the passing of arguments.

Also corrected some minor indentation issues.
@brian-learningpool brian-learningpool self-assigned this Mar 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants