Skip to content

fix(router): shouldn't break history when CanDeactivate cancel back - #18135

Closed
jasonaden wants to merge 1 commit into
angular:masterfrom
jasonaden:fix_13586_history_can_deactivate_cancel
Closed

jasonaden wants to merge 1 commit into
angular:masterfrom
jasonaden:fix_13586_history_can_deactivate_cancel

Conversation

@jasonaden

Copy link
Copy Markdown
Contributor

navigation

Rebased from #13922.

Closes #13586

@googlebot

Copy link
Copy Markdown

So there's good news and bad news.

馃憤 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

馃槙 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@jasonaden
jasonaden requested a review from vicb July 14, 2017 20:24
@jasonaden jasonaden added area: router action: review The PR is still awaiting reviews from at least one requested reviewer and removed cla: no labels Jul 14, 2017

@vicb vicb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in #13922 (comment) this does not account for the forward button.

Might be better to fix all in a single PR.

@jibbers42

Copy link
Copy Markdown

@vicb 馃 Is waiting for forward to be fixed really required? Back is used so much more and it looks this was first fixed 6 months ago. Nobody seems too interested in addressing forward.

@mhevery
mhevery force-pushed the fix_13586_history_can_deactivate_cancel branch from 042cae1 to 61e7fda Compare July 26, 2017 17:12
@mary-poppins

Copy link
Copy Markdown

You can preview 61e7fda at https://pr18135-61e7fda.ngbuilds.io/.

@MacKinley

Copy link
Copy Markdown

Anyone working this? Is this being postponed until the forward button issue #15664 is solved as well and put into a single pull request?

@jasonaden

Copy link
Copy Markdown
Contributor Author

Closing this PR as it's not getting approved as-is. We need to create a new PR to fix both issues.

@jasonaden jasonaden closed this Dec 21, 2017
@alfredorevilla

alfredorevilla commented Mar 5, 2018 •

Copy link
Copy Markdown

@jasonaden so are issues still present?

@thanhnhan2tn

Copy link
Copy Markdown

+1

@apostnov

apostnov commented Jan 24, 2019 •

Copy link
Copy Markdown

@jasonaden so are issues still present?

yes, still reproducible in 7.2

@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer area: router cla: no

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deactivation Guard breaks the routing history

10 participants