From 381e913bf7050729b0e402ded45522b3ab608074 Mon Sep 17 00:00:00 2001 From: Mizga Ionut-Alexandru Date: Mon, 18 Nov 2024 15:32:02 +0200 Subject: [PATCH 1/5] fix: align css and docs with spectrum-css --- packages/card/src/Card.ts | 4 ++-- packages/card/src/card.css | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/card/src/Card.ts b/packages/card/src/Card.ts index 5fb6b19aed..05da3712d9 100644 --- a/packages/card/src/Card.ts +++ b/packages/card/src/Card.ts @@ -41,8 +41,8 @@ import detailStyles from '@spectrum-web-components/styles/detail.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 preview - This is the preview image for Gallery and Quiet Cards + * @slot cover-photo - This is the cover photo for Default * @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 diff --git a/packages/card/src/card.css b/packages/card/src/card.css index 6eb3084ae9..62284a1710 100644 --- a/packages/card/src/card.css +++ b/packages/card/src/card.css @@ -49,8 +49,8 @@ slot[name='description'] { } #cover-photo ::slotted(*), -:host(:not([variant='quiet'])) #preview ::slotted(*) { - width: 100%; +#preview ::slotted(*) { + inline-size: 100%; display: block; /* Since we're using tags instead of background-image for the cover photo, From acb3339a8ffbd0bdbcf16d701ee0c68e052252a4 Mon Sep 17 00:00:00 2001 From: Mizga Ionut-Alexandru Date: Mon, 18 Nov 2024 16:22:56 +0200 Subject: [PATCH 2/5] chore(card): slots cleanup --- packages/card/README.md | 40 ++-- packages/card/src/Card.ts | 78 ++----- packages/card/src/card.css | 18 +- packages/card/stories/card.stories.ts | 42 ++-- packages/card/test/benchmark/test-basic.ts | 2 +- packages/card/test/card.test.ts | 223 ++++++++++----------- tools/grid/stories/grid.stories.ts | 2 +- 7 files changed, 159 insertions(+), 246 deletions(-) diff --git a/packages/card/README.md b/packages/card/README.md index 90221a7890..9d3465dc97 100644 --- a/packages/card/README.md +++ b/packages/card/README.md @@ -31,18 +31,14 @@ import { Card } from '@spectrum-web-components/card'; ```html demo - Demo Image + Demo Image
Footer
``` ```html demo - Demo Image + Demo Image
Footer
``` @@ -57,7 +53,7 @@ By default, the heading for an `` is applied via the `heading` attribut style="--spectrum-card-body-header-height: auto;" >

Card Heading

- +
Footer
``` @@ -74,7 +70,7 @@ An `` can be provided with an `href` attribute in order for it to act a href="https://opensource.adobe.com/spectrum-web-components" target="_blank" > - Demo Image + Demo Image ``` @@ -89,7 +85,7 @@ Normal cards can contain a heading, a subheading, a cover photo, and a footer. ```html - + JPG photo
Footer
@@ -101,11 +97,7 @@ Cards can be supplied an `actions` via a names slot. ```html - Demo Image + Demo Image
Footer
- +
10/15/18
Footer
@@ -158,7 +150,7 @@ When leveraging the `asset` attribute, a card can be declared as representing a subheading="JPG Photo" asset="file" > - +
File Name
10/15/18
Footer
@@ -171,7 +163,7 @@ Or a `folder`: ```html
- +
Folder Name
10/15/18
Footer
@@ -184,7 +176,7 @@ Quiet cards will also accept `actions` via a named slot. ```html
- +
10/15/18
- +
10/15/18
Footer
@@ -225,11 +217,7 @@ Gallery cards can contain a heading, a subheading, an image preview, a descripti ```html demo
- Demo Image + Demo Image
Footer
@@ -240,7 +228,7 @@ A `horizontal` card: ```html demo
- + - Demo Image + Demo Image
Footer
- - - ${this.variant !== 'quiet' && !this.horizontal - ? html` - - ` - : nothing} - `; - } + protected renderImage(): TemplateResult { + const hasPreview = + this.variant === 'gallery' || + this.variant === 'quiet' || + this.horizontal; - protected get renderCoverImage(): TemplateResult { return html` - - + + ${this.variant !== 'quiet' && !this.horizontal ? html` @@ -244,24 +223,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`
${this.subheading} @@ -275,9 +237,9 @@ export class Card extends LikeAnchor( ${this.renderImage()}
- ${this.renderHeading} + ${this.renderHeading()} ${this.variant === 'gallery' - ? this.renderSubtitleAndDescription + ? this.renderSubtitleAndDescription() : nothing} ${this.variant !== 'quiet' || this.size !== 's' ? html` @@ -293,7 +255,7 @@ export class Card extends LikeAnchor( ${this.variant !== 'gallery' ? html`
- ${this.renderSubtitleAndDescription} + ${this.renderSubtitleAndDescription()}
` : nothing} diff --git a/packages/card/src/card.css b/packages/card/src/card.css index 62284a1710..e0440d61a1 100644 --- a/packages/card/src/card.css +++ b/packages/card/src/card.css @@ -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,10 +37,6 @@ slot[name='description'] { ); } -#preview + #cover-photo { - display: none; -} - #cover-photo ::slotted(*), #preview ::slotted(*) { inline-size: 100%; @@ -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; diff --git a/packages/card/stories/card.stories.ts b/packages/card/stories/card.stories.ts index 4e5251530d..9620b3ee86 100644 --- a/packages/card/stories/card.stories.ts +++ b/packages/card/stories/card.stories.ts @@ -9,17 +9,17 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ +import '@spectrum-web-components/action-menu/sp-action-menu.js'; import { html, TemplateResult } from '@spectrum-web-components/base'; - +import { ifDefined } from '@spectrum-web-components/base/src/directives.js'; import '@spectrum-web-components/card/sp-card.js'; -import { landscape, portrait } from './images'; import '@spectrum-web-components/icons-workflow/icons/sp-icon-file-txt.js'; -import '@spectrum-web-components/textfield/sp-textfield.js'; -import '@spectrum-web-components/action-menu/sp-action-menu.js'; -import '@spectrum-web-components/menu/sp-menu.js'; -import '@spectrum-web-components/menu/sp-menu-item.js'; -import '@spectrum-web-components/menu/sp-menu-divider.js'; import '@spectrum-web-components/link/sp-link.js'; +import '@spectrum-web-components/menu/sp-menu-divider.js'; +import '@spectrum-web-components/menu/sp-menu-item.js'; +import '@spectrum-web-components/menu/sp-menu.js'; +import '@spectrum-web-components/textfield/sp-textfield.js'; +import { landscape, portrait } from './images'; export default { component: 'sp-card', @@ -56,7 +56,7 @@ export const Default = (args: StoryArgs): TemplateResult => { ?horizontal=${args.horizontal} style="width: 200px;" > - Demo Graphic + Demo Graphic
Footer
`; @@ -102,7 +102,7 @@ export const SmallQuiet = (args: StoryArgs): TemplateResult => { Save Selection Make Work Path - Demo Graphic + Demo Graphic `; }; @@ -146,7 +146,7 @@ export const href = (args: StoryArgs): TemplateResult => { Save Selection Make Work Path - Demo Graphic + Demo Graphic `; }; @@ -162,7 +162,7 @@ export const actions = (args: StoryArgs): TemplateResult => { ?horizontal=${args.horizontal} style="width: 200px;" > - Demo Graphic + Demo Graphic
Footer
{ ?horizontal=${args.horizontal} > Demo Graphic { ?horizontal=${args.horizontal} style="width: 208px; height: 264px" > - Demo Graphic + Demo Graphic
10/15/18
@@ -241,7 +241,7 @@ export const quietFile = (args: StoryArgs): TemplateResult => { ?horizontal=${args.horizontal} style="width: 208px; height: 264px" > - Demo Graphic + Demo Graphic
File Name
10/15/18
@@ -259,7 +259,7 @@ export const quietFolder = (args: StoryArgs): TemplateResult => { ?horizontal=${args.horizontal} style="width: 208px; height: 264px" > - Demo Graphic + Demo Graphic
Folder Name
10/15/18
@@ -277,7 +277,7 @@ export const quietActions = (args: StoryArgs): TemplateResult => { ?horizontal=${args.horizontal} style="width: 208px; height: 264px" > - Demo Graphic + Demo Graphic
10/15/18
{ subheading="JPG" > @@ -328,7 +328,7 @@ export const horizontalWithHREF = (args: StoryArgs): TemplateResult => { target="_blank" > @@ -342,14 +342,14 @@ export const smallQuiet = (args: StoryArgs): TemplateResult => { return html`
- Demo Graphic + Demo Graphic
Footer
{ " ?horizontal=${args.horizontal} > - Demo Graphic + Demo Graphic Demo Graphic diff --git a/packages/card/test/card.test.ts b/packages/card/test/card.test.ts index 83af9f45dd..51eea61896 100644 --- a/packages/card/test/card.test.ts +++ b/packages/card/test/card.test.ts @@ -10,78 +10,69 @@ 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/card/sp-card.js'; -import { Card } from '@spectrum-web-components/card'; +import { elementUpdated, expect, fixture, html } from '@open-wc/testing'; import '@spectrum-web-components/action-menu/sp-action-menu.js'; -import '@spectrum-web-components/menu/sp-menu.js'; -import '@spectrum-web-components/menu/sp-menu-item.js'; +import { Card } from '@spectrum-web-components/card'; +import '@spectrum-web-components/card/sp-card.js'; +import { Checkbox } from '@spectrum-web-components/checkbox/src/Checkbox.js'; import '@spectrum-web-components/menu/sp-menu-divider.js'; -import { elementUpdated, expect, fixture, html } from '@open-wc/testing'; - +import '@spectrum-web-components/menu/sp-menu-item.js'; +import '@spectrum-web-components/menu/sp-menu.js'; +import { spy } from 'sinon'; +import { sendMouse } from '../../../test/plugins/browser.js'; +import { + spaceEvent, + testForLitDevWarnings, +} from '../../../test/testing-helpers.js'; import { Default, Horizontal, href, StoryArgs, } from '../stories/card.stories.js'; -import { Checkbox } from '@spectrum-web-components/checkbox/src/Checkbox'; -import { spy } from 'sinon'; -import { spaceEvent } from '../../../test/testing-helpers.js'; -import { sendMouse } from '../../../test/plugins/browser.js'; -import { testForLitDevWarnings } from '../../../test/testing-helpers.js'; describe('card', () => { testForLitDevWarnings( async () => - await fixture( - html` - - Slotted Preview -
Footer
-
- ` - ) - ); - it('loads', async () => { - const el = await fixture( - html` + await fixture(html` Slotted Preview
Footer
- ` - ); + `) + ); + it('loads', async () => { + const el = await fixture(html` + + Slotted Preview +
Footer
+
+ `); await elementUpdated(el); await expect(el).to.be.accessible(); }); it('loads - [quiet]', async () => { - const el = await fixture( - html` - - Slotted Preview -
10/15/18
-
Footer
-
- ` - ); + const el = await fixture(html` + + Slotted Preview +
10/15/18
+
Footer
+
+ `); await elementUpdated(el); @@ -89,63 +80,55 @@ describe('card', () => { }); it('loads - [quiet][small]', async () => { - const el = await fixture( - html` - (html` + + Demo Graphic +
Footer
+ - Demo Graphic -
Footer
- - - Deselect - Select Inverse - Feather... - Select and Mask... - - Save Selection - Make Work Path - - -
- ` - ); + + Deselect + Select Inverse + Feather... + Select and Mask... + + Save Selection + Make Work Path + +
+
+ `); await elementUpdated(el); await expect(el).to.be.accessible(); }); it('loads - [gallery]', async () => { - const el = await fixture( - html` - - Slotted Preview -
10/15/18
-
Footer
-
- ` - ); + const el = await fixture(html` + + Slotted Preview +
10/15/18
+
Footer
+
+ `); await elementUpdated(el); @@ -367,18 +350,16 @@ describe('card', () => { }); it('displays the `heading` attribute as `.title`', async () => { const testHeading = 'This is a test heading'; - const el = await fixture( - html` - - Slotted Preview -
Footer
-
- ` - ); + const el = await fixture(html` + + Slotted Preview +
Footer
+
+ `); await elementUpdated(el); @@ -393,19 +374,17 @@ describe('card', () => { }); it('displays the slotted content as `.title`', async () => { const testHeading = 'This is a test heading'; - const el = await fixture( - html` - -

${testHeading}

- Slotted Preview -
Footer
-
- ` - ); + const el = await fixture(html` + +

${testHeading}

+ Slotted Preview +
Footer
+
+ `); await elementUpdated(el); diff --git a/tools/grid/stories/grid.stories.ts b/tools/grid/stories/grid.stories.ts index 304f39a976..341994f192 100644 --- a/tools/grid/stories/grid.stories.ts +++ b/tools/grid/stories/grid.stories.ts @@ -68,7 +68,7 @@ const renderItem = ( > From c072b878c7c7a2d160e25d69af25801db289d14c Mon Sep 17 00:00:00 2001 From: Mizga Ionut-Alexandru Date: Mon, 18 Nov 2024 17:27:06 +0200 Subject: [PATCH 3/5] fix(card): guard clause for image rendering --- packages/card/src/Card.ts | 42 ++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/packages/card/src/Card.ts b/packages/card/src/Card.ts index 9cd2ab486d..410f463035 100644 --- a/packages/card/src/Card.ts +++ b/packages/card/src/Card.ts @@ -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,22 +20,24 @@ 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 { 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 @@ -48,10 +51,13 @@ import detailStyles from '@spectrum-web-components/styles/detail.js'; * @slot footer - Footer text */ export class Card extends LikeAnchor( - SizedMixin(FocusVisiblePolyfillMixin(SpectrumElement), { - validSizes: ['s', 'm'], - noDefaultSize: true, - }) + ObserveSlotPresence( + SizedMixin(FocusVisiblePolyfillMixin(SpectrumElement), { + validSizes: ['s', 'm'], + noDefaultSize: true, + }), + '[slot="image"]' + ) ) { public static override get styles(): CSSResultArray { return [headingStyles, detailStyles, cardStyles]; @@ -96,6 +102,10 @@ export class Card extends LikeAnchor( @property() public subheading = ''; + protected hasImage(): boolean { + return this.getSlotContentPresence('[slot="image"]'); + } + public override click(): void { this.likeAnchor?.click(); } @@ -234,7 +244,11 @@ export class Card extends LikeAnchor( protected override render(): TemplateResult { return html` - ${this.renderImage()} + ${when( + this.hasImage(), + () => this.renderImage(), + () => nothing + )}
${this.renderHeading()} From 35dbcc3899109afb10269aa14d6968201b357505 Mon Sep 17 00:00:00 2001 From: Mizga Ionut-Alexandru Date: Tue, 19 Nov 2024 15:33:34 +0200 Subject: [PATCH 4/5] feat: mod for object fit css prop --- packages/card/src/card.css | 2 +- packages/card/stories/card.stories.ts | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/card/src/card.css b/packages/card/src/card.css index e0440d61a1..c1c68f3c1d 100644 --- a/packages/card/src/card.css +++ b/packages/card/src/card.css @@ -45,7 +45,7 @@ slot[name='description'] { /* Since we're using 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(*) { diff --git a/packages/card/stories/card.stories.ts b/packages/card/stories/card.stories.ts index 9620b3ee86..7f5869978a 100644 --- a/packages/card/stories/card.stories.ts +++ b/packages/card/stories/card.stories.ts @@ -231,6 +231,28 @@ export const Quiet = (args: StoryArgs): TemplateResult => { `; }; +export const quietContained = (args: StoryArgs): TemplateResult => { + return html` +
+ + Demo Graphic +
10/15/18
+
+
+ `; +}; + export const quietFile = (args: StoryArgs): TemplateResult => { return html`
From fc6c4ca956cb5f6542f11ad3a1aef3dcfa0f4876 Mon Sep 17 00:00:00 2001 From: Mizga Ionut-Alexandru Date: Tue, 19 Nov 2024 15:41:05 +0200 Subject: [PATCH 5/5] chore: fix mixin order --- packages/card/src/Card.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/card/src/Card.ts b/packages/card/src/Card.ts index 410f463035..3c1cc9d36c 100644 --- a/packages/card/src/Card.ts +++ b/packages/card/src/Card.ts @@ -51,12 +51,15 @@ import cardStyles from './card.css.js'; * @slot footer - Footer text */ export class Card extends LikeAnchor( - ObserveSlotPresence( - SizedMixin(FocusVisiblePolyfillMixin(SpectrumElement), { + SizedMixin( + ObserveSlotPresence( + FocusVisiblePolyfillMixin(SpectrumElement), + '[slot="image"]' + ), + { validSizes: ['s', 'm'], noDefaultSize: true, - }), - '[slot="image"]' + } ) ) { public static override get styles(): CSSResultArray {