Repository: exadel-inc/esl
Pull request #2117 (2023-12-14 02:13:04)

fix(esl-toggleable/esl-popup): fix toggleable and popup flow


Patch:
diff --git a/src/modules/esl-popup/core/esl-popup.ts b/src/modules/esl-popup/core/esl-popup.ts
index 825aae321..ecfcfd6a4 100644
--- a/src/modules/esl-popup/core/esl-popup.ts
+++ b/src/modules/esl-popup/core/esl-popup.ts
@@ -178,16 +178,9 @@ export class ESLPopup extends ESLToggleable {
   }
 
   /** Runs additional actions on show popup request */
-  protected override onBeforeShow(params: ESLToggleableActionParams): boolean | void {
-    this.activator = params.activator;
-    if (this.open) {
-      this.afterOnHide();
-      this._extraClass = params.extraClass;
-      this._extraStyle = params.extraStyle;
-      this.afterOnShow();
-    }
-
-    if (!params.force && this.open) return false;
+  protected override shouldShow(params: ESLToggleableActionParams): boolean {
+    if (params.activator !== this.activator) return true;
+    return super.shouldShow(params);
   }
 
   /**
@@ -196,6 +189,12 @@ export class ESLPopup extends ESLToggleable {
    * Adds CSS classes, update a11y and fire esl:refresh event by default.
    */
   protected override onShow(params: PopupActionParams): void {
+    if (this.open) {
+      this.beforeOnHide();
+      super.onHide(params);
+      this.afterOnHide();
+    }
+
     super.onShow(params);
 
     // TODO: change flow to use merged params unless attribute state is used in CSS
diff --git a/src/modules/esl-share/core/esl-share-popup.ts b/src/modules/esl-share/core/esl-share-popup.ts
index c652f1725..3b9857f9a 100644
--- a/src/modules/esl-share/core/esl-share-popup.ts
+++ b/src/modules/esl-share/core/esl-share-popup.ts
@@ -4,7 +4,6 @@ import {bind, listen, memoize} from '../../esl-utils/decorators';
 import {ESLShareButton} from './esl-share-button';
 import {ESLShareConfig} from './esl-share-config';
 
-import type {ESLToggleableActionParams} from '../../esl-toggleable/core';
 import type {TooltipActionParams} from '../../esl-tooltip/core/esl-tooltip';
 import type {ESLShareButtonConfig} from './esl-share-config';
 
@@ -57,18 +56,13 @@ export class ESLSharePopup extends ESLTooltip {
   /** Hashstring with a list of buttons already rendered in the popup */
   protected _list: string = '';
 
-  /**
-   * Actions to execute before showing of popup.
-   * @returns false if the show task should be canceled
-   */
-  protected override onBeforeShow(params: ESLToggleableActionParams): boolean | void {
-    const result = super.onBeforeShow(params);
+  public override onShow(params: TooltipActionParams): void {
     if (params.list) {
       const buttonsList = ESLShareConfig.instance.get(params.list);
       this.appendButtonsFromList(buttonsList);
     }
     this.forwardAttributes();
-    return result;
+    super.onShow(params);
   }
 
   /** Checks that the button list from the config was already rendered in the popup. */
diff --git a/src/modules/esl-toggleable/core/esl-toggleable.ts b/src/modules/esl-toggleable/core/esl-toggleable.ts
index 44be3628e..da293ad47 100644
--- a/src/modules/esl-toggleable/core/esl-toggleable.ts
+++ b/src/modules/esl-toggleable/core/esl-toggleable.ts
@@ -220,15 +220,16 @@ export class ESLToggleable extends ESLBaseElement {
 
   /** Actual show task to execute by toggleable task manger ({@link DelayedTask} out of the box) */
   protected showTask(params: ESLToggleableActionParams): void {
-    if (this.onBeforeShow(params) === false) return;
+    if (!this.shouldShow(params)) return;
     if (!params.silent && !this.$$fire(this.BEFORE_SHOW_EVENT, {detail: {params}})) return;
+    this.activator = params.activator;
     this.open = true;
     this.onShow(params);
     if (!params.silent) this.$$fire(this.SHOW_EVENT, {detail: {params}, cancelable: false});
   }
   /** Actual hide task to execute by toggleable task manger ({@link DelayedTask} out of the box) */
   protected hideTask(params: ESLToggleableActionParams): void {
-    if (this.onBeforeHide(params) === false) return;
+    if (!this.shouldHide(params)) return;
     if (!params.silent && !this.$$fire(this.BEFORE_HIDE_EVENT, {detail: {params}})) return;
     this.open = false;
     this.onHide(params);
@@ -240,9 +241,8 @@ export class ESLToggleable extends ESLBaseElement {
    * Actions to execute before showing of toggleable.
    * Returns false if the show action should not be executed.
    */
-  protected onBeforeShow(params: ESLToggleableActionParams): boolean | void {
-    this.activator = params.activator;
-    if (!params.force && this.open) return false;
+  protected shouldShow(params: ESLToggleableActionParams): boolean {
+    return params.force || !this.open;
   }
 
   /**
@@ -266,8 +266,8 @@ export class ESLToggleable extends ESLBaseElement {
    * Actions to execute before hiding of toggleable.
    * Returns false if the hide action should not be executed.
    */
-  protected onBeforeHide(params: ESLToggleableActionParams): boolean | undefined {
-    if (!params.force && !this.open) return false;
+  protected shouldHide(params: ESLToggleableActionParams): boolean {
+    return params.force || this.open;
   }
 
   /**
diff --git a/src/modules/esl-tooltip/core/esl-tooltip.ts b/src/modules/esl-tooltip/core/esl-tooltip.ts
index 772e2670c..fdff16fc3 100644
--- a/src/modules/esl-tooltip/core/esl-tooltip.ts
+++ b/src/modules/esl-tooltip/core/esl-tooltip.ts
@@ -97,7 +97,7 @@ export class ESLTooltip extends ESLPopup {
     }
     this.dir = params.dir || '';
     this.lang = params.lang || '';
-    document.body.appendChild(this);
+    this.parentNode !== document.body && document.body.appendChild(this);
     super.onShow(params);
     this._updateActivatorState(true);
   }
@@ -106,7 +106,7 @@ export class ESLTooltip extends ESLPopup {
   public override onHide(params: TooltipActionParams): void {
     this._updateActivatorState(false);
     super.onHide(params);
-    document.body.removeChild(this);
+    this.parentNode === document.body && document.body.removeChild(this);
   }
 
   /**
@@ -121,6 +121,7 @@ export class ESLTooltip extends ESLPopup {
    * Actions to execute before hiding of popup.
    */
   protected override beforeOnHide(): void {
+    super.beforeOnHide();
     this.activator?.focus({preventScroll: true});
   }
 


Test patch:
diff --git a/src/modules/esl-share/test/esl-share-popup.test.ts b/src/modules/esl-share/test/esl-share-popup.test.ts
index 915bb6236..89501a7b9 100644
--- a/src/modules/esl-share/test/esl-share-popup.test.ts
+++ b/src/modules/esl-share/test/esl-share-popup.test.ts
@@ -11,6 +11,7 @@ import type {ESLShareButton} from '../core/esl-share-button';
 
 describe('ESLSharePopup tests', () => {
   beforeAll(() => {
+    jest.useFakeTimers();
     IntersectionObserverMock.mock();
     ESLShare.register();
   });
@@ -30,6 +31,7 @@ describe('ESLSharePopup tests', () => {
 
     const showPopupAndGetButton = (): ESLShareButton => {
       $trigger.showTarget();
+      jest.advanceTimersByTime(500);
       return document.querySelector('esl-share-button') as ESLShareButton;
     };
 
@@ -47,6 +49,7 @@ describe('ESLSharePopup tests', () => {
       $lvl.forEach(clearShareAttributes);
       clearShareAttributes($trigger);
       ESLSharePopup.sharedInstance.hide();
+      jest.advanceTimersByTime(500);
     });
 
     afterAll(() => {