Fix borders not always toggling back when exiting preview. Fixes artf/grapesjs#2637

As described by [this issue](artf/grapesjs#2637), the component borders may not be toggled back when exiting preview mode in some cases.

This fixes this issue by adding a `shouldRunSwVisibility` flag to the Preview command, set via the Commands API on its run (before stopping the `sw-visibility` command if it's active), & used on stop to reactivate the `sw-visibility` command if needed.

Which also implies:

- Prevent toggling of component borders while preview is active
- Rely on the Commands API to detect if the `sw-visibility` command is active rather than a panel button
- Prevent resetting the `shouldRunVisibility` flag on multiple subsequent (forced) runs / stops of the preview command

src/commands/view/Preview.js

@@ -18,7 +18,14 @@ export default {
   run(editor, sender) {
     this.sender = sender;
 
-    editor.stopCommand('sw-visibility');
+    if (!this.shouldRunSwVisibility) {
+      this.shouldRunSwVisibility = editor.Commands.isActive('sw-visibility');
+    }
+
+    if (this.shouldRunSwVisibility) {
+      editor.stopCommand('sw-visibility');
+    }
+
     editor.getModel().stopDefault();
 
     const panels = this.getPanels(editor);
@@ -54,13 +61,9 @@ export default {
     sender.set && sender.set('active', 0);
     const panels = this.getPanels(editor);
 
-    const swVisibilityButton = editor.Panels.getButton(
-      'options',
-      'sw-visibility'
-    );
-
-    if (swVisibilityButton && swVisibilityButton.get('active')) {
+    if (this.shouldRunSwVisibility) {
       editor.runCommand('sw-visibility');
+      this.shouldRunSwVisibility = false;
     }
 
     editor.getModel().runDefault();

src/commands/view/SwitchVisibility.js

@@ -8,9 +8,12 @@ export default {
   },
 
   toggleVis(ed, active = 1) {
-    const method = active ? 'add' : 'remove';
-    ed.Canvas.getFrames().forEach(frame => {
-      frame.view.getBody().classList[method](`${this.ppfx}dashed`);
-    });
+    if (!ed.Commands.isActive('preview')) {
+      const method = active ? 'add' : 'remove';
+
+      ed.Canvas.getFrames().forEach(frame => {
+        frame.view.getBody().classList[method](`${this.ppfx}dashed`);
+      });
+    }
   }
 };

test/specs/commands/view/Preview.js

@@ -1,17 +1,17 @@
 import Panel from 'panels/model/Panel';
 import Preview from 'commands/view/Preview';
-import Button from 'panels/model/Button';
 
 describe('Preview command', () => {
-  let fakeButton, fakePanels, fakeEditor;
+  let fakePanels, fakeEditor, fakeIsActive;
 
   beforeEach(() => {
-    fakeButton = new Button();
     fakePanels = [new Panel(), new Panel(), new Panel()];
+    fakeIsActive = false;
 
     fakeEditor = {
       getEl: jest.fn(),
       refresh: jest.fn(),
+      runCommand: jest.fn(),
       stopCommand: jest.fn(),
 
       getModel: jest.fn().mockReturnValue({
@@ -28,13 +28,17 @@ describe('Preview command', () => {
         })
       },
 
+      Commands: {
+        isActive: jest.fn(() => fakeIsActive)
+      },
+
       Panels: {
-        getButton: jest.fn(() => fakeButton),
         getPanels: jest.fn(() => fakePanels)
       }
     };
 
     Preview.panels = undefined;
+    Preview.shouldRunSwVisibility = undefined;
     spyOn(Preview, 'tglPointers');
   });
 
@@ -57,6 +61,24 @@ describe('Preview command', () => {
       Preview.run(fakeEditor);
       fakePanels.forEach(panel => expect(panel.get('visible')).toEqual(false));
     });
+
+    it("should stop the 'sw-visibility' command if active", () => {
+      Preview.run(fakeEditor);
+      expect(fakeEditor.stopCommand).not.toHaveBeenCalled();
+      fakeIsActive = true;
+      Preview.run(fakeEditor);
+      expect(fakeEditor.stopCommand).toHaveBeenCalledWith('sw-visibility');
+    });
+
+    it('should not reset the `shouldRunSwVisibility` state once active if run multiple times', () => {
+      expect(Preview.shouldRunSwVisibility).toBeUndefined();
+      fakeIsActive = true;
+      Preview.run(fakeEditor);
+      expect(Preview.shouldRunSwVisibility).toEqual(true);
+      fakeIsActive = false;
+      Preview.run(fakeEditor);
+      expect(Preview.shouldRunSwVisibility).toEqual(true);
+    });
   });
 
   describe('.stop', () => {
@@ -66,9 +88,13 @@ describe('Preview command', () => {
       fakePanels.forEach(panel => expect(panel.get('visible')).toEqual(true));
     });
 
-    it('should not break when the sw-visibility button is not present', () => {
-      fakeButton = null;
-      expect(() => Preview.stop(fakeEditor)).not.toThrow();
+    it("should run the 'sw-visibility' command if it was active before run", () => {
+      Preview.stop(fakeEditor);
+      expect(fakeEditor.runCommand).not.toHaveBeenCalled();
+      Preview.shouldRunSwVisibility = true;
+      Preview.stop(fakeEditor);
+      expect(fakeEditor.runCommand).toHaveBeenCalledWith('sw-visibility');
+      expect(Preview.shouldRunSwVisibility).toEqual(false);
     });
   });
 });

test/specs/commands/view/SwitchVisibility.js

@@ -0,0 +1,29 @@
+import SwitchVisibility from '../../../../src/commands/view/SwitchVisibility';
+
+describe('SwitchVisibility command', () => {
+  let fakeEditor, fakeFrames, fakeIsActive;
+
+  beforeEach(() => {
+    fakeFrames = [];
+    fakeIsActive = false;
+
+    fakeEditor = {
+      Canvas: {
+        getFrames: jest.fn(() => fakeFrames)
+      },
+
+      Commands: {
+        isActive: jest.fn(() => fakeIsActive)
+      }
+    };
+  });
+
+  describe('.toggleVis', () => {
+    it('should do nothing if the preview command is active', () => {
+      expect(fakeEditor.Canvas.getFrames).not.toHaveBeenCalled();
+      fakeIsActive = true;
+      SwitchVisibility.toggleVis(fakeEditor);
+      expect(fakeEditor.Canvas.getFrames).not.toHaveBeenCalled();
+    });
+  });
+});