From 5ab22cb27ba88b06bd2681ee5c2e83ecead6e9f6 Mon Sep 17 00:00:00 2001 From: Kevin Rodrigues Date: Mon, 23 Sep 2024 10:56:15 +0100 Subject: [PATCH 01/10] feat(pie-card): DSW-2426 adds opacity change to image if card is disabled --- .changeset/mean-planets-begin.md | 5 ++ packages/components/pie-card/src/index.ts | 58 ++++++++++++++++++- .../pie-card/test/component/pie-card.spec.ts | 27 +++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 .changeset/mean-planets-begin.md diff --git a/.changeset/mean-planets-begin.md b/.changeset/mean-planets-begin.md new file mode 100644 index 0000000000..b9237a8875 --- /dev/null +++ b/.changeset/mean-planets-begin.md @@ -0,0 +1,5 @@ +--- +"@justeattakeaway/pie-card": minor +--- + +[Added] Ability to set images to 50% opacity when disabled diff --git a/packages/components/pie-card/src/index.ts b/packages/components/pie-card/src/index.ts index 4978028d7b..270eca1728 100644 --- a/packages/components/pie-card/src/index.ts +++ b/packages/components/pie-card/src/index.ts @@ -117,6 +117,62 @@ export class PieCard extends LitElement implements CardProps { return `padding: ${paddingCSS}`; } + /** + * Handles the slot change event and applies/removes opacity to images based on the `disabled` state. + * + * @private + */ + private handleSlotChange () { + this.updateImagesOpacity(); + } + + /** + * Updates opacity of all images (slotted and non-slotted) based on the `disabled` property. + */ + private updateImagesOpacity () { + // Handle slotted images + const slot = this.shadowRoot?.querySelector('slot'); + + if (slot) { + const slottedElements = slot.assignedElements({ flatten: true }); + + slottedElements.forEach((element) => { + const images = element.querySelectorAll('img'); + this.applyOpacityToImages(images); + }); + } + + // Handle non-slotted direct content images + const directImages = this.querySelectorAll('img'); + this.applyOpacityToImages(directImages); + } + + /** + * Applies or removes opacity from the given images based on the `disabled` property. + * + * @param {NodeListOf} images - The images to apply opacity to. + */ + private applyOpacityToImages (images: NodeListOf) { + images.forEach((img) => { + if (this.disabled) { + img.style.opacity = '50%'; + } else { + img.style.opacity = ''; + } + }); + } + + /** + * Observes changes in the `disabled` property and triggers the update of images' opacity. + * + * @param changedProperties + */ + updated (changedProperties: Map) { + if (changedProperties.has('disabled')) { + this.updateImagesOpacity(); // Re-apply styles when disabled state changes + } + } + render () { const { variant, @@ -146,7 +202,7 @@ export class PieCard extends LitElement implements CardProps { aria-label=${aria?.label || nothing} aria-disabled=${disabled ? 'true' : 'false'} style=${paddingCSS || ''}> - + `; } diff --git a/packages/components/pie-card/test/component/pie-card.spec.ts b/packages/components/pie-card/test/component/pie-card.spec.ts index f6d29f4210..6348b890b2 100644 --- a/packages/components/pie-card/test/component/pie-card.spec.ts +++ b/packages/components/pie-card/test/component/pie-card.spec.ts @@ -304,4 +304,31 @@ test.describe('PieCard - Component tests', () => { }); }); }); + + test.describe('Prop: disabled', () => { + test.describe('when an image exists and the prop `disabled` is set to `true`', () => { + test('should set the opacity to 50%', async ({ mount, page }) => { + // Arrange + const slottedImageContent = `
+ Sample Image +
`; + + await mount(PieCard, { + props: { + disabled: true, + } as CardProps, + slots: { + default: slottedImageContent, + }, + }); + + // Act + const component = page.locator(`[data-test-id="slot-content"]`); + const image = component.locator('img'); + + // Assert the image has the correct opacity + await expect(image).toHaveCSS('opacity', '0.5'); + }); + }); + }); }); From 72d81e387cf95872fe603c15c3da57a9fd5736ef Mon Sep 17 00:00:00 2001 From: Kevin Rodrigues Date: Mon, 23 Sep 2024 11:19:22 +0100 Subject: [PATCH 02/10] feat(pie-card): DSW-2426 fix lint error --- packages/components/pie-card/test/component/pie-card.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/pie-card/test/component/pie-card.spec.ts b/packages/components/pie-card/test/component/pie-card.spec.ts index 6348b890b2..baab4bc8a8 100644 --- a/packages/components/pie-card/test/component/pie-card.spec.ts +++ b/packages/components/pie-card/test/component/pie-card.spec.ts @@ -323,7 +323,7 @@ test.describe('PieCard - Component tests', () => { }); // Act - const component = page.locator(`[data-test-id="slot-content"]`); + const component = page.locator('[data-test-id="slot-content"]'); const image = component.locator('img'); // Assert the image has the correct opacity From 81b7fb3d96e6aed21e396598f1e35edf00753a17 Mon Sep 17 00:00:00 2001 From: Kevin Rodrigues Date: Tue, 24 Sep 2024 10:14:09 +0100 Subject: [PATCH 03/10] feat(pie-card): DSW-2426 pR comment to use propertyValues --- packages/components/pie-card/src/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/pie-card/src/index.ts b/packages/components/pie-card/src/index.ts index 270eca1728..3e8b415ee1 100644 --- a/packages/components/pie-card/src/index.ts +++ b/packages/components/pie-card/src/index.ts @@ -1,5 +1,5 @@ import { - html, LitElement, unsafeCSS, nothing, TemplateResult, + html, LitElement, unsafeCSS, nothing, TemplateResult, type PropertyValues, } from 'lit'; import { classMap, type ClassInfo } from 'lit/directives/class-map.js'; import { ifDefined } from 'lit/directives/if-defined.js'; @@ -167,8 +167,8 @@ export class PieCard extends LitElement implements CardProps { * * @param changedProperties */ - updated (changedProperties: Map) { - if (changedProperties.has('disabled')) { + updated(changedProperties: PropertyValues): void { + if (changedProperties.has('disabled' as any)) { this.updateImagesOpacity(); // Re-apply styles when disabled state changes } } From e2969e456272ffed683eb864d0fb5afb435b00a5 Mon Sep 17 00:00:00 2001 From: Kevin Rodrigues Date: Tue, 24 Sep 2024 11:22:24 +0100 Subject: [PATCH 04/10] feat(pie-card): DSW-2426 update method changed to handle propertyValues --- packages/components/pie-card/src/index.ts | 38 ++++++++++------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/packages/components/pie-card/src/index.ts b/packages/components/pie-card/src/index.ts index 3e8b415ee1..23ae0b481d 100644 --- a/packages/components/pie-card/src/index.ts +++ b/packages/components/pie-card/src/index.ts @@ -3,7 +3,7 @@ import { } from 'lit'; import { classMap, type ClassInfo } from 'lit/directives/class-map.js'; import { ifDefined } from 'lit/directives/if-defined.js'; -import { property } from 'lit/decorators.js'; +import { property, queryAssignedElements } from 'lit/decorators.js'; import { validPropertyValues, defineCustomElement } from '@justeattakeaway/pie-webc-core'; import styles from './card.scss?inline'; import { @@ -53,6 +53,9 @@ export class PieCard extends LitElement implements CardProps { @validPropertyValues(componentSelector, paddingValues, undefined) public padding?: CardProps['padding']; + @queryAssignedElements({ flatten: true }) + private assignedElements!: HTMLElement[]; + /** * Renders the card as an anchor element. * @@ -128,19 +131,15 @@ export class PieCard extends LitElement implements CardProps { /** * Updates opacity of all images (slotted and non-slotted) based on the `disabled` property. + * + * @private */ - private updateImagesOpacity () { + private updateImagesOpacity(): void { // Handle slotted images - const slot = this.shadowRoot?.querySelector('slot'); - - if (slot) { - const slottedElements = slot.assignedElements({ flatten: true }); - - slottedElements.forEach((element) => { - const images = element.querySelectorAll('img'); - this.applyOpacityToImages(images); - }); - } + this.assignedElements.forEach((element) => { + const images = element.querySelectorAll('img'); + this.applyOpacityToImages(images); + }); // Handle non-slotted direct content images const directImages = this.querySelectorAll('img'); @@ -150,15 +149,12 @@ export class PieCard extends LitElement implements CardProps { /** * Applies or removes opacity from the given images based on the `disabled` property. * - * @param {NodeListOf} images - The images to apply opacity to. + * @param images + * @private */ - private applyOpacityToImages (images: NodeListOf) { + private applyOpacityToImages(images: NodeListOf): void { images.forEach((img) => { - if (this.disabled) { - img.style.opacity = '50%'; - } else { - img.style.opacity = ''; - } + img.style.opacity = this.disabled ? '0.5' : ''; }); } @@ -167,8 +163,8 @@ export class PieCard extends LitElement implements CardProps { * * @param changedProperties */ - updated(changedProperties: PropertyValues): void { - if (changedProperties.has('disabled' as any)) { + updated(changedProperties: PropertyValues) : void { + if (changedProperties.has('disabled')) { this.updateImagesOpacity(); // Re-apply styles when disabled state changes } } From f2291e0fa49c6835e4825d6fe3fb318eb5748d08 Mon Sep 17 00:00:00 2001 From: Kevin Rodrigues Date: Tue, 24 Sep 2024 11:25:34 +0100 Subject: [PATCH 05/10] feat(pie-card): DSW-2426 fix lint errors --- packages/components/pie-card/src/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/pie-card/src/index.ts b/packages/components/pie-card/src/index.ts index 23ae0b481d..f74fb8ff33 100644 --- a/packages/components/pie-card/src/index.ts +++ b/packages/components/pie-card/src/index.ts @@ -134,7 +134,7 @@ export class PieCard extends LitElement implements CardProps { * * @private */ - private updateImagesOpacity(): void { + private updateImagesOpacity (): void { // Handle slotted images this.assignedElements.forEach((element) => { const images = element.querySelectorAll('img'); @@ -152,7 +152,7 @@ export class PieCard extends LitElement implements CardProps { * @param images * @private */ - private applyOpacityToImages(images: NodeListOf): void { + private applyOpacityToImages (images: NodeListOf): void { images.forEach((img) => { img.style.opacity = this.disabled ? '0.5' : ''; }); @@ -163,7 +163,7 @@ export class PieCard extends LitElement implements CardProps { * * @param changedProperties */ - updated(changedProperties: PropertyValues) : void { + updated (changedProperties: PropertyValues) : void { if (changedProperties.has('disabled')) { this.updateImagesOpacity(); // Re-apply styles when disabled state changes } From 855591588c6a6f401ffbcb77ddc5eb46106542f0 Mon Sep 17 00:00:00 2001 From: Kevin Rodrigues Date: Tue, 24 Sep 2024 13:46:07 +0100 Subject: [PATCH 06/10] feat(pie-card): DSW-2426 add visual tests --- .../pie-card/test/visual/pie-card.spec.ts | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/packages/components/pie-card/test/visual/pie-card.spec.ts b/packages/components/pie-card/test/visual/pie-card.spec.ts index 53237ad0a0..e575c886d3 100644 --- a/packages/components/pie-card/test/visual/pie-card.spec.ts +++ b/packages/components/pie-card/test/visual/pie-card.spec.ts @@ -1,4 +1,4 @@ -import { test } from '@sand4rt/experimental-ct-web'; +import { test, expect } from '@sand4rt/experimental-ct-web'; import percySnapshot from '@percy/playwright'; import type { PropObject, WebComponentPropValues, WebComponentTestInput, @@ -95,3 +95,57 @@ test.describe('PieCard - `padding` prop', async () => { await percySnapshot(page, `PIE Card - Padding values | batch number: ${index}`, percyWidths); })); }); + +test.describe('PieCard - Disabled Prop Visual Tests', () => { + const slotContent = ` +
+

Card title

+ Sample image +

Card content

+
`; + + const renderTestPieCard = (disabled: boolean) => ` + + ${slotContent} + `; + + test('should set image opacity to 50% when disabled is true', async ({ page, mount }) => { + const testComponent = renderTestPieCard(true); + + await mount( + WebComponentTestWrapper, + { + slots: { + component: testComponent.trim(), + }, + }, + ); + + const image = page.locator('img'); + const opacity = await image.evaluate(img => getComputedStyle(img).opacity); + + expect(opacity).toBe('0.5'); + + await percySnapshot(page, 'PIE Card - Disabled State & image set to opacity of 50%'); + }); + + test('should not set image opacity style when disabled is false', async ({ page, mount }) => { + const testComponent = renderTestPieCard(false); + + await mount( + WebComponentTestWrapper, + { + slots: { + component: testComponent.trim(), + }, + }, + ); + + const image = page.locator('img'); + const opacity = await image.evaluate(img => getComputedStyle(img).opacity); + + expect(opacity).not.toBe('0.5'); + + await percySnapshot(page, 'PIE Card - Enabled State & image opacity not set'); + }); +}); From acb6cca068eceecb7d133f26d28650800505e3c6 Mon Sep 17 00:00:00 2001 From: Kevin Rodrigues Date: Tue, 24 Sep 2024 13:49:09 +0100 Subject: [PATCH 07/10] feat(pie-card): DSW-2426 fix linting --- packages/components/pie-card/test/visual/pie-card.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/pie-card/test/visual/pie-card.spec.ts b/packages/components/pie-card/test/visual/pie-card.spec.ts index e575c886d3..26928a8f3c 100644 --- a/packages/components/pie-card/test/visual/pie-card.spec.ts +++ b/packages/components/pie-card/test/visual/pie-card.spec.ts @@ -122,7 +122,7 @@ test.describe('PieCard - Disabled Prop Visual Tests', () => { ); const image = page.locator('img'); - const opacity = await image.evaluate(img => getComputedStyle(img).opacity); + const opacity = await image.evaluate((img) => getComputedStyle(img).opacity); expect(opacity).toBe('0.5'); @@ -142,7 +142,7 @@ test.describe('PieCard - Disabled Prop Visual Tests', () => { ); const image = page.locator('img'); - const opacity = await image.evaluate(img => getComputedStyle(img).opacity); + const opacity = await image.evaluate((img) => getComputedStyle(img).opacity); expect(opacity).not.toBe('0.5'); From ab8e0ffb9173f97655fe5567d208f91bd9d46c04 Mon Sep 17 00:00:00 2001 From: Kevin Rodrigues Date: Tue, 24 Sep 2024 14:46:11 +0100 Subject: [PATCH 08/10] feat(pie-card): DSW-2426 add image card sb example --- apps/pie-storybook/stories/pie-card.stories.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/apps/pie-storybook/stories/pie-card.stories.ts b/apps/pie-storybook/stories/pie-card.stories.ts index 34d195e0a0..109c7592b1 100644 --- a/apps/pie-storybook/stories/pie-card.stories.ts +++ b/apps/pie-storybook/stories/pie-card.stories.ts @@ -143,3 +143,18 @@ export const Default = createCardStory(); export const Outline = createCardStory({ variant: 'outline' }); export const Inverse = createCardStory({ variant: 'inverse' }, { bgColor: 'dark (container-dark)' }); export const OutlineInverse = createCardStory({ variant: 'outline-inverse' }, { bgColor: 'dark (container-dark)' }); +export const CardWithImage = createCardStory({ + ...defaultArgs, + slot: `
+

Card title

+

Card content

+

Lorem ipsum dolor sit amet + consectetur adipisicing elit. + Fugiat dolore dolorem maxime, + quod, in minima esse fugit + distinctio, officia et soluta + dicta consequuntur commodi officiis + tempora asperiores aspernatur atque quas.

+ Sample image +
` +}); From 4af4d732daf00fe08c2bbf7ba2aa68b0eb12b935 Mon Sep 17 00:00:00 2001 From: Kevin Rodrigues Date: Wed, 25 Sep 2024 10:06:20 +0100 Subject: [PATCH 09/10] feat(pie-card): DSW-2426 pr comment --- packages/components/pie-card/src/index.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/components/pie-card/src/index.ts b/packages/components/pie-card/src/index.ts index f74fb8ff33..9d9999ed56 100644 --- a/packages/components/pie-card/src/index.ts +++ b/packages/components/pie-card/src/index.ts @@ -54,7 +54,7 @@ export class PieCard extends LitElement implements CardProps { public padding?: CardProps['padding']; @queryAssignedElements({ flatten: true }) - private assignedElements!: HTMLElement[]; + private assignedElements?: HTMLElement[]; /** * Renders the card as an anchor element. @@ -135,11 +135,13 @@ export class PieCard extends LitElement implements CardProps { * @private */ private updateImagesOpacity (): void { - // Handle slotted images - this.assignedElements.forEach((element) => { - const images = element.querySelectorAll('img'); - this.applyOpacityToImages(images); - }); + if (this.assignedElements) { + // Handle slotted images + this.assignedElements.forEach((element) => { + const images = element.querySelectorAll('img'); + this.applyOpacityToImages(images); + }); + } // Handle non-slotted direct content images const directImages = this.querySelectorAll('img'); From 96f9044928807b04ff04528f1a0e487b6e120b58 Mon Sep 17 00:00:00 2001 From: Kevin Rodrigues Date: Wed, 25 Sep 2024 15:46:48 +0100 Subject: [PATCH 10/10] feat(pie-card): DSW-2426 add further tests and update comments --- packages/components/pie-card/src/index.ts | 4 +-- .../pie-card/test/component/pie-card.spec.ts | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/packages/components/pie-card/src/index.ts b/packages/components/pie-card/src/index.ts index 9d9999ed56..c03517e94e 100644 --- a/packages/components/pie-card/src/index.ts +++ b/packages/components/pie-card/src/index.ts @@ -136,14 +136,14 @@ export class PieCard extends LitElement implements CardProps { */ private updateImagesOpacity (): void { if (this.assignedElements) { - // Handle slotted images + // Handle images nested inside slotted elements this.assignedElements.forEach((element) => { const images = element.querySelectorAll('img'); this.applyOpacityToImages(images); }); } - // Handle non-slotted direct content images + // Handle directly slotted images const directImages = this.querySelectorAll('img'); this.applyOpacityToImages(directImages); } diff --git a/packages/components/pie-card/test/component/pie-card.spec.ts b/packages/components/pie-card/test/component/pie-card.spec.ts index baab4bc8a8..498e4663a9 100644 --- a/packages/components/pie-card/test/component/pie-card.spec.ts +++ b/packages/components/pie-card/test/component/pie-card.spec.ts @@ -330,5 +330,30 @@ test.describe('PieCard - Component tests', () => { await expect(image).toHaveCSS('opacity', '0.5'); }); }); + + test.describe('when an image exists and the prop `disabled` is set to `false`', () => { + test('should not set the opacity style on the image element', async ({ mount, page }) => { + // Arrange + const slottedImageContent = `
+ Sample Image +
`; + + await mount(PieCard, { + props: { + disabled: false, + } as CardProps, + slots: { + default: slottedImageContent, + }, + }); + + // Act + const component = page.locator('[data-test-id="slot-content"]'); + const image = component.locator('img'); + + // Assert the image has the correct opacity + await expect(image).not.toHaveCSS('opacity', '0.5'); + }); + }); }); });