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

fix(angular): transition animation plays when using browser back and forward buttons #28188

Closed
wants to merge 12 commits into from
Closed
4 changes: 2 additions & 2 deletions packages/angular/src/providers/nav-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ export class NavController {
if (router) {
router.events.subscribe((ev) => {
if (ev instanceof NavigationStart) {
// restoredState is set if the browser back/forward button is used
const id = ev.restoredState ? ev.restoredState.navigationId : ev.id;
this.guessDirection = id < this.lastNavId ? 'back' : 'forward';
this.guessAnimation = !ev.restoredState ? this.guessDirection : undefined;
this.guessDirection = this.guessAnimation = id < this.lastNavId ? 'back' : 'forward';
this.lastNavId = this.guessDirection === 'forward' ? ev.id : id;
}
});
Expand Down
5 changes: 3 additions & 2 deletions packages/angular/test/base/e2e/src/router-link.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ function testForward() {
cy.get('app-router-link-page #canGoBack').should('have.text', 'true');

cy.go('back');
cy.wait(500);
Copy link
Contributor

@liamdebeasi liamdebeasi Oct 11, 2023

Choose a reason for hiding this comment

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

Are the timeout changes here necessary? If not, could we revert them? (Sorry, I meant to comment on this with my last round of reviews, but it slipped my mind)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also if possible, could you allow me to push changes to your branch? I can resolve the merge conflicts for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think, it is necessary. Before, the back transition was instant and now it takes the time for the animation which is why we need the additional wait
I invited you to my fork. Thanks for taking care of the merge conflicts.

cy.testStack('ion-router-outlet', ['app-router-link']);
cy.testLifeCycle('app-router-link', {
ionViewWillEnter: 2,
Expand All @@ -159,7 +160,7 @@ function testRoot() {
cy.get('app-router-link-page #canGoBack').should('have.text', 'false');

cy.go('back');
cy.wait(100);
cy.wait(500);
cy.testStack('ion-router-outlet', ['app-router-link']);
cy.testLifeCycle('app-router-link', {
ionViewWillEnter: 1,
Expand All @@ -181,7 +182,7 @@ function testBack() {
cy.get('app-router-link-page #canGoBack').should('have.text', 'false');

cy.go('back');
cy.wait(100);
cy.wait(500);
cy.testStack('ion-router-outlet', ['app-router-link']);
cy.testLifeCycle('app-router-link', {
ionViewWillEnter: 1,
Expand Down