From c023e55da877a03e2378e6685d86839bcecb7dd1 Mon Sep 17 00:00:00 2001
From: "ala'n (Alexey Stsefanovich)" <astsefanovich@exadel.com>
Date: Thu, 14 Dec 2023 02:34:17 +0100
Subject: [PATCH] fix(esl-toggleable): rework actions pre-checks to fix flow on
 reopening toggleable

BREAKING CHANGE: `onBeforeShow` and `onBeforeHide` have retired. The constraint now inside `shouldShow`/`shouldHide` methods,
activator change now is the part of main togglable flow
---
 src/modules/esl-popup/core/esl-popup.ts       | 19 +++++++++----------
 src/modules/esl-share/core/esl-share-popup.ts | 10 ++--------
 .../esl-toggleable/core/esl-toggleable.ts     | 14 +++++++-------
 3 files changed, 18 insertions(+), 25 deletions(-)

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;
   }
 
   /**