fix(popup-menu): remove container on close

Simplifies open behavior in the process. No need to keep the old (previous) open state around.
bpmn-io · Nov 25, 2022 · e1df3ed · e1df3ed
1 parent 86e0b1d
commit e1df3ed
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 11 deletions.
diff --git a/lib/features/popup-menu/PopupMenu.js b/lib/features/popup-menu/PopupMenu.js
@@ -47,7 +47,8 @@ var DEFAULT_PRIORITY = 1000;
 export default function PopupMenu(config, eventBus, canvas) {
   this._eventBus = eventBus;
   this._canvas = canvas;
-  this._current = {};
+
+  this._current = null;
 
   var scale = isDefined(config && config.scale) ? config.scale : {
     min: 1,
@@ -60,7 +61,7 @@ export default function PopupMenu(config, eventBus, canvas) {
 
 
   eventBus.on('diagram.destroy', () => {
-    this._destroy();
+    this.close();
   });
 
   eventBus.on('element.changed', event => {
@@ -209,11 +210,15 @@ PopupMenu.prototype.close = function() {
 
   this.reset();
 
-  this._current.container = null;
+  this._current = null;
 };
 
 PopupMenu.prototype.reset = function() {
-  render(null, this._current.container);
+  const container = this._current.container;
+
+  render(null, container);
+
+  domRemove(container);
 };
 
 PopupMenu.prototype._emit = function(event, payload) {
@@ -251,11 +256,6 @@ PopupMenu.prototype._unbindAutoClose = function() {
 };
 
 
-PopupMenu.prototype._destroy = function() {
-  this._current.container && domRemove(this._current.container);
-};
-
-
 /**
  * Updates popup style.transform with respect to the config and zoom level.
  *
@@ -475,7 +475,7 @@ PopupMenu.prototype._getHeaderEntries = function(element, providers) {
  * @return {boolean} true if open
  */
 PopupMenu.prototype.isOpen = function() {
-  return !!this._current.container;
+  return !!this._current;
 };
 
 

diff --git a/test/spec/features/popup-menu/PopupMenuSpec.js b/test/spec/features/popup-menu/PopupMenuSpec.js
@@ -440,7 +440,6 @@ describe('features/popup-menu', function() {
       it('should display group name if provided', inject(function(popupMenu) {
 
         // given
-        var container = getPopupContainer(popupMenu);
         popupMenu.registerProvider('group-menu', {
           getEntries: function() {
             return [
@@ -454,6 +453,7 @@ describe('features/popup-menu', function() {
         // when
         popupMenu.open({}, 'group-menu', { x: 100, y: 100 });
 
+        const container = getPopupContainer(popupMenu);
         const entryHeaders = domQueryAll('.entry-header', container);
 
         // then
@@ -559,6 +559,28 @@ describe('features/popup-menu', function() {
     }));
 
 
+    it('should remove container', inject(function(popupMenu, eventBus) {
+
+      // given
+      var closeSpy = sinon.spy();
+      var container = popupMenu._current.container;
+
+      eventBus.on('popupMenu.close', closeSpy);
+
+      // assume
+      expect(container.parentNode).to.exist;
+
+      // when
+      popupMenu.close();
+
+      // then
+      expect(popupMenu.isOpen()).to.be.false;
+
+      expect(closeSpy).to.have.been.calledOnce;
+      expect(container.parentNode).not.to.exist;
+    }));
+
+
     it('should not fail if already closed', inject(function(popupMenu) {
 
       // when