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

bug: incorrect nav animation when using browser back and forward button to navigate through multiple pages #28307

Open
3 tasks done
hoi4 opened this issue Oct 9, 2023 · 8 comments
Labels
package: angular @ionic/angular package type: bug a confirmed bug report

Comments

@hoi4
Copy link
Contributor

hoi4 commented Oct 9, 2023

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

Sometimes the nav animation is incorrect when using the browser back and forward buttons to navigate through multiple pages.

Expected Behavior

The nav animation direction should be correct based on the internal navigation stack.

Steps to Reproduce

The following video demonstrates the issue and serves as reproduction steps:

Screen.Recording.2023-10-06.at.08.29.22.mov
  1. clone the ionic framework repository and apply the necessary build steps to provide the ng16 test project
  2. open the tabs page
  3. navigate into pages of tab1 twice (see video)
  4. use the browser back button twice (back to the tab's root page)
  5. use the browser forward button twice --> The second animation is incorrectly a "back" instead of a "forward" animation.

Code Reproduction URL

No response

Ionic Info

Ionic:

   Ionic CLI                     : 7.1.1
   Ionic Framework               : @ionic/angular 7.4.0
   @angular-devkit/build-angular : 16.2.2
   @angular-devkit/schematics    : 16.2.2
   @angular/cli                  : 16.2.2
   @ionic/angular-toolkit        : 9.0.0

System:

   NodeJS : v18.17.1 (/Users/philipp/.nvm/versions/node/v18.17.1/bin/node)
   npm    : 9.6.7
   OS     : macOS Unknown (--> Sonoma)

Additional Information

No response

@thetaPC
Copy link
Contributor

thetaPC commented Dec 4, 2023

Thank you so much for submitting the issue!

I've been trying to reproduce the issue locally, but unfortunately, I'm having some difficulty. Please provide a minimal reproduction, I recommend using the Starters project and provide a GitHub repo link or a StackBlitz link. You can also provide the build (as mentioned in the description) from Framework Angular.

@thetaPC thetaPC added the ionitron: needs reproduction a code reproduction is needed from the issue author label Dec 4, 2023
Copy link

ionitron-bot bot commented Dec 4, 2023

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

@ionitron-bot ionitron-bot bot removed the triage label Dec 4, 2023
@hoi4
Copy link
Contributor Author

hoi4 commented Dec 7, 2023

@thetaPC Thanks for your answer. I'm not very sure what else I can provide. Did you follow the steps I provided above with internal test project? :)

@thetaPC
Copy link
Contributor

thetaPC commented Dec 8, 2023

@hoi4 Yes, thank you for the steps! I gave them a go, but unfortunately, my team member and I couldn't recreate the animations during browser navigation. I've attached a video showing what we observed. Any chance you could share some extra steps to guide us in reproducing the issue? Appreciate your help!

Screen.Recording.2023-12-08.at.1.08.13.PM.mov

@hoi4
Copy link
Contributor Author

hoi4 commented Dec 9, 2023

I see that this is confusing. Here is the background to the issue and the linked PR:

  • I originally created this MR to fix the issue that the nav animation is not applied when using the browser back and forward button
  • As the original PR included the fix for the nav animation as well as the fix for the incorrect nav animation (described in this ticket), @liamdebeasi asked me to split up the PR into two separate ones (and create a separate issue for the second part of the original PR)
  • The fix to apply the nav animation when using the browser back button has been merged, but will only be released in the next major release. (It is also included in the linked PR to fix the issue described above)
  • To reproduce the original behavior you can check out my branch and just set the useDirectionBasedOnStackOrder variable in the StackController to false.

Hope that helps :)

@averyjohnston averyjohnston added triage and removed ionitron: needs reproduction a code reproduction is needed from the issue author labels Dec 11, 2023
@thetaPC
Copy link
Contributor

thetaPC commented Dec 13, 2023

@hoi4 Thank you for the reply!

The challenge that we are facing is we are unable to reproduce your issue/recording when checking out a version of the Ionic Framework repository prior to the fix for the nav animation.

To appropriately validate and review your proposed pull request, we require reproducing the issue against the latest version of Ionic Framework. We are currently not able to do that.

Without testing against a modified version of Ionic Framework, is there a minimal reproduction or specific testing steps that we can follow to validate this bug and to verify your fix against?

@thetaPC thetaPC added the needs: reply the issue needs a response from the user label Dec 13, 2023
@ionitron-bot ionitron-bot bot removed the triage label Dec 13, 2023
@hoi4
Copy link
Contributor Author

hoi4 commented Dec 14, 2023

As the latest version of Ionic does not apply the nav animation when using the browser back / forward buttons, it is not possible to use the latest version to validate my improvement against.
This is only possible when using the feature-8.0 branch (which includes the changes from #28530 )
When using the feature-8.0 branch, it should be possible to follow the steps in the video :)

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Dec 14, 2023
@thetaPC
Copy link
Contributor

thetaPC commented Jan 16, 2024

Thank you again for the response! I was able to confirm that this is a bug that is displayed in feature-8.0 branch.

@thetaPC thetaPC added the type: bug a confirmed bug report label Jan 16, 2024
@ionitron-bot ionitron-bot bot removed the triage label Jan 16, 2024
@thetaPC thetaPC added the package: angular @ionic/angular package label Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: angular @ionic/angular package type: bug a confirmed bug report
Projects
None yet
Development

No branches or pull requests

3 participants