Skip to content

Commit

Permalink
fix(esl-toggleable): rework actions pre-checks to fix flow on reopeni…
Browse files Browse the repository at this point in the history
…ng 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
  • Loading branch information
ala-n committed Dec 14, 2023
1 parent 9c25137 commit c023e55
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 25 deletions.
19 changes: 9 additions & 10 deletions src/modules/esl-popup/core/esl-popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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
Expand Down
10 changes: 2 additions & 8 deletions src/modules/esl-share/core/esl-share-popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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. */
Expand Down
14 changes: 7 additions & 7 deletions src/modules/esl-toggleable/core/esl-toggleable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

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

/**
Expand Down

0 comments on commit c023e55

Please sign in to comment.