Fix/SFINT-3352: Ensure internal state is updated first. (#86)

* fix(SFINT-3352): Ensure state transitions are executed after the internal field transitions * fix(SFINT-3352): One more test
coveo · Jul 28, 2020 · 0d3f6ae · 0d3f6ae
1 parent 3c3a73c
commit 0d3f6ae
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 9 deletions.
diff --git a/src/components/ActionButton/StatefulActionButton.ts b/src/components/ActionButton/StatefulActionButton.ts
@@ -93,11 +93,12 @@ export class StatefulActionButton {
             );
             return;
         }
-        this.currentState.onStateExit?.apply(this);
-        state.onStateEntry?.apply(this);
+        const [oldStateExit, newStateEntry] = [this.currentState.onStateExit, state.onStateEntry];
         this.innerActionButton.updateIcon(state.icon);
         this.innerActionButton.updateTooltip(state.tooltip);
         this.currentState = state;
+        oldStateExit?.call(this);
+        newStateEntry?.call(this);
     }
 
     /**

diff --git a/tests/components/ActionButton/StatefulActionButton.spec.ts b/tests/components/ActionButton/StatefulActionButton.spec.ts
@@ -1,19 +1,36 @@
 import { StatefulActionButton, StatefulActionButtonState } from '../../../src/components/ActionButton/StatefulActionButton';
 import { createSandbox, SinonSpy, SinonSandbox } from 'sinon';
+import { ActionButton } from '../../../src/components/ActionButton/ActionButton';
 
 describe('StatefulActionButton', () => {
     let testSubject: StatefulActionButton;
     let sandbox: SinonSandbox;
     let spyConsoleWarn: SinonSpy;
-    const createSpiedState: (stateName: string) => [StatefulActionButtonState, SinonSpy, SinonSpy] = (stateName: string) => {
-        const stateEntrySpy = sandbox.spy();
-        const stateExitSpy = sandbox.spy();
+    const createSpiedState: (
+        stateName: string,
+        onStateEntry?: (this: StatefulActionButton) => void,
+        onStateExit?: (this: StatefulActionButton) => void
+    ) => [StatefulActionButtonState, SinonSpy, SinonSpy] = (
+        stateName: string,
+        onStateEntry?: (this: StatefulActionButton) => void,
+        onStateExit?: (this: StatefulActionButton) => void
+    ) => {
+        const stateEntrySpy = onStateEntry ? sandbox.spy(onStateEntry) : sandbox.spy();
+        const stateExitSpy = onStateExit ? sandbox.spy(onStateExit) : sandbox.spy();
         const state: StatefulActionButtonState = {
             name: stateName,
             icon: 'foo',
             title: 'bar',
-            onStateEntry: stateEntrySpy,
-            onStateExit: stateExitSpy,
+            onStateEntry:
+                onStateEntry ??
+                function () {
+                    stateEntrySpy();
+                },
+            onStateExit:
+                onStateExit ??
+                function () {
+                    stateExitSpy();
+                },
         };
 
         return [state, stateEntrySpy, stateExitSpy];
@@ -127,23 +144,31 @@ describe('StatefulActionButton', () => {
         let targetState: StatefulActionButtonState;
         let onInitialStateExitSpy: SinonSpy;
         let onTargetStateEntrySpy: SinonSpy;
+        let updateIconSpy: SinonSpy;
+        let updateTooltipSpy: SinonSpy;
 
         function expectUnsuccesfulTransition() {
             expect(testSubject.getCurrentState()).toBe(initialState);
             expect(onInitialStateExitSpy.called).toBeFalse();
             expect(onTargetStateEntrySpy.called).toBeFalse();
+            expect(updateIconSpy.called).toBeFalse();
+            expect(updateTooltipSpy.called).toBeFalse();
         }
 
         function expectSuccesfulTransition() {
             expect(spyConsoleWarn.called).toBeFalse();
             expect(testSubject.getCurrentState()).toBe(targetState);
-            expect(onInitialStateExitSpy.called).toBeTrue();
-            expect(onTargetStateEntrySpy.called).toBeTrue();
+            expect(updateIconSpy.called).toBeTrue();
+            expect(updateTooltipSpy.called).toBeTrue();
+            expect(onInitialStateExitSpy.calledAfter(updateTooltipSpy)).toBeTrue();
+            expect(onTargetStateEntrySpy.calledAfter(onInitialStateExitSpy)).toBeTrue();
         }
 
         beforeEach(() => {
             [initialState, , onInitialStateExitSpy] = createSpiedState('initialState');
             [targetState, onTargetStateEntrySpy] = createSpiedState('targetState');
+            updateIconSpy = sandbox.spy(ActionButton.prototype, 'updateIcon');
+            updateTooltipSpy = sandbox.spy(ActionButton.prototype, 'updateTooltip');
         });
 
         describe('if the state given in parameter is not in the options.state', () => {

diff --git a/tests/components/ActionButton/ToggleActionButton.spec.ts b/tests/components/ActionButton/ToggleActionButton.spec.ts
@@ -5,6 +5,7 @@ import * as icons from '../../../src/utils/icons';
 import { ActionButton } from '../../../src/components/ActionButton/ActionButton';
 import { IComponentOptions } from 'coveo-search-ui';
 import { IToggleableButtonOptions } from '../../../src/components/ActionButton/ToggleableButton';
+import { StatefulActionButton } from '../../../src/components/ActionButton/StatefulActionButton';
 
 describe('ToggleActionButton', () => {
     let sandbox: SinonSandbox;
@@ -16,6 +17,7 @@ describe('ToggleActionButton', () => {
     let deactivateSpy: SinonSpy;
     let updateIconSpy: SinonSpy;
     let updateTooltipSpy: SinonSpy;
+    let switchToSpy: SinonSpy;
 
     beforeAll(() => {
         sandbox = createSandbox();
@@ -25,6 +27,7 @@ describe('ToggleActionButton', () => {
         deactivateSpy = sandbox.spy();
         updateIconSpy = sandbox.spy(<any>ActionButton.prototype, 'updateIcon');
         updateTooltipSpy = sandbox.spy(<any>ActionButton.prototype, 'updateTooltip');
+        switchToSpy = sandbox.spy(<any>StatefulActionButton.prototype, 'switchTo');
     });
 
     beforeEach(() => {
@@ -150,5 +153,83 @@ describe('ToggleActionButton', () => {
                 expect(option.alias).toContain(legacy);
             });
         });
+
+        describe(`if activated triggers an event that would activate the button. `, () => {
+            const activateEvent = 'activate-event';
+            beforeEach(() => {
+                const activateWithEvent: (this: ToggleActionButton) => void = function () {
+                    this.element.dispatchEvent(new CustomEvent(activateEvent));
+                };
+                activateSpy = sandbox.spy(activateWithEvent);
+                options.activate = activateWithEvent;
+                testSubject = createToggleButton(options);
+                testSubject.element.addEventListener(activateEvent, () => {
+                    testSubject.setActivated(true);
+                });
+            });
+
+            describe('if already activated', () => {
+                beforeEach(() => {
+                    testSubject.setActivated(true);
+                    sandbox.reset();
+                });
+
+                it('should not call switchTo when setActivated is called with true', () => {
+                    testSubject.setActivated(true);
+                    expect(switchToSpy.called).toBeFalse();
+                });
+            });
+
+            describe('if not activated', () => {
+                beforeEach(() => {
+                    testSubject.setActivated(false);
+                    sandbox.reset();
+                });
+
+                it('should call switchTo only once when setActivated is called with true', () => {
+                    testSubject.setActivated(true);
+                    expect(switchToSpy.calledOnce).toBeTrue();
+                });
+            });
+        });
+
+        describe(`if deactivated triggers an event that would deactivate the button. `, () => {
+            const deactivateEvent = 'deactivate-event';
+            beforeEach(() => {
+                const deactivateWithEvent: (this: ToggleActionButton) => void = function () {
+                    this.element.dispatchEvent(new CustomEvent(deactivateEvent));
+                };
+                activateSpy = sandbox.spy(deactivateWithEvent);
+                options.deactivate = deactivateWithEvent;
+                testSubject = createToggleButton(options);
+                testSubject.element.addEventListener(deactivateEvent, () => {
+                    testSubject.setActivated(false);
+                });
+            });
+
+            describe('if already deactivated', () => {
+                beforeEach(() => {
+                    testSubject.setActivated(false);
+                    sandbox.reset();
+                });
+
+                it('should not call switchTo when setActivated is called with false', () => {
+                    testSubject.setActivated(false);
+                    expect(switchToSpy.called).toBeFalse();
+                });
+            });
+
+            describe('if activated', () => {
+                beforeEach(() => {
+                    testSubject.setActivated(true);
+                    sandbox.reset();
+                });
+
+                it('should call switchTo only once when setActivated is called with false', () => {
+                    testSubject.setActivated(false);
+                    expect(switchToSpy.calledOnce).toBeTrue();
+                });
+            });
+        });
     });
 });