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

[Popup] Preserve Should Be Set to False by Default #1369

Closed
Arkni opened this issue Dec 3, 2014 · 8 comments
Closed

[Popup] Preserve Should Be Set to False by Default #1369

Arkni opened this issue Dec 3, 2014 · 8 comments
Milestone

Comments

@Arkni
Copy link

Arkni commented Dec 3, 2014

ISSUE 1

when trying to remove the Popup definitely, that's not working for me and the popup still here.
Here is a fiddle that shows the isssue.
I found an old issue about that, this comment of one of the owners said that :
Calling

$('.ui.button').popup('remove');

will remove the popup from the DOM when you use

$('.ui.button').popup({preserve: true});

But sadly, doesn't work for me :(
What is the right way to definitely remove the popup from the DOM ?

ISSUE 2

Please check this fiddle
For the first time, there is only one popup added to the DOM.
The destroy button is for destroying the popup
According to the same comment , this

$('.ui.button').popup('destroy')

removes all events and disables the popup that's ok, but when you click again on the destroy button this will add a new popup to the DOM.

@Arkni Arkni changed the title Definitely Remove the Popup from the DOM [Popup] Definitely Remove the Popup from the DOM Dec 3, 2014
@jlukic
Copy link
Member

jlukic commented Dec 3, 2014

Sorry, that was a mistake, or was changed since April. I'm not sure which.

The correct behavior is $('.ui.button').popup('remove popup');.

I'll adjust the mistake in the docs as well.

destroy was changed to only remove events if settings: preserve is set to false. You will need to call both functions to remove and destroy now.

This was added because destroy is called when an existing component is re-initialized. Not removing the popup on destroy prevents users from accidentally deleting their popups when initializing an elements twice.

So in brief

$('.ui.button').popup('remove popup').popup('destroy');

@Arkni
Copy link
Author

Arkni commented Dec 3, 2014

@jlukic
Just tested it and it's working as expected Thanks :)

@Arkni Arkni closed this as completed Dec 3, 2014
@Arkni
Copy link
Author

Arkni commented Dec 4, 2014

@jlukic
I don't know why this is not working with the versions 1.0.0 and 1.0.1 or I have to upgrade to upper versions ?

console output
Uncaught TypeError: undefined is not a function semantic.min.js:13
g.removePopup                                   semantic.min.js:13
g.invoke                                        semantic.min.js:13
(anonymous function)                            semantic.min.js:13
m.extend.each                                   jquery.min.js:2
m.fn.m.each                                     jquery.min.js:2
e.fn.popup                                      semantic.min.js:13
(anonymous function)                            example.html:169
m.event.dispatch                                jquery.min.js:3r.handle

@Arkni
Copy link
Author

Arkni commented Dec 4, 2014

Tested with version 1.1.0 and 1.1.2 and it works as expected
So i have to upgrade to versions >= 1.1.0

@startswithaj
Copy link

This has to be documented. As of 2.0.2 a new popup element is added to the body each and every time a popup is initialised on page. They are not longer created and appended to the dom as the user hovers over them. Which is contrary to what the docs say i.e

"Otherwise popups will appended to body and removed after being hidden."

This means if you use them with some dynamic content or angular bindings you can end up with hundreds of orphaned elements in your body.

And as this thread points out calling 'destroy' should but does not remove those elements.

@jlukic
Copy link
Member

jlukic commented Jul 8, 2015

@startswithaj
It looks like the setting preserve was accidentally set to true by default. I probably accidentally did this when debugging layout issues. I'll fix this in the next release. destroy would remove it, but the setting also prevents that too..

if($popup && !settings.preserve) {
   module.removePopup();
}

For now you can just add $.fn.popup.settings.preserve = false; or set it to false in your init.

@jlukic jlukic reopened this Jul 8, 2015
@jlukic jlukic added this to the 2.0.3 milestone Jul 8, 2015
@jlukic jlukic changed the title [Popup] Definitely Remove the Popup from the DOM [Popup] Preserve Should Be Set to False Jul 8, 2015
@jlukic jlukic changed the title [Popup] Preserve Should Be Set to False [Popup] Preserve Should Be Set to False by Default Jul 8, 2015
jlukic added a commit that referenced this issue Jul 8, 2015
@jlukic jlukic closed this as completed Jul 8, 2015
@startswithaj
Copy link

@jlukic hey I appreciate the quick response. While this is in your mind could you clarify something for me.

$('.ui.button').popup('remove popup').popup('destroy'); is this the correct way of 'destroying' a popup or will calling ('destroy') remove it from the dom if the settings a default (after the fix above)?

If this is the case, could we put in in the documentation? In my mind calling destroy should remove any extra dom elements created. I didnt really follow the 'This was added because destroy is called when an existing component is re-initialized. ' logic?

@jlukic
Copy link
Member

jlukic commented Jul 8, 2015

$('.ui.button').popup('destroy'); as long as preserve: false, otherwise as you stated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants