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.03k stars 13.51k forks source link

bug: Should be able to cancel back button action from onClick function #24938

Closed kevinclarkadstech closed 2 years ago

kevinclarkadstech commented 2 years ago

Prerequisites

Ionic Framework Version

Current Behavior

When I have a form on a page, I want to ask the user if they would like to navigate back BEFORE actually going back. I.e. with code:

`

this.handleBackClicked(evt)}>
        <ion-buttons slot='end'>
            <ion-button color='primary' fill='clear' disabled={this.validationErrors.length > 0} onClick={() => this.handleSaveClicked()}>Save</ion-button>
        </ion-buttons>
    </ion-toolbar></ion-header>`

and

private async handleBackClicked(evt: MouseEvent) { evt.stopImmediatePropagation(); evt.preventDefault(); if (this.hasUnsavedUserChanges) { alertController.create({ message: 'You may have some unsaved changes. Are you sure you want to cancel?', buttons: [ { text: 'No' }, { text: 'Yes', handler: () => { const ionRouter = document.querySelector('ion-router'); if (ionRouter) { ionRouter.back(); } } } ] }).then(alert => alert.present()); } else { const ionRouter = document.querySelector('ion-router'); if (ionRouter) { ionRouter.back(); } } }

Unfortunately, the back action happens right away and stopping propogation or preventing default does not work.

However, this DOES work on a normal ion-button, i.e.

<ion-button fill="clear" href={/profile/${authenticatedUser.value?.username}} onClick={async (evt) => { if (!authenticatedUser.value) { evt.preventDefault(); evt.stopImmediatePropagation(); const modal = await modalController.create({ component: 'sign-up-modal' }); await modal.present(); } }}>

Expected Behavior

The navigation/animation does not happen and the back action is canceled programatically.

Steps to Reproduce

Create a simple page with above code as I have it and you will see it navigate back, THEN pop up the alert (or pop it up simultaneously).

Code Reproduction URL

No response

Ionic Info

Ionic:

Ionic CLI : 6.18.1 (C:\Users\Kevin\AppData\Roaming\npm\node_modules\@ionic\cli) Ionic Framework : not installed

Capacitor:

Capacitor CLI : not installed @capacitor/android : not installed @capacitor/core : 3.3.1 @capacitor/ios : not installed

Utility:

cordova-res : not installed globally native-run : not installed globally

System:

NodeJS : v16.13.1 (C:\Program Files\nodejs\node.exe) npm : 8.1.2 OS : Windows 10

Additional Information

No response

ionitron-bot[bot] commented 2 years ago

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.

kevinclarkadstech commented 2 years ago

Please see https://github.com/kevinclarkadstech/ionicbackbuttondemo

liamdebeasi commented 2 years ago

Thanks. The problem here is that the onClick handler that does the routing happens before your app's onClick handler, so doing stopPropagation() will have no effect: https://github.com/ionic-team/ionic-framework/blob/main/core/src/components/back-button/back-button.tsx#L117

The routing guards in ion-router were designed for this use case, and I encourage you to use those instead:

<ion-app>
  <ion-router useHash={false}>
    <ion-route url="/home" component="app-home" beforeLeave={() => {
      // You can also present an alert and require confirmation before routing
      return false;
    }} />
    <ion-route url="/regular" component="regular-button-page" />
    <ion-route url="/back" component="back-button-page" />
    <ion-route url="/profile/:name" component="app-profile" />
  </ion-router>
  <ion-nav />
</ion-app>

Note: You may need to make your root URL be /home instead of / in case your browser does a redirect from '' to '/'. You can also add some special logic to check for that edge case if you need the base URL to be /.

See https://ionicframework.com/docs/api/route#navigation-hooks for more information. Can you try that and let me know if it resolves your issue?

kevinclarkadstech commented 2 years ago

Thanks @liamdebeasi . I still think that is a bug and seems kind of backwards. The hooks on the route are okay, but then it would require having some kind of global app state to check whether the form is dirty....not ideal.

liamdebeasi commented 2 years ago

We do not intend for ion-back-button to be used in this way as there are multiple ways of routing away from the page. For example, users could press the browser back button instead of the ion-back-button to navigate away.

For temporary data views, I recommend using a modal instead. We have a new feature in ion-modal coming that will let you prevent modals from being dismissed. This new canDismiss property might be something worth looking at when it ships in an upcoming minor release of Ionic. You would have full control over when a modal can dismiss to prevent accidental data loss without needing to have a global form state.

At the moment we do not have any plans to change how ion-back-button functions, though I do encourage you to check out canDismiss when it ships. Here is the PR if you are interested in looking at the code and usage examples: https://github.com/ionic-team/ionic-framework/pull/24928

I am going to close this. Thanks!

ionitron-bot[bot] commented 2 years ago

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.