-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(card): clarify component's slot API #4938
base: main
Are you sure you want to change the base?
Changes from 4 commits
381e913
acb3339
c072b87
35dbcc3
fc6c4ca
99c8e3c
bf8320b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -10,6 +10,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag | |||||
governing permissions and limitations under the License. | ||||||
*/ | ||||||
|
||||||
import '@spectrum-web-components/asset/sp-asset.js'; | ||||||
import { | ||||||
CSSResultArray, | ||||||
html, | ||||||
|
@@ -19,46 +20,43 @@ import { | |||||
SpectrumElement, | ||||||
TemplateResult, | ||||||
} from '@spectrum-web-components/base'; | ||||||
import { ifDefined } from '@spectrum-web-components/base/src/directives.js'; | ||||||
import { | ||||||
property, | ||||||
query, | ||||||
} from '@spectrum-web-components/base/src/decorators.js'; | ||||||
import { FocusVisiblePolyfillMixin } from '@spectrum-web-components/shared/src/focus-visible.js'; | ||||||
import { ObserveSlotPresence } from '@spectrum-web-components/shared/src/observe-slot-presence.js'; | ||||||
import { LikeAnchor } from '@spectrum-web-components/shared/src/like-anchor.js'; | ||||||
import '@spectrum-web-components/asset/sp-asset.js'; | ||||||
|
||||||
import { Checkbox } from '@spectrum-web-components/checkbox/src/Checkbox'; | ||||||
import { | ||||||
ifDefined, | ||||||
when, | ||||||
} from '@spectrum-web-components/base/src/directives.js'; | ||||||
import '@spectrum-web-components/checkbox/sp-checkbox.js'; | ||||||
import '@spectrum-web-components/popover/sp-popover.js'; | ||||||
import { Checkbox } from '@spectrum-web-components/checkbox/src/Checkbox'; | ||||||
import '@spectrum-web-components/divider/sp-divider.js'; | ||||||
import cardStyles from './card.css.js'; | ||||||
import headingStyles from '@spectrum-web-components/styles/heading.js'; | ||||||
import '@spectrum-web-components/popover/sp-popover.js'; | ||||||
import { FocusVisiblePolyfillMixin } from '@spectrum-web-components/shared/src/focus-visible.js'; | ||||||
import { LikeAnchor } from '@spectrum-web-components/shared/src/like-anchor.js'; | ||||||
import { ObserveSlotPresence } from '@spectrum-web-components/shared/src/observe-slot-presence.js'; | ||||||
import detailStyles from '@spectrum-web-components/styles/detail.js'; | ||||||
import headingStyles from '@spectrum-web-components/styles/heading.js'; | ||||||
import cardStyles from './card.css.js'; | ||||||
|
||||||
/** | ||||||
* @element sp-card | ||||||
* | ||||||
* @fires change - Announces a change in the `selected` property of a card | ||||||
* @slot preview - This is the preview image for Gallery Cards | ||||||
* @slot cover-photo - This is the cover photo for Default and Quiet Cards | ||||||
* @slot image - This is the image of the card | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
* @slot heading - HTML content to be listed as the heading | ||||||
* @slot subheading - HTML content to be listed as the subheading | ||||||
* @slot description - A description of the card | ||||||
* @slot actions - an `sp-action-menu` element outlining actions to take on the represened object | ||||||
* @slot footer - Footer text | ||||||
*/ | ||||||
export class Card extends LikeAnchor( | ||||||
SizedMixin( | ||||||
ObserveSlotPresence(FocusVisiblePolyfillMixin(SpectrumElement), [ | ||||||
'[slot="cover-photo"]', | ||||||
'[slot="preview"]', | ||||||
]), | ||||||
{ | ||||||
ObserveSlotPresence( | ||||||
SizedMixin(FocusVisiblePolyfillMixin(SpectrumElement), { | ||||||
validSizes: ['s', 'm'], | ||||||
noDefaultSize: true, | ||||||
} | ||||||
}), | ||||||
'[slot="image"]' | ||||||
) | ||||||
) { | ||||||
public static override get styles(): CSSResultArray { | ||||||
|
@@ -104,12 +102,8 @@ export class Card extends LikeAnchor( | |||||
@property() | ||||||
public subheading = ''; | ||||||
|
||||||
protected get hasCoverPhoto(): boolean { | ||||||
return this.getSlotContentPresence('[slot="cover-photo"]'); | ||||||
} | ||||||
|
||||||
protected get hasPreview(): boolean { | ||||||
return this.getSlotContentPresence('[slot="preview"]'); | ||||||
protected hasImage(): boolean { | ||||||
return this.getSlotContentPresence('[slot="image"]'); | ||||||
} | ||||||
|
||||||
public override click(): void { | ||||||
|
@@ -207,7 +201,7 @@ export class Card extends LikeAnchor( | |||||
this.addEventListener('pointercancel', handleEnd); | ||||||
} | ||||||
|
||||||
protected get renderHeading(): TemplateResult { | ||||||
protected renderHeading(): TemplateResult { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont see any computation logic here so why change to a callable function instead of keeping it as a getter? |
||||||
return html` | ||||||
<div | ||||||
class="title spectrum-Heading spectrum-Heading--sizeXS" | ||||||
|
@@ -218,23 +212,18 @@ export class Card extends LikeAnchor( | |||||
`; | ||||||
} | ||||||
|
||||||
protected get renderPreviewImage(): TemplateResult { | ||||||
return html` | ||||||
<sp-asset id="preview" variant=${ifDefined(this.asset)}> | ||||||
<slot name="preview"></slot> | ||||||
</sp-asset> | ||||||
${this.variant !== 'quiet' && !this.horizontal | ||||||
? html` | ||||||
<sp-divider size="s"></sp-divider> | ||||||
` | ||||||
: nothing} | ||||||
`; | ||||||
} | ||||||
protected renderImage(): TemplateResult { | ||||||
const hasPreview = | ||||||
this.variant === 'gallery' || | ||||||
this.variant === 'quiet' || | ||||||
this.horizontal; | ||||||
|
||||||
protected get renderCoverImage(): TemplateResult { | ||||||
return html` | ||||||
<sp-asset id="cover-photo" variant=${ifDefined(this.asset)}> | ||||||
<slot name="cover-photo"></slot> | ||||||
<sp-asset | ||||||
id=${hasPreview ? 'preview' : 'cover-photo'} | ||||||
variant=${ifDefined(this.asset)} | ||||||
> | ||||||
<slot name="image"></slot> | ||||||
</sp-asset> | ||||||
${this.variant !== 'quiet' && !this.horizontal | ||||||
? html` | ||||||
|
@@ -244,24 +233,7 @@ export class Card extends LikeAnchor( | |||||
`; | ||||||
} | ||||||
|
||||||
protected get images(): TemplateResult[] { | ||||||
const images: TemplateResult[] = []; | ||||||
if (this.hasPreview) images.push(this.renderPreviewImage); | ||||||
if (this.hasCoverPhoto) images.push(this.renderCoverImage); | ||||||
return images; | ||||||
} | ||||||
|
||||||
private renderImage(): TemplateResult[] { | ||||||
if (this.horizontal) { | ||||||
return this.images; | ||||||
} | ||||||
if (this.variant !== 'standard') { | ||||||
return [this.renderPreviewImage]; | ||||||
} | ||||||
return this.images; | ||||||
} | ||||||
|
||||||
private get renderSubtitleAndDescription(): TemplateResult { | ||||||
protected renderSubtitleAndDescription(): TemplateResult { | ||||||
return html` | ||||||
<div class="subtitle spectrum-Detail spectrum-Detail--sizeS"> | ||||||
<slot name="subheading">${this.subheading}</slot> | ||||||
|
@@ -272,12 +244,16 @@ export class Card extends LikeAnchor( | |||||
|
||||||
protected override render(): TemplateResult { | ||||||
return html` | ||||||
${this.renderImage()} | ||||||
${when( | ||||||
this.hasImage(), | ||||||
() => this.renderImage(), | ||||||
() => nothing | ||||||
)} | ||||||
<div class="body"> | ||||||
<div class="header"> | ||||||
${this.renderHeading} | ||||||
${this.renderHeading()} | ||||||
${this.variant === 'gallery' | ||||||
? this.renderSubtitleAndDescription | ||||||
? this.renderSubtitleAndDescription() | ||||||
: nothing} | ||||||
${this.variant !== 'quiet' || this.size !== 's' | ||||||
? html` | ||||||
|
@@ -293,7 +269,7 @@ export class Card extends LikeAnchor( | |||||
${this.variant !== 'gallery' | ||||||
? html` | ||||||
<div class="content"> | ||||||
${this.renderSubtitleAndDescription} | ||||||
${this.renderSubtitleAndDescription()} | ||||||
</div> | ||||||
` | ||||||
: nothing} | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,14 +25,7 @@ governing permissions and limitations under the License. | |
|
||
.action-button { | ||
flex-grow: 0; | ||
} | ||
|
||
:host([dir='ltr']) .action-button { | ||
margin-left: auto; | ||
} | ||
|
||
:host([dir='rtl']) .action-button { | ||
margin-right: auto; | ||
margin-inline-start: auto; | ||
} | ||
|
||
/* The description slot has a psuedo-element that also needs to receive the font styling. | ||
|
@@ -44,19 +37,15 @@ slot[name='description'] { | |
); | ||
} | ||
|
||
#preview + #cover-photo { | ||
display: none; | ||
} | ||
|
||
#cover-photo ::slotted(*), | ||
:host(:not([variant='quiet'])) #preview ::slotted(*) { | ||
width: 100%; | ||
#preview ::slotted(*) { | ||
inline-size: 100%; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should include the |
||
display: block; | ||
|
||
/* Since we're using <img> tags instead of background-image for the cover photo, | ||
we need an additional object-fit property to preserve the aspect ratio of the image | ||
In spectrum-css, background-size is used */ | ||
object-fit: cover; | ||
object-fit: var(--mod-card-image-object-fit, cover); | ||
} | ||
|
||
:host(:not([variant='gallery'])) #preview ::slotted(*) { | ||
|
@@ -88,11 +77,6 @@ sp-popover { | |
width: var(--spectrum-card-title-width); | ||
} | ||
|
||
.subtitle { | ||
/* Override until https://github.com/adobe/spectrum-css/issues/1054 is fixed */ | ||
text-transform: none; | ||
} | ||
|
||
:host:before, | ||
:host:after { | ||
pointer-events: none; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.