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: isHighest causing problems with consecutive popups in v1.0.1 #4061

Closed
raphaklaus opened this issue Jul 8, 2015 · 6 comments
Closed

Comments

@raphaklaus
Copy link

Type: bug

Platform: all

Hello!

I'm facing a problem related to consecutive popups. You can simulate it through:
http://codepen.io/anon/pen/XbqWwB

Click on the button Test,then click save, the second popup will open, click save again. Now, try to click on the button Test again. It seems only the highest popup got closed, and the first popup remains in the DOM, but invisible.

This issue didn't happen in 1.0.0

Here is the problem line:
8095b12#diff-e91afb9d089c4a023017992e5bf71f3eR45574

I appreciate any attention over this issue.

@raphaklaus raphaklaus changed the title isHighest causing problems with consecutive popups in v1.0.1 bug: isHighest causing problems with consecutive popups in v1.0.1 Jul 8, 2015
zittix added a commit to zittix/ionic that referenced this issue Jul 21, 2015
Make sure to close and remove the currently shown popup before showing the previous one. Closes ionic-team#4061
@vnagasivam
Copy link

👍

@mhartington
Copy link
Contributor

So ionic is just getting confused, trying to show the second popup while also trying to hide the first one.
You could do this

http://codepen.io/mhartington/pen/zGeJog?editors=001

Or this

http://codepen.io/mhartington/pen/JdxaOG?editors=001

@mhartington mhartington added the needs: reply the issue needs a response from the user label Aug 6, 2015
@raphaklaus
Copy link
Author

Okay, tested on v1.0.1, and it works using this approach.

I'd suggest to include an explanation in $ionicPopup documentation. Something like:

onTap callback should not call other popup instance, if necessary, it must be called inside then block.

@adamdbradley adamdbradley removed the needs: reply the issue needs a response from the user label Aug 6, 2015
@mhartington
Copy link
Contributor

Will make sure it gets in there. Thanks

@olliebennett
Copy link

We have experienced the above problem in a couple of situations, not all with valid workarounds:

  • One of our custom $httpProvider.interceptors detects 500 errors and displays an $ionicPopup.alert informing user that a "Server Error" occurred. In some cases, multiple API requests may return 500s in quick succession. In such cases, only one of the popups can be closed. Our workaround is to use $ionicActionSheet.show and store the returned hideActionSheet function (globally), to be called before creating any new $ionicActionSheet. From a design/UI perspective, this isn't a valid "workaround".
  • We display an $ionicPopup.confirm dialogue when a date input field changes (with debounce). There are situations where the (debounced) change occurs while an existing confirm dialogue is displayed. (Our situation is slightly more complex than this - I can share code if required). In this case, there is also no way to hide the second popup.

Do you think we could re-open this issue, or else I could create a separate one? It appears to me that the recent changes introduce various openings for race conditions, not all of which can be averted.

Links: Comparison of 1.0.0 with 1.0.1 and the "guilty" commit: bcfe210

@zarko-tg
Copy link

For whoever might be looking for a proper way to show (two) popups one after another, you could have a look at this demo as well: http://codepen.io/zarko/full/jPJqeb/
And yes, at least the docs should say something about the limitation as probably most developers are not even aware of this change that might come around to bite them.

zittix added a commit to zittix/ionic that referenced this issue Aug 12, 2015
Make sure to close and remove the currently shown popup before showing the previous one. Closes ionic-team#4061
zittix added a commit to zittix/ionic that referenced this issue Aug 12, 2015
Make sure to close and remove the currently shown popup before showing the previous one. Closes ionic-team#4061
zittix added a commit to zittix/ionic that referenced this issue Aug 12, 2015
Make sure to close and remove the currently shown popup before showing the previous one. Closes ionic-team#4061
mhartington pushed a commit that referenced this issue Oct 27, 2015
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants