Semantic-ui: Detachable modal could lead to DOM leaks if not disposed properly

Created on 28 Mar 2015 · 7Comments · Source: Semantic-Org/Semantic-UI

If a modal is initialised with a _detachable_ setting set to true (which is the default value) then the following code gets executed inside module.create.dimmer (modal.js):

if(settings.detachable) {
    module.verbose('Modal is detachable, moving content into dimmer');
    $dimmable.dimmer('add content', $module);
}

The modal DOM is moved to the (a.k.a. the $dimmable) and calling its 'destroy method' leaves it there.

In applications with client side routing (such as Angular apps) proper component disposal is crucial. On route change one would expect that calling 'destroy' on all initialized Semantic modules in the current view is enough to properly dispose them. The router will then remove all DOM elements from the current view and initialize the new one. However, because our module is moved from the partial view to the it won't be removed by the router. Furthermore, subsequent navigations to the same page will spawn multiple copies of the same modal DOM elements inside the

<body class="dimmable">
    <div class="view partials go in here">...</div>
    <div class="ui dimmer modals page">
        <div id="leaked-modal" class="ui small modal">...</div>
        <div id="leaked-modal" class="ui small modal">...</div>
        <div id="leaked-modal" class="ui small modal">...</div>
        ...
    </div>
</body>

This is not an issue with Semantic itself, but is a common pitfall in single page applications that use Semantic. If additional steps are required for a proper module disposal (such as cleaning moved to the body DOM elements, removing window event listeners, etc) this should be clear from the documentation without the need to understand the internal implementation of the module.

Single page applications are widely used nowadays and from my experience Semantic is the best UI framework to fit in these situations. So if full module disposal is not always possible using the 'destroy' behaviour this should be explicitly noted in the _Usage_ section of the documentation.

Docs Issue

Source

revov

Most helpful comment

When running up against this in an Angular SPA that leverages ui-router, a solution that may work for you is to run some cleanup via jQuery before every view load:

$rootScope.$on('$viewContentLoading', 
  function(event, viewConfig) {
    $('.ui.modal').remove();
  }
);

So before every view load, all semantic-ui modal DOM elements will be removed from the DOM entirely, and the regular Angular view DOM stuff will rebuild it as expected when the view renders

alex-sherwin on 2 Mar 2016

👍3

All 7 comments

You can use detachable: false to make sure content is not moved. Generally though destroy only removes events and not the associated dom element.

jlukic on 13 Apr 2015

I agree - changing the behaviour of destroy is pointless.

The disposal of a module is just as important in a single page application as is its initialization. Having a dedicated section in the documentation that provides notes on how to properly destroy a module will favour Semantic UI when choosing a UI framework for a single page app.

If there are implementation specific side effects of a module such as creating or moving DOM nodes outside of the element the module was initialized on, leaving event handlers after 'destroy', etc - these should be noted in the documentation. I know that this sounds cosmetic and is certainly not a priority but it is extremely important in scalable single page apps.

@jlukic you can close this issue if there is nothing else to discuss. Semantic already does a good job on cleaning up after itself with the destroy method. I just want to raise awareness of the importance of proper component cleanup and that sometimes additional steps need to be taken (and it is difficult to find these if you are not familiar with the implementation specifics).

revov on 13 Apr 2015

I agree this is one of those sore points that I've has come up a few times with developers since modal behaves a bit funny with regards to assumptions about structure. Sidebar also has the same issues of expected markup.

I'll try to make a note in the docs on modal.

jlukic on 14 Apr 2015

I think the note in usage is now fairly clear.

jlukic on 27 May 2015

When running up against this in an Angular SPA that leverages ui-router, a solution that may work for you is to run some cleanup via jQuery before every view load:

$rootScope.$on('$viewContentLoading', 
  function(event, viewConfig) {
    $('.ui.modal').remove();
  }
);

So before every view load, all semantic-ui modal DOM elements will be removed from the DOM entirely, and the regular Angular view DOM stuff will rebuild it as expected when the view renders

alex-sherwin on 2 Mar 2016

👍3

alex-sherwin remove elements in that way is a very hacky way of doing it, if you read carefully the discussion you realize that they already give the solution, just change the setting of the modal with:

$('.ui.modal').modal({detachable: false})

With that the modal will stay in his place and Angular will take care of the rest.

RuVT on 3 Mar 2016

@RuVT That's not necessarily true.

That was the first thing I tried, and depending on your application, apparently, the modal ends up rendering behind the dimmer, which is no good.

Debugged that for awhile and came to the conclusion I would have to hack CSS z-index's to overcome it.

This may very well be a bug in semantic-ui, but it was a far cleaner solution for this case, since it's a very predictable behavior and end's up using the semantic-ui default preference for {detachable: true}

alex-sherwin on 3 Mar 2016

Was this page helpful?

0 / 5 - 0 ratings