ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.05k stars 13.51k forks source link

bug: ion-back-button picks wrong route #19030

Open simonhaenisch opened 5 years ago

simonhaenisch commented 5 years ago

Bug Report

Ionic version: [x] 4.x [x] 5.x

Current behavior: I have multiple routes that load the same component with different route props. When clicking the ion-back-button, it goes to a different route for the same component, dropping its route props.

Expected behavior: When clicking the ion-back-button, it should have the same behavior as using the browser's back button and go back to the url of the previous history entry.

Steps to reproduce/Related code: I created a minimal reproduction of this issue on https://github.com/simonhaenisch/ionic-nav-bug, which has been deployed to ionic-nav-bug.vercel.app.

The router looks like this:

<ion-router useHash={false}>
  <ion-route url="/" component="app-home" />
  <ion-route url="/about" component="app-about" />
  <ion-route url="/profile" component="app-profile" />
  <ion-route url="/profile/:name" component="app-profile" />
</ion-router>

To reproduce, do the following:

  1. Go to Ionic's profile page (/profile/ionic).
  2. Go to About page (/about).
  3. Click the ion-back-button (not the browser's back button).

Expected: Goes back to /profile/ionic route and shows Ionic's profile. Actual: It shows Ionic's profile, but the route changes to /profile (drops the "name" route prop).

Other information: In this case, using the browser's back button instead works as expected, because unlike nav.pop it uses the browser's history stack to go back to the previous url. So I guess a work-around for me would be to implement my own back button that simply calls window.history.back().

Sounds like it could (maybe) be related: #18569.

gauravprasadgp commented 4 years ago

seems perfect . any other issues you have ?

simonhaenisch commented 4 years ago

What seems perfect? The bug report or the back button? I don’t actually know if this is solved because we’ve been using my “custom back button with window.history.back()” workaround since then.

gauravprasadgp commented 4 years ago

you could have used navCtrl.pop() operation which allows to pop the page you are in .

simonhaenisch commented 4 years ago

Oh really? Could you explain to me how I can access navCtrl in my Stencil Ionic PWA which uses ion-router?

gauravprasadgp commented 4 years ago

i think you can use <ion-back-button defaultHref="/"></ion-back-button>

simonhaenisch commented 4 years ago

Appreciate you trying to help but could you please have a look at the the repo I made first? I don't think you have understood the bug I'm describing. defaultHref sets the page to go to when there's no other page to go back to in the history anymore, which is completely unrelated.

Given three routes /foo, /foo/:name and /elsewhere: when you go from e. g. /foo/bar to /elsewhere and then click the ion-back-button on the elsewhere page, the route (in the browser bar) will incorrectly change to /foo instead of going back to /foo/bar (even though the page is still rendered with the correct route prop because it's being cached by the Ionic router).

@gp559 if you really want to help: clone my repo, install the @next versions of Ionic and Stencil, and verify whether or not the bug still exists. Steps to reproduce are in the readme.

gauravprasadgp commented 4 years ago

is the issue resolved ?

simonhaenisch commented 4 years ago

@gp559 🤔Issues don't just resolve themselves.

I can confirm that this issue still exists with latest (@ionic/core@4.11.10) as well as next (@ionic/core@5.0.0-rc.2).

@brandyscarney would you mind having a look at this and label it as a bug if you can confirm it?

simonhaenisch commented 3 years ago

I just updated to @ionic/core@5.5.2 and @stencil/core@2.3.0 and this issue still exists. @liamdebeasi I saw you've been working on the router quite a bit, maybe you could have a look at that? Thanks!

simonhaenisch commented 3 years ago

Is there any way that someone takes a look at this before this issue turns a year old, @liamdebeasi? If I don't get a reply in the next couple months, I'll just delete my repro. I don't expect this to make the roadmap but it should at least be out of triage and labelled a bug or something else by now.

Just a hint at what I think might be going wrong: it checks the router for the first URL where the component matches (sth like routes.find(...)) but the same component is registered for two different routes.

liamdebeasi commented 3 years ago

I can reproduce this issue. The problem looks like we are returning too early in the path matching since the issue only reproduces if the /profile/:name route is defined after the /profile route. If the order is reversed, the /profile/:name route works fine, but the /profile route breaks.

I likely will not have time to work on a fix soon, but as a temporary workaround you could have the /profile route redirect to a "default" /profile/:name route. This might not work for every use case, but you can do something like the following:

<ion-route-redirect from="/profile" to="/profile/default" />
<ion-route url="/profile/:name" component="app-profile" />
simonhaenisch commented 3 years ago

Thanks! (: At least it's being tracked now.

The workaround I was currently using is this:

<ion-button onClick={() => window.history.back()}>
    <ion-icon slot="icon-only" name="arrow-back" />
</ion-button>

But that doesn't support defaultHref so I only use it on pages where this route setup (two routes for the same component) is used.

ifcwlme commented 3 years ago

I come up with another workaround that support defaultHref. Use the <ion-back-button> and here is my code with jQuery:

let call_history_back_after_nav_div_change = false;
$(function(){
    $('ion-nav')[0].addEventListener('ionNavDidChange', function(){
        if (call_history_back_after_nav_div_change)
        {
            call_history_back_after_nav_div_change = false;
            window.history.back();
        }
    });
});

customElements.define('page1', class Page1 extends HTMLElement
{
    connectedCallback()
    {
        $(this).find('ion-back-button').on('click', function(){
            const childElementCount = $('ion-nav')[0].childElementCount;
            if (childElementCount > 1)
            {
                call_history_back_after_nav_div_change = true;
            }
        });
    }
}

But the downside is it creates a forward history.