From b7e317a5de5895b2bac2acb2590e31b7a039c746 Mon Sep 17 00:00:00 2001 From: Jack Lukic Date: Tue, 17 May 2016 18:20:23 -0700 Subject: [PATCH] Fix popup to use observeChanges instead of autoremove --- RELEASE-NOTES.md | 2 +- src/definitions/modules/popup.js | 92 ++++++++++++++++---------------- 2 files changed, 48 insertions(+), 46 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 8783a545d..3cc0cfa80 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -7,7 +7,7 @@ - **Webpack** - Modified all relative paths in project to prefix with `./` to make them webpack friendly (wont be misinterpreted as module) - **Form Validation** - Added ability for field validation to depend on other fields being filled out - **Popup** - Added new setting `boundary` and `scrollContext`. `boundary` lets you specify an element that the popup will try to position itself to be contained inside of. `scrollContext` lets you specify the element which when scrolled should hide the popup -- **Popup** - Added new settings `autoRemove`, which is enabled by default. This will add special event listeners to auto hide a popup if the triggering element is removed from the DOM. This is useful in controlled DOM environments like Meteor/Ember/React to ensure a popup auto-hides itself when a page navigation or other DOM change occurs that may not trigger `mouseout`. +- **Popup** - Added new settings `observeChanges`, which is enabled by default. This will add special mutation observers to trigger `destroy` when the element is removed from the document, preventing memory leaks. - **Dropdown** - Dropdown now changes user selection on keyboard shortcuts **immediately**, this will save the extra `enter` key press to confirm selection in most cases. To enable previous pre `2.2` selection style use the setting `selectOnKeydown: false` NEEDS DOCS - **Dropdown** - Multiple select dropdown now sizes current dropdown input based on rendered width of a hidden element, not using an estimate based on character count. This means search will never break to a second line earlier than would normally fit in current line. - **Button** - Added compatibility with `primary` `secondary` `positive` `negative` buttons with the `basic` styling variation. #3756 diff --git a/src/definitions/modules/popup.js b/src/definitions/modules/popup.js index 12b6e4619..d19e2aef7 100644 --- a/src/definitions/modules/popup.js +++ b/src/definitions/modules/popup.js @@ -73,6 +73,7 @@ $.fn.popup = function(parameters) { element = this, instance = $module.data(moduleNamespace), + contextObserver, elementNamespace, id, module @@ -88,6 +89,7 @@ $.fn.popup = function(parameters) { if(!module.exists() && settings.preserve) { module.create(); } + module.observeChanges(); module.instantiate(); }, @@ -99,6 +101,18 @@ $.fn.popup = function(parameters) { ; }, + observeChanges: function() { + if('MutationObserver' in window) { + contextObserver = new MutationObserver(module.event.contextChanged); + contextObserver.observe($context[0], { + childList : true, + subtree : true + }); + module.debug('Setting up mutation observer', contextObserver); + } + }, + + refresh: function() { if(settings.popup) { $popup = $(settings.popup).eq(0); @@ -153,9 +167,9 @@ $.fn.popup = function(parameters) { clearTimeout(module.hideTimer); clearTimeout(module.showTimer); // remove events - $window.off(elementNamespace); + module.unbind.close(); + module.unbind.events(); $module - .off(eventNamespace) .removeData(moduleNamespace) ; }, @@ -190,6 +204,18 @@ $.fn.popup = function(parameters) { module.set.position(); } }, + contextMutation: function(mutations) { + [].forEach.call(mutations, function(mutation) { + if(mutation.removedNodes) { + [].forEach.call(mutation.removedNodes, function(node) { + if(node == element || $(node).find(element).length > 0) { + module.debug('Element removed from DOM, tearing down events'); + module.destroy(); + } + }); + } + }); + }, hideGracefully: function(event) { var $target = $(event.target), @@ -271,7 +297,7 @@ $.fn.popup = function(parameters) { }, createID: function() { - id = (Math.random().toString(16) + '000000000').substr(2,8); + id = (Math.random().toString(16) + '000000000').substr(2, 8); elementNamespace = '.' + id; module.verbose('Creating unique id for element', id); }, @@ -335,7 +361,7 @@ $.fn.popup = function(parameters) { .each(function() { $(this) .data(metadata.activator) - .popup('hide') + .popup('hide') ; }) ; @@ -394,9 +420,6 @@ $.fn.popup = function(parameters) { callback = $.isFunction(callback) ? callback : function(){}; if(settings.transition && $.fn.transition !== undefined && $module.transition('is supported')) { module.set.visible(); - if(settings.autoRemove) { - module.bind.autoRemoval(); - } $popup .transition({ animation : settings.transition + ' in', @@ -435,9 +458,6 @@ $.fn.popup = function(parameters) { module.reset(); callback.call($popup, element); settings.onHidden.call($popup, element); - if(settings.autoRemove) { - module.unbind.autoRemoval(); - } } }) ; @@ -959,15 +979,6 @@ $.fn.popup = function(parameters) { ; } }, - autoRemoval: function() { - $module - .one('remove' + eventNamespace, function() { - module.hide(function() { - module.removePopup(); - }); - }) - ; - }, close: function() { if(settings.hideOnScroll === true || (settings.hideOnScroll == 'auto' && settings.on != 'click')) { $scrollContext @@ -996,31 +1007,22 @@ $.fn.popup = function(parameters) { }, unbind: { + events: function() { + $window + .off(elementNamespace) + ; + $module + .off(eventNamespace) + ; + }, close: function() { - if(settings.hideOnScroll === true || (settings.hideOnScroll == 'auto' && settings.on != 'click')) { - $document - .off('scroll' + elementNamespace, module.hide) - ; - $context - .off('scroll' + elementNamespace, module.hide) - ; - } - if(settings.on == 'hover' && openedWithTouch) { - $document - .off('touchstart' + elementNamespace) - ; - openedWithTouch = false; - } - if(settings.on == 'click' && settings.closable) { - module.verbose('Removing close event from document'); - $document - .off('click' + elementNamespace) - ; - } + $document + .off(elementNamespace) + ; + $scrollContext + .off(elementNamespace) + ; }, - autoRemoval: function() { - $module.off('remove' + elementNamespace); - } }, has: { @@ -1276,6 +1278,9 @@ $.fn.popup.settings = { performance : true, namespace : 'popup', + // whether it should use dom mutation observers + observeChanges : true, + // callback only when element added to dom onCreate : function(){}, @@ -1297,9 +1302,6 @@ $.fn.popup.settings = { // callback after hide animation onHidden : function(){}, - // hides popup when triggering element is destroyed - autoRemove : true, - // when to show popup on : 'hover',