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

Ionic 2: Confirm button with role:cancel does not fire on android back button #7824

Closed
daveshirman opened this issue Aug 22, 2016 · 11 comments
Closed
Assignees
Labels
ionitron: v3 moves the issue to the ionic-v3 repository

Comments

@daveshirman
Copy link

daveshirman commented Aug 22, 2016

Short description of the problem:

When presenting an alert since beta 11, if the user presses the Android hardware back button, the role: 'cancel' handler is not fired.

In previous beta versions, this was fired.

What behavior are you expecting?

For the Android back button to fire the handler with role: 'cancel'

Steps to reproduce:

  1. Put the code below on a page linked to a button
  2. Notice that when pressing hardware back button, alert is not fired.
let confirm = this._alertController.create({
        title: 'Do You agree?',
        message: 'Please say whether you agree',
        buttons: [
          {
            role: 'cancel',
            text: 'Disagree',
            handler: () => {
              alert('cancelled')
            }
          },
          {
            text: 'Agree',
            handler: () => {
            }
          }
        ]
});
confirm.present();

Other information: (e.g. stacktraces, related issues, suggestions how to fix, stackoverflow links, forum links, etc)

Which Ionic Version? 1.x or 2.x
2.x

Plunker that shows an example of your issue

Sorry, but I can't show this in a plunkr as it requires the android hardware button.

Run ionic info from terminal/cmd prompt: (paste output below)
Davids-Air:AppHazard daveshirman$ ionic info

Your system information:

Cordova CLI: 6.2.0
Ionic Framework Version: 2.0.0-beta.11
Ionic CLI Version: 2.0.0-beta.32
Ionic App Lib Version: 2.0.0-beta.18
ios-deploy version: 1.8.6
ios-sim version: 5.0.8
OS: Mac OS X El Capitan
Node Version: v4.3.0
Xcode version: Xcode 7.2.1 Build version 7C1002

@daveshirman
Copy link
Author

daveshirman commented Sep 6, 2016

Hi, any news on this? Anyone?? @jgw96

@daveshirman
Copy link
Author

Hello, can someone from the Ionic team please reply to this? @jgw96

@jgw96
Copy link
Contributor

jgw96 commented Sep 28, 2016

Hello, thanks for opening an issue with us! I am very sorry for the delay on this one, we have been very focused with getting the RC.0 release out. We will be looking at adding this enhancement.

@manucorporat
Copy link
Contributor

manucorporat commented Dec 7, 2016

@daveshirman we are aware of this issue, and I have disabled hardware go back support for Alerts/Actionsheet, since this bug can lead to deadlocks. For example, if the alert needs to resolve a promise, the cancel handler is never called, and the promise is never resolved.

Hardware Go back support has been improved a lot, now it works with modals, tabs, and nested navigation stacks.

Support for Alerts is pending though

@daveshirman
Copy link
Author

@manucorporat Hey, is this really a good solution though? All Android apps use allow the hardware/software back button to dismiss alerts and actionsheets. Isn't this just breaking user expectation and therefore the illusion of a native app?

@manucorporat
Copy link
Contributor

@daveshirman it is not a solution, but it is better to disable a buggy feature that can lead to worst scenarios (like a frozen app).

The fix for this issue is not trivial and we are about to release a new version very soon, it is too later for big changes.

From a UX, you are right. But a Alert is a UI element that is asking for user interaction, an alert WANTS the user to do something, so we think disabling the hardware go back is not a big deal from an UX point of view in the meantime.

@manucorporat manucorporat added this to the 2.0.0-rc.5 milestone Dec 7, 2016
@manucorporat manucorporat self-assigned this Dec 7, 2016
@daveshirman
Copy link
Author

@manucorporat Yep, totally agree. Look forward to the next release, especially navigation fixes (setRoot etc).

Thanks for all your team's hard work. Loving working with this framework.

@fiuzaB
Copy link

fiuzaB commented Feb 20, 2018

Hello ! Any idea when this will be implemented ?

@adityajaiz
Copy link

Is this Issue solved or still open?

@adamdbradley adamdbradley added the ionitron: v3 moves the issue to the ionic-v3 repository label Nov 1, 2018
@imhoffd imhoffd added ionitron: v3 moves the issue to the ionic-v3 repository and removed ionitron: v3 moves the issue to the ionic-v3 repository labels Nov 1, 2018
@imhoffd imhoffd removed the ionitron: v3 moves the issue to the ionic-v3 repository label Nov 28, 2018
@Ionitron Ionitron added the ionitron: v3 moves the issue to the ionic-v3 repository label Nov 28, 2018
@ionitron-bot
Copy link

ionitron-bot bot commented Nov 28, 2018

This issue has been automatically identified as an Ionic 3 issue. We recently moved Ionic 3 to its own repository. I am moving this issue to the repository for Ionic 3. Please track this issue over there.

If I've made a mistake, and if this issue is still relevant to Ionic 4, please let the Ionic Framework team know!

Thank you for using Ionic!

@ionitron-bot
Copy link

ionitron-bot bot commented Nov 28, 2018

Issue moved to: ionic-team/ionic-v3#128

@ionitron-bot ionitron-bot bot closed this as completed Nov 28, 2018
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Nov 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ionitron: v3 moves the issue to the ionic-v3 repository
Projects
None yet
Development

No branches or pull requests

9 participants