From 10f045fe6e5bed13b81f9db8d429f2af2cc534e7 Mon Sep 17 00:00:00 2001 From: Cory LaViska Date: Thu, 27 May 2021 16:29:10 -0400 Subject: [PATCH] fix show/hide logic --- docs/resources/changelog.md | 10 ++ src/components/alert/alert.ts | 77 +++++----- src/components/color-picker/color-picker.ts | 12 +- src/components/details/details.ts | 76 +++++----- src/components/dialog/dialog.ts | 145 +++++++++---------- src/components/drawer/drawer.ts | 151 ++++++++++---------- src/components/dropdown/dropdown.ts | 94 ++++++------ src/components/form/form.ts | 18 +-- src/components/icon/library.ts | 2 +- src/components/menu/menu.ts | 4 +- src/components/select/select.ts | 16 +-- src/components/tab-group/tab-group.ts | 4 +- src/components/tooltip/tooltip.ts | 124 ++++++++-------- src/internal/decorators.ts | 2 +- src/internal/event.ts | 15 ++ 15 files changed, 385 insertions(+), 365 deletions(-) create mode 100644 src/internal/event.ts diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md index 46fca49e8..93726944e 100644 --- a/docs/resources/changelog.md +++ b/docs/resources/changelog.md @@ -8,7 +8,17 @@ _During the beta period, these restrictions may be relaxed in the event of a mis ## Next +This release addresses an issue with the `open` prop no longer working in a number of components, as a result of the changes in beta.41. It also removes a small but controversial feature that complicated show/hide logic and led to a poor experience for developers and end users. + +There are two ways to show/hide affected components: by calling `show() | hide()` and by toggling the `open` prop. Previously, it was possible to call `event.preventDefault()` in an `sl-show | sl-hide ` handler to stop the component from showing/hiding. The problem becomes obvious when you set `el.open = false`, the event gets canceled, and in the next cycle `el.open` has reverted to `true`. Not only is this unexpected, but it also doesn't play nicely with frameworks. Additionally, this made it impossible to await `show() | hide()` since there was a chance they'd never resolve. + +Technical reasons aside, canceling these events seldom led to a good user experience, so the decision was made to no longer allow `sl-show | sl-hide` to be cancelable. + +- 🚨 BREAKING: `sl-show` and `sl-hide` events are no longer cancelable - Added Iconoir example to the icon docs +- Changed the `cancelable` default to `false` for the internal `@event` decorator +- Fixed a bug where toggling `open` stopped working in `sl-alert`, `sl-dialog`, `sl-drawer`, `sl-dropdown`, and `sl-tooltip` +- Fixed a number of imports that should have been type imports ## 2.0.0-beta.41 diff --git a/src/components/alert/alert.ts b/src/components/alert/alert.ts index bf1e834c3..3ba8830ca 100644 --- a/src/components/alert/alert.ts +++ b/src/components/alert/alert.ts @@ -3,6 +3,7 @@ import { customElement, property, query } from 'lit/decorators'; import { classMap } from 'lit-html/directives/class-map'; import { animateTo, stopAnimations } from '../../internal/animate'; import { event, EventEmitter, watch } from '../../internal/decorators'; +import { waitForEvent } from '../../internal/event'; import { getAnimation, setDefaultAnimation } from '../../utilities/animation-registry'; import styles from 'sass:./alert.scss'; @@ -51,13 +52,13 @@ export default class SlAlert extends LitElement { */ @property({ type: Number }) duration = Infinity; - /** Emitted when the alert opens. Calling `event.preventDefault()` will prevent it from being opened. */ + /** Emitted when the alert opens. */ @event('sl-show') slShow: EventEmitter; /** Emitted after the alert opens and all transitions are complete. */ @event('sl-after-show') slAfterShow: EventEmitter; - /** Emitted when the alert closes. Calling `event.preventDefault()` will prevent it from being closed. */ + /** Emitted when the alert closes. */ @event('sl-hide') slHide: EventEmitter; /** Emitted after the alert closes and all transitions are complete. */ @@ -74,52 +75,22 @@ export default class SlAlert extends LitElement { /** Shows the alert. */ async show() { - if (!this.hasInitialized || this.open) { - return; - } - - const slShow = this.slShow.emit(); - if (slShow.defaultPrevented) { - this.open = false; + if (this.open) { return; } this.open = true; - - if (this.duration < Infinity) { - this.restartAutoHide(); - } - - await stopAnimations(this.base); - this.base.hidden = false; - const { keyframes, options } = getAnimation(this, 'alert.show'); - await animateTo(this.base, keyframes, options); - - this.slAfterShow.emit(); + return waitForEvent(this, 'sl-after-show'); } /** Hides the alert */ async hide() { - if (!this.hasInitialized || !this.open) { - return; - } - - const slHide = this.slHide.emit(); - if (slHide.defaultPrevented) { - this.open = true; + if (!this.open) { return; } this.open = false; - - clearTimeout(this.autoHideTimeout); - - await stopAnimations(this.base); - const { keyframes, options } = getAnimation(this, 'alert.hide'); - await animateTo(this.base, keyframes, options); - this.base.hidden = true; - - this.slAfterHide.emit(); + return waitForEvent(this, 'sl-after-hide'); } /** @@ -173,8 +144,38 @@ export default class SlAlert extends LitElement { } @watch('open') - handleOpenChange() { - this.open ? this.show() : this.hide(); + async handleOpenChange() { + if (!this.hasInitialized) { + return; + } + + if (this.open) { + // Show + this.slShow.emit(); + + if (this.duration < Infinity) { + this.restartAutoHide(); + } + + await stopAnimations(this.base); + this.base.hidden = false; + const { keyframes, options } = getAnimation(this, 'alert.show'); + await animateTo(this.base, keyframes, options); + + this.slAfterShow.emit(); + } else { + // Hide + this.slHide.emit(); + + clearTimeout(this.autoHideTimeout); + + await stopAnimations(this.base); + const { keyframes, options } = getAnimation(this, 'alert.hide'); + await animateTo(this.base, keyframes, options); + this.base.hidden = true; + + this.slAfterHide.emit(); + } } @watch('duration') diff --git a/src/components/color-picker/color-picker.ts b/src/components/color-picker/color-picker.ts index 4a735a638..91c25c73d 100644 --- a/src/components/color-picker/color-picker.ts +++ b/src/components/color-picker/color-picker.ts @@ -5,8 +5,8 @@ import { ifDefined } from 'lit-html/directives/if-defined'; import { styleMap } from 'lit-html/directives/style-map'; import { event, EventEmitter, watch } from '../../internal/decorators'; import { clamp } from '../../internal/math'; -import SlDropdown from '../dropdown/dropdown'; -import SlInput from '../input/input'; +import type SlDropdown from '../dropdown/dropdown'; +import type SlInput from '../input/input'; import color from 'color'; import styles from 'sass:./color-picker.scss'; @@ -380,12 +380,6 @@ export default class SlColorPicker extends LitElement { } } - handleDropdownShow(event: CustomEvent) { - if (this.disabled) { - event.preventDefault(); - } - } - handleDropdownAfterHide() { this.showCopyFeedback = false; } @@ -774,8 +768,8 @@ export default class SlColorPicker extends LitElement { class="color-dropdown" aria-disabled=${this.disabled ? 'true' : 'false'} .containing-element=${this} + ?disabled=${this.disabled} ?hoist=${this.hoist} - @sl-show=${this.handleDropdownShow} @sl-after-hide=${this.handleDropdownAfterHide} >