From 2d148aebcded318c1ba186b35f6c334c97ad1ba7 Mon Sep 17 00:00:00 2001 From: "mathieu.cottret" Date: Fri, 27 Mar 2020 19:12:25 +0100 Subject: [PATCH] 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 | 17 +++++---- src/commands/view/SwitchVisibility.js | 11 ++++-- test/specs/commands/view/Preview.js | 40 ++++++++++++++++---- test/specs/commands/view/SwitchVisibility.js | 29 ++++++++++++++ 4 files changed, 79 insertions(+), 18 deletions(-) create mode 100644 test/specs/commands/view/SwitchVisibility.js diff --git a/src/commands/view/Preview.js b/src/commands/view/Preview.js index 8d244a8ce..cc64f3e92 100644 --- a/src/commands/view/Preview.js +++ b/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(); diff --git a/src/commands/view/SwitchVisibility.js b/src/commands/view/SwitchVisibility.js index 3164c62e0..9b853b3a4 100644 --- a/src/commands/view/SwitchVisibility.js +++ b/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`); + }); + } } }; diff --git a/test/specs/commands/view/Preview.js b/test/specs/commands/view/Preview.js index b09f92c3b..34bb62f9d 100644 --- a/test/specs/commands/view/Preview.js +++ b/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); }); }); }); diff --git a/test/specs/commands/view/SwitchVisibility.js b/test/specs/commands/view/SwitchVisibility.js new file mode 100644 index 000000000..9a3c437c3 --- /dev/null +++ b/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(); + }); + }); +});