diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md index e69b5786c..130386b07 100644 --- a/docs/resources/changelog.md +++ b/docs/resources/changelog.md @@ -23,6 +23,8 @@ This release includes a complete rewrite of `` to improve accessibili - Added Traditional Chinese translation [#1086](https://github.com/shoelace-style/shoelace/pull/1086) - Fixed a bug in `` where the checked/indeterminate states could get out of sync when using the `multiple` option [#1076](https://github.com/shoelace-style/shoelace/issues/1076) - Fixed a bug in `` that caused `sl-selection-change` to emit before the DOM updated [#1096](https://github.com/shoelace-style/shoelace/issues/1096) +- Reorganized all components to make class structures more consistent +- Updated non-public fields to use the `private` keyword (these were previously private only by convention, but now TypeScript will warn you) - Updated the hover style of `` to be consistent with `` - Updated the status of `` and `` from experimental to stable - Updated React wrappers to use the latest API from `@lit-labs/react` [#1090](https://github.com/shoelace-style/shoelace/pull/1090) diff --git a/docs/resources/contributing.md b/docs/resources/contributing.md index 18a768e52..ccf1be3b6 100644 --- a/docs/resources/contributing.md +++ b/docs/resources/contributing.md @@ -200,6 +200,23 @@ All components have a host element, which is a reference to the `` element Aside from `display`, avoid setting styles on the host element when possible. The reason for this is that styles applied to the host element are not encapsulated. Instead, create a base element that wraps the component's internals and style that instead. This convention also makes it easier to use BEM in components, as the base element can serve as the "block" entity. +When authoring components, please try to follow the same structure and conventions found in other components. Classes, for example, generally follow this structure: + +- Static properties/methods +- Private/public properties (that are _not_ reactive) +- `@query` decorators +- `@state` decorators +- `@property` decorators +- Lifecycle methods (`connectedCallback()`, `disconnectedCallback()`, `firstUpdated()`, etc.) +- Private methods +- `@watch` decorators +- Public methods +- The `render()` method + +Please avoid using the `public` keyword for class fields. It's simply too verbose when combined with decorators, property names, and arguments. However, _please do_ add `private` in front of any property or method that is intended to be private. + +?> This might seem like a lot, but it's fairly intuitive once you start working with the library. However, don't let this structure prevent you from submitting a PR. [Code can change](https://www.abeautifulsite.net/posts/code-can-change/) and nobody will chastise you for "getting it wrong." At the same time, encouraging consistency helps keep the library maintainable and easy for others to understand. (A lint rule that helps with things like this would be a very welcome PR!) + ### Class Names All components use a [shadow DOM](https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_shadow_DOM), so styles are completely encapsulated from the rest of the document. As a result, class names used _inside_ a component won't conflict with class names _outside_ the component, so we're free to name them anything we want. diff --git a/src/components/alert/alert.ts b/src/components/alert/alert.ts index 9c06f1603..961f98966 100644 --- a/src/components/alert/alert.ts +++ b/src/components/alert/alert.ts @@ -73,6 +73,57 @@ export default class SlAlert extends ShoelaceElement { this.base.hidden = !this.open; } + private restartAutoHide() { + clearTimeout(this.autoHideTimeout); + if (this.open && this.duration < Infinity) { + this.autoHideTimeout = window.setTimeout(() => this.hide(), this.duration); + } + } + + private handleCloseClick() { + this.hide(); + } + + private handleMouseMove() { + this.restartAutoHide(); + } + + @watch('open', { waitUntilFirstUpdate: true }) + async handleOpenChange() { + if (this.open) { + // Show + this.emit('sl-show'); + + if (this.duration < Infinity) { + this.restartAutoHide(); + } + + await stopAnimations(this.base); + this.base.hidden = false; + const { keyframes, options } = getAnimation(this, 'alert.show', { dir: this.localize.dir() }); + await animateTo(this.base, keyframes, options); + + this.emit('sl-after-show'); + } else { + // Hide + this.emit('sl-hide'); + + clearTimeout(this.autoHideTimeout); + + await stopAnimations(this.base); + const { keyframes, options } = getAnimation(this, 'alert.hide', { dir: this.localize.dir() }); + await animateTo(this.base, keyframes, options); + this.base.hidden = true; + + this.emit('sl-after-hide'); + } + } + + @watch('duration') + handleDurationChange() { + this.restartAutoHide(); + } + /** Shows the alert. */ async show() { if (this.open) { @@ -129,57 +180,6 @@ export default class SlAlert extends ShoelaceElement { }); } - restartAutoHide() { - clearTimeout(this.autoHideTimeout); - if (this.open && this.duration < Infinity) { - this.autoHideTimeout = window.setTimeout(() => this.hide(), this.duration); - } - } - - handleCloseClick() { - this.hide(); - } - - handleMouseMove() { - this.restartAutoHide(); - } - - @watch('open', { waitUntilFirstUpdate: true }) - async handleOpenChange() { - if (this.open) { - // Show - this.emit('sl-show'); - - if (this.duration < Infinity) { - this.restartAutoHide(); - } - - await stopAnimations(this.base); - this.base.hidden = false; - const { keyframes, options } = getAnimation(this, 'alert.show', { dir: this.localize.dir() }); - await animateTo(this.base, keyframes, options); - - this.emit('sl-after-show'); - } else { - // Hide - this.emit('sl-hide'); - - clearTimeout(this.autoHideTimeout); - - await stopAnimations(this.base); - const { keyframes, options } = getAnimation(this, 'alert.hide', { dir: this.localize.dir() }); - await animateTo(this.base, keyframes, options); - this.base.hidden = true; - - this.emit('sl-after-hide'); - } - } - - @watch('duration') - handleDurationChange() { - this.restartAutoHide(); - } - render() { return html`
>)[this.name]; const slot = await this.defaultSlot; @@ -196,7 +152,7 @@ export default class SlAnimation extends ShoelaceElement { return true; } - destroyAnimation() { + private destroyAnimation() { if (this.animation) { this.animation.cancel(); this.animation.removeEventListener('cancel', this.handleAnimationCancel); @@ -205,6 +161,50 @@ export default class SlAnimation extends ShoelaceElement { } } + @watch('name') + @watch('delay') + @watch('direction') + @watch('duration') + @watch('easing') + @watch('endDelay') + @watch('fill') + @watch('iterations') + @watch('iterationsStart') + @watch('keyframes') + handleAnimationChange() { + if (!this.hasUpdated) { + return; + } + + this.createAnimation(); + } + + @watch('play') + handlePlayChange() { + if (this.animation) { + if (this.play && !this.hasStarted) { + this.hasStarted = true; + this.emit('sl-start'); + } + + if (this.play) { + this.animation.play(); + } else { + this.animation.pause(); + } + + return true; + } + return false; + } + + @watch('playbackRate') + handlePlaybackRateChange() { + if (this.animation) { + this.animation.playbackRate = this.playbackRate; + } + } + /** Clears all keyframe effects caused by this animation and aborts its playback. */ cancel() { this.animation?.cancel(); diff --git a/src/components/breadcrumb/breadcrumb.ts b/src/components/breadcrumb/breadcrumb.ts index ccf9d6eb1..7f30a238e 100644 --- a/src/components/breadcrumb/breadcrumb.ts +++ b/src/components/breadcrumb/breadcrumb.ts @@ -24,12 +24,12 @@ import type { CSSResultGroup } from 'lit'; export default class SlBreadcrumb extends ShoelaceElement { static styles: CSSResultGroup = styles; - @query('slot') defaultSlot: HTMLSlotElement; - @query('slot[name="separator"]') separatorSlot: HTMLSlotElement; - private readonly localize = new LocalizeController(this); private separatorDir = this.localize.dir(); + @query('slot') defaultSlot: HTMLSlotElement; + @query('slot[name="separator"]') separatorSlot: HTMLSlotElement; + /** * The label to use for the breadcrumb control. This will not be shown on the screen, but it will be announced by * screen readers and other assistive devices to provide more context for users. @@ -49,7 +49,7 @@ export default class SlBreadcrumb extends ShoelaceElement { return clone; } - handleSlotChange() { + private handleSlotChange() { const items = [...this.defaultSlot.assignedElements({ flatten: true })].filter( item => item.tagName.toLowerCase() === 'sl-breadcrumb-item' ) as SlBreadcrumbItem[]; diff --git a/src/components/button-group/button-group.ts b/src/components/button-group/button-group.ts index 1d89cc658..ef26022b6 100644 --- a/src/components/button-group/button-group.ts +++ b/src/components/button-group/button-group.ts @@ -28,27 +28,27 @@ export default class SlButtonGroup extends ShoelaceElement { */ @property() label = ''; - handleFocus(event: CustomEvent) { + private handleFocus(event: CustomEvent) { const button = findButton(event.target as HTMLElement); button?.classList.add('sl-button-group__button--focus'); } - handleBlur(event: CustomEvent) { + private handleBlur(event: CustomEvent) { const button = findButton(event.target as HTMLElement); button?.classList.remove('sl-button-group__button--focus'); } - handleMouseOver(event: CustomEvent) { + private handleMouseOver(event: CustomEvent) { const button = findButton(event.target as HTMLElement); button?.classList.add('sl-button-group__button--hover'); } - handleMouseOut(event: CustomEvent) { + private handleMouseOut(event: CustomEvent) { const button = findButton(event.target as HTMLElement); button?.classList.remove('sl-button-group__button--hover'); } - handleSlotChange() { + private handleSlotChange() { const slottedElements = [...this.defaultSlot.assignedElements({ flatten: true })] as HTMLElement[]; slottedElements.forEach(el => { diff --git a/src/components/button/button.ts b/src/components/button/button.ts index 0ddf1ab47..c6e948f48 100644 --- a/src/components/button/button.ts +++ b/src/components/button/button.ts @@ -39,8 +39,6 @@ import type { CSSResultGroup } from 'lit'; export default class SlButton extends ShoelaceElement implements ShoelaceFormControl { static styles: CSSResultGroup = styles; - @query('.button') button: HTMLButtonElement | HTMLLinkElement; - private readonly formSubmitController = new FormSubmitController(this, { form: input => { // Buttons support a form attribute that points to an arbitrary form, so if this attribute it set we need to query @@ -58,6 +56,8 @@ export default class SlButton extends ShoelaceElement implements ShoelaceFormCon private readonly hasSlotController = new HasSlotController(this, '[default]', 'prefix', 'suffix'); private readonly localize = new LocalizeController(this); + @query('.button') button: HTMLButtonElement | HTMLLinkElement; + @state() private hasFocus = false; @state() invalid = false; @property() title = ''; // make reactive to pass through @@ -145,6 +145,49 @@ export default class SlButton extends ShoelaceElement implements ShoelaceFormCon } } + private handleBlur() { + this.hasFocus = false; + this.emit('sl-blur'); + } + + private handleFocus() { + this.hasFocus = true; + this.emit('sl-focus'); + } + + private handleClick(event: MouseEvent) { + if (this.disabled || this.loading) { + event.preventDefault(); + event.stopPropagation(); + return; + } + + if (this.type === 'submit') { + this.formSubmitController.submit(this); + } + + if (this.type === 'reset') { + this.formSubmitController.reset(this); + } + } + + private isButton() { + return this.href ? false : true; + } + + private isLink() { + return this.href ? true : false; + } + + @watch('disabled', { waitUntilFirstUpdate: true }) + handleDisabledChange() { + // Disabled form controls are always valid, so we need to recheck validity when the state changes + if (this.isButton()) { + this.button.disabled = this.disabled; + this.invalid = !(this.button as HTMLButtonElement).checkValidity(); + } + } + /** Simulates a click on the button. */ click() { this.button.click(); @@ -186,49 +229,6 @@ export default class SlButton extends ShoelaceElement implements ShoelaceFormCon } } - handleBlur() { - this.hasFocus = false; - this.emit('sl-blur'); - } - - handleFocus() { - this.hasFocus = true; - this.emit('sl-focus'); - } - - handleClick(event: MouseEvent) { - if (this.disabled || this.loading) { - event.preventDefault(); - event.stopPropagation(); - return; - } - - if (this.type === 'submit') { - this.formSubmitController.submit(this); - } - - if (this.type === 'reset') { - this.formSubmitController.reset(this); - } - } - - @watch('disabled', { waitUntilFirstUpdate: true }) - handleDisabledChange() { - // Disabled form controls are always valid, so we need to recheck validity when the state changes - if (this.isButton()) { - this.button.disabled = this.disabled; - this.invalid = !(this.button as HTMLButtonElement).checkValidity(); - } - } - - private isButton() { - return this.href ? false : true; - } - - private isLink() { - return this.href ? true : false; - } - render() { const isLink = this.isLink(); const tag = isLink ? literal`a` : literal`button`; diff --git a/src/components/checkbox/checkbox.ts b/src/components/checkbox/checkbox.ts index 6c2ea1206..dcddcdddc 100644 --- a/src/components/checkbox/checkbox.ts +++ b/src/components/checkbox/checkbox.ts @@ -39,8 +39,6 @@ import type { CSSResultGroup } from 'lit'; export default class SlCheckbox extends ShoelaceElement implements ShoelaceFormControl { static styles: CSSResultGroup = styles; - @query('input[type="checkbox"]') input: HTMLInputElement; - // @ts-expect-error -- Controller is currently unused private readonly formSubmitController = new FormSubmitController(this, { value: (control: SlCheckbox) => (control.checked ? control.value || 'on' : undefined), @@ -48,8 +46,11 @@ export default class SlCheckbox extends ShoelaceElement implements ShoelaceFormC setValue: (control: SlCheckbox, checked: boolean) => (control.checked = checked) }); + @query('input[type="checkbox"]') input: HTMLInputElement; + @state() private hasFocus = false; @state() invalid = false; + @property() title = ''; // make reactive to pass through /** The name of the checkbox, submitted as a name/value pair with form data. */ @@ -83,6 +84,41 @@ export default class SlCheckbox extends ShoelaceElement implements ShoelaceFormC this.invalid = !this.input.checkValidity(); } + private handleClick() { + this.checked = !this.checked; + this.indeterminate = false; + this.emit('sl-change'); + } + + private handleBlur() { + this.hasFocus = false; + this.emit('sl-blur'); + } + + private handleInput() { + this.emit('sl-input'); + } + + private handleFocus() { + this.hasFocus = true; + this.emit('sl-focus'); + } + + @watch('disabled', { waitUntilFirstUpdate: true }) + handleDisabledChange() { + // Disabled form controls are always valid, so we need to recheck validity when the state changes + this.input.disabled = this.disabled; + this.invalid = !this.input.checkValidity(); + } + + @watch('checked', { waitUntilFirstUpdate: true }) + @watch('indeterminate', { waitUntilFirstUpdate: true }) + handleStateChange() { + this.input.checked = this.checked; // force a sync update + this.input.indeterminate = this.indeterminate; // force a sync update + this.invalid = !this.input.checkValidity(); + } + /** Simulates a click on the checkbox. */ click() { this.input.click(); @@ -117,41 +153,6 @@ export default class SlCheckbox extends ShoelaceElement implements ShoelaceFormC this.invalid = !this.input.checkValidity(); } - handleClick() { - this.checked = !this.checked; - this.indeterminate = false; - this.emit('sl-change'); - } - - handleBlur() { - this.hasFocus = false; - this.emit('sl-blur'); - } - - handleInput() { - this.emit('sl-input'); - } - - @watch('disabled', { waitUntilFirstUpdate: true }) - handleDisabledChange() { - // Disabled form controls are always valid, so we need to recheck validity when the state changes - this.input.disabled = this.disabled; - this.invalid = !this.input.checkValidity(); - } - - handleFocus() { - this.hasFocus = true; - this.emit('sl-focus'); - } - - @watch('checked', { waitUntilFirstUpdate: true }) - @watch('indeterminate', { waitUntilFirstUpdate: true }) - handleStateChange() { - this.input.checked = this.checked; // force a sync update - this.input.indeterminate = this.indeterminate; // force a sync update - this.invalid = !this.input.checkValidity(); - } - render() { return html`