From 2bd88b7d925b954d4bf417c609aa710422567317 Mon Sep 17 00:00:00 2001 From: Cory LaViska Date: Thu, 15 Oct 2020 16:35:11 -0400 Subject: [PATCH] Rework slot logic --- CONTRIBUTING.md | 8 ++++---- docs/getting-started/changelog.md | 1 + src/components/card/card.tsx | 19 +++++-------------- src/components/dialog/dialog.tsx | 19 ++++++------------- src/components/drawer/drawer.tsx | 19 ++++++------------- 5 files changed, 22 insertions(+), 44 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 38648c53..b58c3d3b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -189,11 +189,11 @@ To expose custom properties for a component, define them in the `:host` block an ### Conditional Slots -When a component relies on the presence of a slot to do something, don't assume the initial state of the component is its permanent state. Users can add and remove slotted content anytime, and components should be aware of this. +When a component relies on the presence of slotted content to do something, don't assume its initial state is permanent. Slotted content can be added or removed any time and components must be aware of this. A good practice to manage this is: -- Create an `updateSlots` method that uses `hasSlot` (imported from `src/utilities/slot.ts`) to check for the required slot(s) -- Listen on the host element for the `slotchange` event and run `updateSlots` -- Don't conditionally render any slots — always use `hidden` or `display: none` so the slot exists in the DOM +- Create a `handleSlotChange` method that uses `hasSlot` (imported from `src/utilities/slot.ts`) to update state variables for the associated slot(s) +- Add `onSlotchange={this.handleSlotChange}` to the slots you want to watch +- Never conditionally render `` elements in a component — always use `hidden` so the slot remains in the DOM and the `slotchange` event can be captured See the source of card, dialog, or drawer for examples. diff --git a/docs/getting-started/changelog.md b/docs/getting-started/changelog.md index ddc86ce0..f2e89de0 100644 --- a/docs/getting-started/changelog.md +++ b/docs/getting-started/changelog.md @@ -14,6 +14,7 @@ _During the beta period, these restrictions may be relaxed in the event of a mis - Fixed a bug where concurrent active modals (i.e. dialog, drawer) would try to steal focus from each other - Improved `sl-color-picker` grid and slider handles [#246](https://github.com/shoelace-style/shoelace/issues/246) - Reworked show/hide logic in `sl-alert`, `sl-dialog`, and `sl-drawer` to not use reflow hacks and the `hidden` attribute +- Reworked slot logic in `sl-card`, `sl-dialog`, and `sl-drawer` - Updated to Popper 2.5.3 to address a fixed position bug in Firefox ## 2.0.0-beta.20 diff --git a/src/components/card/card.tsx b/src/components/card/card.tsx index 59d2b7b7..53dd1861 100644 --- a/src/components/card/card.tsx +++ b/src/components/card/card.tsx @@ -30,19 +30,10 @@ export class Card { @State() hasHeader = false; connectedCallback() { - this.updateSlots = this.updateSlots.bind(this); + this.handleSlotChange = this.handleSlotChange.bind(this); } - componentWillLoad() { - this.updateSlots(); - this.host.shadowRoot.addEventListener('slotchange', this.updateSlots); - } - - disconnectedCallback() { - this.host.shadowRoot.removeEventListener('slotchange', this.updateSlots); - } - - updateSlots() { + handleSlotChange() { this.hasFooter = hasSlot(this.host, 'footer'); this.hasImage = hasSlot(this.host, 'image'); this.hasHeader = hasSlot(this.host, 'header'); @@ -60,11 +51,11 @@ export class Card { }} >
- +
- +
@@ -72,7 +63,7 @@ export class Card {
- +
); diff --git a/src/components/dialog/dialog.tsx b/src/components/dialog/dialog.tsx index 5eb18cd0..2f35bf6f 100644 --- a/src/components/dialog/dialog.tsx +++ b/src/components/dialog/dialog.tsx @@ -78,18 +78,13 @@ export class Dialog { this.handleTransitionEnd = this.handleTransitionEnd.bind(this); this.handleKeyDown = this.handleKeyDown.bind(this); this.handleOverlayClick = this.handleOverlayClick.bind(this); - this.updateSlots = this.updateSlots.bind(this); + this.handleSlotChange = this.handleSlotChange.bind(this); this.modal = new Modal(this.host, { onFocusOut: () => this.panel.focus() }); } - componentWillLoad() { - this.updateSlots(); - this.host.shadowRoot.addEventListener('slotchange', this.updateSlots); - } - componentDidLoad() { // Show on init if open if (this.open) { @@ -99,8 +94,6 @@ export class Dialog { disconnectedCallback() { unlockBodyScrolling(this.host); - - this.host.shadowRoot.removeEventListener('slotchange', this.updateSlots); } /** Shows the dialog */ @@ -162,6 +155,10 @@ export class Dialog { } } + handleSlotChange() { + this.hasFooter = hasSlot(this.host, 'footer'); + } + handleTransitionEnd(event: TransitionEvent) { const target = event.target as HTMLElement; @@ -176,10 +173,6 @@ export class Dialog { } } - updateSlots() { - this.hasFooter = hasSlot(this.host, 'footer'); - } - render() { return (
- +
diff --git a/src/components/drawer/drawer.tsx b/src/components/drawer/drawer.tsx index 496ef917..3707291f 100644 --- a/src/components/drawer/drawer.tsx +++ b/src/components/drawer/drawer.tsx @@ -86,18 +86,13 @@ export class Drawer { this.handleTransitionEnd = this.handleTransitionEnd.bind(this); this.handleKeyDown = this.handleKeyDown.bind(this); this.handleOverlayClick = this.handleOverlayClick.bind(this); - this.updateSlots = this.updateSlots.bind(this); + this.handleSlotChange = this.handleSlotChange.bind(this); this.modal = new Modal(this.host, { onFocusOut: () => (this.contained ? null : this.panel.focus()) }); } - componentWillLoad() { - this.updateSlots(); - this.host.shadowRoot.addEventListener('slotchange', this.updateSlots); - } - componentDidLoad() { // Show on init if open if (this.open) { @@ -107,8 +102,6 @@ export class Drawer { disconnectedCallback() { unlockBodyScrolling(this.host); - - this.host.shadowRoot.removeEventListener('slotchange', this.updateSlots); } /** Shows the drawer */ @@ -173,6 +166,10 @@ export class Drawer { } } + handleSlotChange() { + this.hasFooter = hasSlot(this.host, 'footer'); + } + handleTransitionEnd(event: TransitionEvent) { const target = event.target as HTMLElement; @@ -187,10 +184,6 @@ export class Drawer { } } - updateSlots() { - this.hasFooter = hasSlot(this.host, 'footer'); - } - render() { return (