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: vue, react, ion-back-button does not always select correct route when going back #22830

Closed
sheamusburns opened this issue Jan 28, 2021 · 11 comments · Fixed by #22974
Closed
Labels
package: react @ionic/react package package: vue @ionic/vue package type: bug a confirmed bug report
Milestone

Comments

@sheamusburns
Copy link

sheamusburns commented Jan 28, 2021

Bug Report

Ionic version:

[ ] 4.x
[x] 5.x

Current behavior:
The back button behavior works for most straightforward cases, but depending upon the sequence of the history, sometimes the route gets replaced instead of the router.back() getting called.

Expected behavior:
router should go back() if an the available history entry is applicable.

Steps to reproduce:
clone and run test application, follow replication steps:

run ionic serve

  1. visit root at localhost:8100
  • You should be redirected to /A
  • The back button at the top of the page is set to be disabled if the $route.fullPath === history.state.back. (If you're navigating from page to page and never visiting the same page in a row, then we should expect the current path and the next back path to never be the same)
  1. Use the Page buttons to navigate through the following sequence (not limited to this sequence)
  • Page B, Page C, Page B, Page A, Page B, Page C
  1. Press Back Button
  2. Route Should be /B and back button should be enabled
  3. Press Back Button

Expected

  • Route should be /A and back button should be enabled

Actual

  • Route is /A, but back button is disabled

Related

Ionic Back Button Issue with explanation: https://github.com/sheamusburns/issue-ionic-router-back-button
Related Form Post: https://forum.ionicframework.com/t/vue-router-history-not-updating-history-state-back-after-ion-back-button-default-click-event/203468

Other information:
N/A

Ionic info:

Ionic:

   Ionic CLI       : 6.12.3 (/Users/sheamusburns/.nvm/versions/node/v12.16.0/lib/node_modules/@ionic/cli)
   Ionic Framework : @ionic/vue 5.5.2

Capacitor:

   Capacitor CLI   : 2.4.6
   @capacitor/core : 2.4.6

Utility:

   cordova-res : 0.15.2
   native-run  : not installed

System:

   NodeJS : v12.16.0 (/Users/sheamusburns/.nvm/versions/node/v12.16.0/bin/node)
   npm    : 6.14.11
   OS     : macOS Big Sur
@ionitron-bot ionitron-bot bot added the triage label Jan 28, 2021
sheamusburns added a commit to sheamusburns/ionic-framework that referenced this issue Jan 28, 2021
@sheamusburns
Copy link
Author

Will write some tests for this and make PR

@sheamusburns sheamusburns changed the title bug: default ion-back-button behavior replaces route in some circumstances when it should actually go back() bug: (Ionic Vue-Router) default ion-back-button behavior replaces route in some circumstances when it should actually go back() Jan 28, 2021
@liamdebeasi
Copy link
Contributor

Thanks for the issue. I took a look at this and the issue seems to be with your usage of ion-back-button. ion-back-button should be used inside of each page and the framework will automatically determine whether or not the back button should be enabled.

Replace the template in App.vue with the following:

<ion-app>
  <ion-router-outlet id="main-content"></ion-router-outlet>
</ion-app>

Add the following above your ion-content in page.vue:

<ion-header>
  <ion-toolbar>
    <ion-buttons slot="start">
      <ion-back-button></ion-back-button>
    </ion-buttons>
  </ion-toolbar>
</ion-header>

I tried doing those two things and I was no longer able to reproduce the behavior that was described in this issue (the back button was always visible/active when going back). Using the ion-back-button as a global component and outside of ion-router-outlet is not something we currently support. I am going to close this issue, thanks!

@sheamusburns
Copy link
Author

@liamdebeasi well that's fun.
While you say it's not something that's currently supported, I'm not sure that's clear anywhere.
Can you point me to the place in the documentation that makes clear the ion-back-button in only intended to be used inside ion-page?
Maybe that's a gap?

@liamdebeasi
Copy link
Contributor

Yep, I can add some docs for this.

@sheamusburns
Copy link
Author

sheamusburns commented Jan 28, 2021

hey @liamdebeasi
I just tested with the changes you suggested and while the back button state is not an issue, the core problematic behavior this bug is about is still there.

Button sequence:
B, C, B, A, B, C

Watch the URL bar as you press back.
Actual:
/B, /A, /A, /B, /C, /C, /B

Expected:
/B, /A, /B, /C, /B

The router is still calling router.replace() when it should be calling router.back() on the second and fifth click back.

@liamdebeasi
Copy link
Contributor

Ah sorry - I was looking at the enabled/disabled back button part of the issue. I can reproduce this - let me take a closer look at this.

@sheamusburns
Copy link
Author

sheamusburns commented Jan 28, 2021

NP. @liamdebeasi
I'm sure you already know exactly where to look, but code problem is:

// @ionic/vue-router#index.js:221-226
if (routeInfo.lastPathname === routeInfo.pushedByRoute) {
                    router.back();
                }
                else {
                    router.replace({ path: prevInfo.pathname, query: parseQuery(prevInfo.search) });
                }

I've found this to be due to routeInfo.lastPathname not always being what you'd expect because of the sequence.

this change resolves it at least in this scenario (no idea how it impacts tabbed or nested nav)

// @ionic/vue-router#index.js:221-226
if (routeInfo.lastPathname === routeInfo.pushedByRoute || prevInfo.pathname === routeInfo.pushedByRoute) {
                    router.back();
                }
                else {
                    router.replace({ path: prevInfo.pathname, query: parseQuery(prevInfo.search) });
                }

@liamdebeasi
Copy link
Contributor

Ok yeah it looks like the actual navigation behavior is a bug in Ionic Vue. In theory this is also a bug in Ionic React, but I need to test that further.

@liamdebeasi liamdebeasi reopened this Jan 28, 2021
@liamdebeasi liamdebeasi changed the title bug: (Ionic Vue-Router) default ion-back-button behavior replaces route in some circumstances when it should actually go back() bug: ionic vue, ion-back-button does not always select correct route when going back Jan 28, 2021
@liamdebeasi liamdebeasi added package: vue @ionic/vue package type: bug a confirmed bug report labels Jan 28, 2021
@ionitron-bot ionitron-bot bot removed the triage label Jan 28, 2021
@liamdebeasi liamdebeasi added this to the 5.6.0 milestone Jan 28, 2021
@sheamusburns
Copy link
Author

k. thanks @liamdebeasi let me know if there's anything I can do to help.

@liamdebeasi liamdebeasi changed the title bug: ionic vue, ion-back-button does not always select correct route when going back bug: vue, react, ion-back-button does not always select correct route when going back Feb 25, 2021
@liamdebeasi liamdebeasi added the package: react @ionic/react package label Feb 25, 2021
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #22974, and a fix will be available in an upcoming release of Ionic Framework.

@ionitron-bot
Copy link

ionitron-bot bot commented Mar 27, 2021

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Mar 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: react @ionic/react package package: vue @ionic/vue package type: bug a confirmed bug report
Projects
None yet
2 participants