From 474b73bdbe4aee8200ec33ac598366ad7705226e Mon Sep 17 00:00:00 2001 From: lin onetwo Date: Sat, 6 May 2023 19:19:11 +0800 Subject: [PATCH] Feat: destroy() method for widgets to do custom cleanup (#6699) * feat: inform child widget to do some custom cleanup * fix: type * refactor: restore old removeChildDomNodes * refactor: make destroy() a separate method * refactor: make destroy call removeChildDomNodes * refactor: call destroy instead of removeChildDomNodes in each core widgets * fix: refreshSelf does not mean destroy * refactor: use old var insteadof const * docs: about subclass --- core/modules/storyviews/classic.js | 4 +-- core/modules/storyviews/pop.js | 2 +- core/modules/storyviews/zoomin.js | 2 +- core/modules/widgets/importvariables.js | 2 +- core/modules/widgets/list.js | 4 +-- core/modules/widgets/widget.js | 33 ++++++++++++++++++---- plugins/tiddlywiki/cecily/cecily.js | 2 +- plugins/tiddlywiki/stacked-view/stacked.js | 2 +- 8 files changed, 37 insertions(+), 14 deletions(-) diff --git a/core/modules/storyviews/classic.js b/core/modules/storyviews/classic.js index c2848c435..f7f9ff66b 100644 --- a/core/modules/storyviews/classic.js +++ b/core/modules/storyviews/classic.js @@ -80,7 +80,7 @@ ClassicStoryView.prototype.remove = function(widget) { if(duration) { var targetElement = widget.findFirstDomNode(), removeElement = function() { - widget.removeChildDomNodes(); + widget.destroy(); }; // Abandon if the list entry isn't a DOM element (it might be a text node) if(!targetElement || targetElement.nodeType === Node.TEXT_NODE) { @@ -112,7 +112,7 @@ ClassicStoryView.prototype.remove = function(widget) { {opacity: "0.0"} ]); } else { - widget.removeChildDomNodes(); + widget.destroy(); } }; diff --git a/core/modules/storyviews/pop.js b/core/modules/storyviews/pop.js index e2634e3d5..6d11ed37b 100644 --- a/core/modules/storyviews/pop.js +++ b/core/modules/storyviews/pop.js @@ -73,7 +73,7 @@ PopStoryView.prototype.remove = function(widget) { duration = $tw.utils.getAnimationDuration(), removeElement = function() { if(targetElement && targetElement.parentNode) { - widget.removeChildDomNodes(); + widget.destroy(); } }; // Abandon if the list entry isn't a DOM element (it might be a text node) diff --git a/core/modules/storyviews/zoomin.js b/core/modules/storyviews/zoomin.js index d02f705e7..880663a87 100644 --- a/core/modules/storyviews/zoomin.js +++ b/core/modules/storyviews/zoomin.js @@ -154,7 +154,7 @@ ZoominListView.prototype.remove = function(widget) { var targetElement = widget.findFirstDomNode(), duration = $tw.utils.getAnimationDuration(), removeElement = function() { - widget.removeChildDomNodes(); + widget.destroy(); }; // Abandon if the list entry isn't a DOM element (it might be a text node) if(!targetElement || targetElement.nodeType === Node.TEXT_NODE) { diff --git a/core/modules/widgets/importvariables.js b/core/modules/widgets/importvariables.js index aafc8ba8b..5d32f5ffb 100644 --- a/core/modules/widgets/importvariables.js +++ b/core/modules/widgets/importvariables.js @@ -122,7 +122,7 @@ ImportVariablesWidget.prototype.refresh = function(changedTiddlers) { } if(changedAttributes.filter || !$tw.utils.isArrayEqual(this.tiddlerList,tiddlerList) || haveListedTiddlersChanged()) { // Compute the filter - this.removeChildDomNodes(); + this.destroy(); this.execute(tiddlerList); this.renderChildren(this.parentDomNode,this.findNextSiblingDomNode()); return true; diff --git a/core/modules/widgets/list.js b/core/modules/widgets/list.js index 41344a02e..8c69367dc 100755 --- a/core/modules/widgets/list.js +++ b/core/modules/widgets/list.js @@ -219,7 +219,7 @@ ListWidget.prototype.handleListChanges = function(changedTiddlers) { } else { // If the list was empty then we need to remove the empty message if(prevList.length === 0) { - this.removeChildDomNodes(); + this.destroy(); this.children = []; } // If we are providing an counter variable then we must refresh the items, otherwise we can rearrange them @@ -312,7 +312,7 @@ ListWidget.prototype.removeListItem = function(index) { if(this.storyview && this.storyview.remove) { this.storyview.remove(widget); } else { - widget.removeChildDomNodes(); + widget.destroy(); } // Remove the child widget this.children.splice(index,1); diff --git a/core/modules/widgets/widget.js b/core/modules/widgets/widget.js index 741914fdc..def5c63c2 100755 --- a/core/modules/widgets/widget.js +++ b/core/modules/widgets/widget.js @@ -688,7 +688,7 @@ Rebuild a previously rendered widget */ Widget.prototype.refreshSelf = function() { var nextSibling = this.findNextSiblingDomNode(); - this.removeChildDomNodes(); + this.removeChildDomNodes({ recursive: true }); this.render(this.parentDomNode,nextSibling); }; @@ -753,19 +753,42 @@ Widget.prototype.findFirstDomNode = function() { /* Remove any DOM nodes created by this widget or its children */ -Widget.prototype.removeChildDomNodes = function() { - // If this widget has directly created DOM nodes, delete them and exit. This assumes that any child widgets are contained within the created DOM nodes, which would normally be the case +Widget.prototype.removeChildDomNodes = function(options) { + var recursive = options && options.recursive; + /** + * If this widget has directly created DOM nodes, delete them and exit. + * This assumes that any child widgets are contained within the created DOM nodes, which would normally be the case + */ if(this.domNodes.length > 0) { $tw.utils.each(this.domNodes,function(domNode) { domNode.parentNode.removeChild(domNode); }); this.domNodes = []; - } else { + return true; + } else if(recursive) { // Otherwise, ask the child widgets to delete their DOM nodes $tw.utils.each(this.children,function(childWidget) { - childWidget.removeChildDomNodes(); + childWidget.removeChildDomNodes(options); }); } + return false +}; + +/* +Inform widget subclass that extends this widget and children widgets of this widget. Let them know this widget tree is about to destroy, and dom nodes are being unmounted from the document. +*/ +Widget.prototype.destroy = function(options) { + // removeDom by default + var removeDom = (options && options.removeDom) || true; + if (removeDom) { + // prepare options for children, if we have removed the dom, child don't need to remove their dom + removeDom = !this.removeChildDomNodes(); + } + // nothing need to do, as dom is already removed in the removeChildDomNodes + // we just need to inform the children + $tw.utils.each(this.children,function(childWidget) { + childWidget.destroy({ removeDom: removeDom }); + }); }; /* diff --git a/plugins/tiddlywiki/cecily/cecily.js b/plugins/tiddlywiki/cecily/cecily.js index 9eec05fd4..86f16ca1e 100644 --- a/plugins/tiddlywiki/cecily/cecily.js +++ b/plugins/tiddlywiki/cecily/cecily.js @@ -62,7 +62,7 @@ CecilyStoryView.prototype.remove = function(widget) { duration = $tw.utils.getAnimationDuration(); // Remove the widget at the end of the transition setTimeout(function() { - widget.removeChildDomNodes(); + widget.destroy(); },duration); // Animate the closure $tw.utils.setStyle(targetElement,[ diff --git a/plugins/tiddlywiki/stacked-view/stacked.js b/plugins/tiddlywiki/stacked-view/stacked.js index 32d30d6d5..e886f3d7a 100644 --- a/plugins/tiddlywiki/stacked-view/stacked.js +++ b/plugins/tiddlywiki/stacked-view/stacked.js @@ -80,7 +80,7 @@ StackedListView.prototype.insert = function(widget) { }; StackedListView.prototype.remove = function(widget) { - widget.removeChildDomNodes(); + widget.destroy(); }; exports.stacked = StackedListView;