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: Multiple popover have all the same scope since beta.14 #3173

Closed
kristianbenoit opened this issue Feb 24, 2015 · 22 comments
Closed

bug: Multiple popover have all the same scope since beta.14 #3173

kristianbenoit opened this issue Feb 24, 2015 · 22 comments

Comments

@kristianbenoit
Copy link

Type: bug

Platform: all

If there is multiple popover in the same page, for example in a list, they all get the same scope.

The problem is not present when using beta13, but is in beta.14 and rc.0.

Remove one popover and you can't open the other ones ... (reopen the same one)

Here's a pen that show the bug.:
http://codepen.io/kbenoit/pen/OPZoeV

@kristianbenoit
Copy link
Author

Here's a workaround, it implies creating the popover just in time and removing it when it is hidden:
http://codepen.io/kbenoit/pen/emrPdr

The problem is that popover act as a singleton, all popover have reference to the last one created. That's why instantiating the popover just in time works.

@alejandromagnorsky
Copy link

+1. Was quite hard to detect what was going on exactly.

@kristianbenoit kristianbenoit changed the title bug: Multiple popover have all the same scope in beta14 bug: Multiple popover have all the same scope since beta.14 Mar 13, 2015
@Fayozjon
Copy link

Alejandro

http://noanavaro.de/youtube_range

13.03.2015, 02:26, "Alejandro Magnorsky" notifications@github.com:+1. Was quite hard to detect what was going on exactly.

—Reply to this email directly or .

@sirhoe
Copy link

sirhoe commented Apr 8, 2015

+1, Please look into this.

@alejandromagnorsky
Copy link

I tried again @kristianbenoit solution, but now is not even working in his codepen if we update to ionic.rc.3.
Please, this was working before until beta.14 was released...

@nutboltu
Copy link

+1

@kristianbenoit
Copy link
Author

I confirm the workaround I proposed does not work anymore in 1.0.0-rc.3 and nightly.

@YongdeCHEN
Copy link

+1, Someone know the solution for the bug?

@kristianbenoit
Copy link
Author

@YongdeCHEN, so far use my work around with ionic-1.0.0-rc.2

@alejandromagnorsky
Copy link

@brodykidd, this is the issue I mentioned in the email ;)
Just in case you need to update the status.

@ghost
Copy link

ghost commented May 21, 2015

+1

@liyo
Copy link

liyo commented Jun 26, 2015

could this issue also be related? http://codepen.io/anon/pen/yNPmvL

@kristianbenoit
Copy link
Author

@liyo, that's the same problem. Using the solution I provided above should fix your problem (create the popover in the ng-click handler).

@alejandromagnorsky
Copy link

@kristianbenoit, what about version 1.0.0? Your solution doesn't work with the "stable" version, right?
Thanks anyway.

@alejandromagnorsky
Copy link

@kristianbenoit, actually I tried your method with the latest version and it worked! Great, thanks.

@liyo
Copy link

liyo commented Jun 30, 2015

@kristianbenoit thank you, I was able to make it work with the latest Ionic version and your method. To avoid an infinite loop when popovers were removed on hide (because for some reason popover.hidden is still firing on remove), I had to manually remove the popover backdrop DOM elements manually, see: http://codepen.io/liyo/pen/XbZreg

@kristianbenoit
Copy link
Author

@alejandromagnorsky, as @liyo says, there is an issue involving an infinite loop in the previous solution, where we call popover.remove while on "popover.hidden", at this point the popover is not hidden yet so on "popover.hidden" is called again. Solution, call stopListening() before popover.remove.

$scope.$on('popover.hidden', function() {
      stopListening();
      popover.remove();
});

Here's a pen with a working solution.

http://codepen.io/kbenoit/pen/BNVomX

@kristianbenoit
Copy link
Author

@liyo, the pen you provided, does not remove the popover (it's leaking DOM elements). Did you forget to click save ?

@liyo
Copy link

liyo commented Jul 13, 2015

@kristianbenoit yes indeed I forgot to save, I have updated it now just for reference.
Anyway, your new solution works great and there is no need to do a manual DOM removal of popover elements. Thank you!

@digitaluprising
Copy link

This works great. However, the weird thing is that it doesn't appear to work in ionic view, but it works in ionic serve, the emulator, and when I deployed it natively to my iOS device.

@digitaluprising
Copy link

Nevermind. Solution works fine. I realized that my reference to my template was wrong. I named it popOver.html and didn't realize that my URL was pointing to popover.html... 😢

@shaoner
Copy link

shaoner commented Nov 11, 2015

Edit:
I've just had the same issue.
It seems to be related on how the options are handled in $ionicPopover.fromTemplateUrl.

return $ionicModal.fromTemplateUrl(url, ionic.Utils.extend(POPOVER_OPTIONS, options || {}));

So multiple calls to $ionicPopover.fromTemplateUrl overwrites POPOVER_OPTIONS.

It works better using something like this:

return $ionicModal.fromTemplateUrl(url, ionic.Utils.extend({}, POPOVER_OPTIONS, options));

@mlynch mlynch closed this as completed in 44460d7 Dec 6, 2015
mlynch added a commit that referenced this issue Dec 6, 2015
Prevent the popover options to be overwritten (fix #3173)
@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
Development

No branches or pull requests

9 participants