Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

Popup: add a check to not add ui-popover-container if it's already on the page #4661

Closed
frequent opened this issue Jul 9, 2012 · 7 comments
Closed

Popup: add a check to not add ui-popover-container if it's already on the page #4661

frequent opened this issue Jul 9, 2012 · 7 comments
Labels
Component: Popup

Comments

@frequent
Copy link
Contributor

frequent commented Jul 9, 2012

I'm feeding content to a page via AJAX.

Most content blocks contain a few popups, which once pulled into DOM add the ui-popover-container div. All good.

However, once I remove a content section (including the embedded popups), the popover-containers stay in the DOM. Next time I re-loaded the content, the popover containers get added again, so eventually my DOM is mostly popover-containers :-)

I'm now removing them by hand when leaving the page.

A better solution would be for the popover to check if the corresponding container was already there, and if so, don't add it again.
@gabrielschulhof
Copy link

How are you removing them? Are you calling .remove()? popup has code to clean up after itself, including the removal of its container and screen. Is that code running? Put a breakpoint in popup's _destroy() function and see if it gets hit.
@mspisars
Copy link

Running into this issue with custom select menu's.
When a custom select closes we do not remove the ui-popup-container ui-selectmenu-hidden (probably for good reason, i.e. don't want to have to recreate the listview and everything inside the container for the custom select next time the user clicks on the select menu.

Probably would be best to register/tie the ui-popup-container ui-selectmenu-hidden to it's select menu.
This is a problem primarily for dynamically generated pages where portions of the page changes without transitioning to a new page.
@frequent
Copy link
Contributor Author

@gabrielschulhof

Sorry, I missed to answer on this issue. @mspisars is correct, I'm pulling in a lot of dynamic content via Ajax, so for stuff like news messages, I use a shell page and pull in/out blocks of content (including popups). Content blocks are removed using remove(), but - understandably - this will probably not remove any dependent elements outside of the ditched content.

So right now, I feel like uninstalling something from windows and having all sorts of leftover files cluttering the DOM :-D

Just thinking... could elements such as the overlay have a data-dependency-on="some_element_id" to auto-remove once the dependent element is removed (but how to check without wasting resources)? I'm also hoping these things can eventually be made global elements (global overlay, global popup container, global toolbar... just as the loader...).
@gabrielschulhof
Copy link

OK. Popups should be removing themselves correctly, but custom select menus may not be doing likewise because they do not implement _destroy, so the popups they create never get the message.
@frequent
Copy link
Contributor Author

@gabrielschulhof - I haven't updated my JQM version which I'm using popups in, so this was a very early popup widget. Might be this is working now. Let me try to update JQM and see what happens. Then you could close I think.
@mspisars
Copy link

I'm on 1.2.0 (not a latest master build but the released version). It is definitely an issue with custom selects, I do not use popup's outside of custom selects.
gabrielschulhof pushed a commit that referenced this issue Oct 29, 2012
[select] Implement _destroy() -- Fixes #4661
56987c3 
(cherry picked from commit 7273a5e)
@mspisars
Copy link

thank you!
scottjehl pushed a commit that referenced this issue Oct 31, 2012
Merge branch 'master' into tables
ecec15c 
* master: (24 commits)
  [select] Implement _destroy() -- Fixes #4661
  Changing ok out to equal; it is more appropriate for this test
  Changing test to make sure it fails when code change is not there
  Renamed class ui-selectmenu-hidden to ui-popup-hidden -- Fixes #5217
  [popup] Rename function _maybeRefreshTimeout to _expectResizeEvent
  [popup] "detached retina" fix - the window height on iStuff with a retina display seems to fluctuate by one pixel during scroll, causing a spurious resize event right after popup open, even though window size is constant during the entire opening sequence
  Added "mobileinit" handler to set up options
  Whitespace
  Changed urlParseRE to ignore space at beginning. This is expected behavior in browsers. This used to result in pages changing to "%20destination.html" instead of the now "destination.html". Fixes issue #4882
  List autodividers: use trim() to avoid issues with newlines and spaces in markup. Fixes #5197. Thanks @demonslord !
  [popup] Initiate resize expectation during orientationchange -- Fixes #5153
  Select: headers of custom select menus shouldn't inherit the popup corner styling. Fixes #5215.
  [popup] Correctly handle the case when the fallback transition is "none" -- Fixes #5189
  [custom select] Mark as closed after dialog closes - Re: #5195 - Thanks martenbohlin
  [slider] Adding new behaviour to widget definition
  [select] Adding new behaviour to widget definition
  [checkboxradio] Adding new behaviour to widget definition
  [formReset] New behaviour that allows form widgets to become aware of the fact that the form they are in has been reset
  [popup] Only rapid-open the popup if it is open -- Fixes #5157
  Updated third-party libs for Backbone/Require Demo App
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Component: Popup
3 participants
@frequent @gabrielschulhof @mspisars