Skip to content

Commit

Permalink
fix(pie-card): DSW-2425 ensure the card is not interactive when it is…
Browse files Browse the repository at this point in the history
… disabled (#1930)

* fix(pie-card): DSW-2425 ensure the card is not interactive when it is disabled

* fix(pie-card): DSW-2425 general fixes

* fix(pie-card): DSW-2425 fix tests

* fix(pie-card): DSW-2425 add comment

* fix(pie-card): DSW-2425 update tests

* fix(pie-card): DSW-2425 update tests
  • Loading branch information
raoufswe authored Sep 30, 2024
1 parent b145354 commit e34c9c5
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 17 deletions.
7 changes: 7 additions & 0 deletions .changeset/metal-crabs-develop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@justeattakeaway/pie-card": patch
"pie-storybook": patch
"pie-docs": patch
---

[Fixed] - Ensure the card is not interactive when `disabled` is passed
2 changes: 1 addition & 1 deletion apps/pie-docs/src/components/card/code/props.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"type": "code",
"item": ["true", "false"]
},
"Whether or not the card should be disabled. This applies disabled styles and turns off interactivity.",
"Whether or not the card should be disabled. This applies disabled styles and turns off interactivity. <br /> If the card is used as a link, the `href` attribute will be removed so the link can no longer be navigated.",
{
"type": "code",
"item": ["false"]
Expand Down
12 changes: 10 additions & 2 deletions apps/pie-storybook/stories/pie-card.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { nothing } from 'lit';
import { html } from 'lit/static-html.js';
import { ifDefined } from 'lit/directives/if-defined.js';
import { type Meta } from '@storybook/web-components';
import { action } from '@storybook/addon-actions';

import '@justeattakeaway/pie-card';
import {
Expand Down Expand Up @@ -112,6 +113,8 @@ const cardStoryMeta: CardStoryMeta = {

export default cardStoryMeta;

const clickAction = action('clicked');

const Template: TemplateFunction<CardProps> = ({
tag,
href,
Expand All @@ -123,7 +126,10 @@ const Template: TemplateFunction<CardProps> = ({
variant,
padding,
isDraggable,
}) => html`
}) => {
const isButton = tag === 'button';

return html`
<pie-card
tag="${ifDefined(tag)}"
variant="${ifDefined(variant)}"
Expand All @@ -133,9 +139,11 @@ const Template: TemplateFunction<CardProps> = ({
?disabled="${disabled}"
.aria="${aria}"
padding="${padding || nothing}"
?isDraggable="${isDraggable}">
?isDraggable="${isDraggable}"
@click="${isButton ? clickAction : nothing}">
${sanitizeAndRenderHTML(slot)}
</pie-card>`;
};

const createCardStory = createStory<CardProps>(Template, defaultArgs);

Expand Down
8 changes: 4 additions & 4 deletions packages/components/pie-card/src/card.scss
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@
outline: none;
text-decoration: none;

&:focus-visible {
@include p.focus;
}

&.c-card--disabled {
--card-bg-color: var(--dt-color-disabled-01);

Expand Down Expand Up @@ -95,4 +91,8 @@
&.c-card--draggable {
@extend %has-grab-cursor;
}

&:focus-visible {
@include p.focus;
}
}
30 changes: 20 additions & 10 deletions packages/components/pie-card/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,28 +56,37 @@ export class PieCard extends LitElement implements CardProps {
@queryAssignedElements({ flatten: true })
private assignedElements?: HTMLElement[];

private onClickHandler (event: Event) {
if (this.disabled) {
// needed to intercept/prevent click events when the card is disabled.
event.preventDefault();
event.stopPropagation();
}
}

/**
* Renders the card as an anchor element.
*
* @private
*/
private renderAnchor (classes: ClassInfo): TemplateResult {
const paddingCSS = this.generatePaddingCSS();
const {
href, rel, target, disabled, aria,
} = this;

return html`
<a
class="${classMap(classes)}"
data-test-id="pie-card"
?disabled=${this.disabled}
href=${this.href || ''}
target=${this.target || nothing}
rel=${this.rel || nothing}
href=${ifDefined(href && !disabled ? href : undefined)}
target=${target || nothing}
rel=${rel || nothing}
role="link"
aria-label=${this.aria?.label || nothing}
aria-disabled=${this.disabled ? 'true' : 'false'}
aria-label=${aria?.label || nothing}
aria-disabled=${disabled ? 'true' : 'false'}
style=${ifDefined(paddingCSS)}>
<slot></slot>
</div>
<slot @slotchange=${this.handleSlotChange}></slot>
</a>`;
}

Expand Down Expand Up @@ -196,10 +205,11 @@ export class PieCard extends LitElement implements CardProps {
class="${classMap(classes)}"
data-test-id="pie-card"
role="button"
tabindex="0"
tabindex=${disabled ? '-1' : '0'}
aria-label=${aria?.label || nothing}
aria-disabled=${disabled ? 'true' : 'false'}
style=${paddingCSS || ''}>
style=${paddingCSS || ''}
@click=${this.onClickHandler}>
<slot @slotchange=${this.handleSlotChange}></slot>
</div>
</div>`;
Expand Down
109 changes: 109 additions & 0 deletions packages/components/pie-card/test/component/pie-card.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,5 +355,114 @@ test.describe('PieCard - Component tests', () => {
await expect(image).not.toHaveCSS('opacity', '0.5');
});
});

test.describe('when the prop `tag` is set to `button`', () => {
test('should set `aria-disabled` to `true` when disabled', async ({ mount, page }) => {
// Arrange
await mount(PieCard, {
props: {
tag: 'button',
disabled: true,
} as CardProps,
slots: {
default: slotContent,
},
});

// Act
const component = page.locator(componentSelector);

// Assert
await expect(component).toHaveAttribute('aria-disabled', 'true');
});

test('should set `tabindex` to `-1` when disabled', async ({ mount, page }) => {
// Arrange
await mount(PieCard, {
props: {
tag: 'button',
disabled: true,
} as CardProps,
slots: {
default: slotContent,
},
});

// Act
const component = page.locator(componentSelector);

// Assert
await expect(component).toHaveAttribute('tabindex', '-1');
});

test('should not trigger the click event when the tag prop is set to `button` and is `disabled`', async ({ mount, page }) => {
// Arrange
const messages: string[] = [];

await mount(PieCard, {
props: {
tag: 'button',
disabled: true,
} as CardProps,
slots: {
default: slotContent,
},
on: {
click: () => messages.push('1'),
},
});

// Act
const component = page.locator(componentSelector);
await page.evaluate(() => {
const card = document.querySelector('pie-card');
card?.shadowRoot?.querySelector('div')?.click();
});

// Assert
await expect(component).toBeDisabled();
expect(messages).toHaveLength(0);
});
});

test.describe('when the prop `tag` is set to `a`', () => {
test('should set `aria-disabled` to `true` when disabled', async ({ mount, page }) => {
// Arrange
await mount(PieCard, {
props: {
tag: 'a',
disabled: true,
} as CardProps,
slots: {
default: slotContent,
},
});

// Act
const component = page.locator(componentSelector);

// Assert
await expect(component).toHaveAttribute('aria-disabled', 'true');
});

test('should not set the href attribute when disabled', async ({ mount, page }) => {
// Arrange
await mount(PieCard, {
props: {
tag: 'a',
disabled: true,
} as CardProps,
slots: {
default: slotContent,
},
});

// Act
const component = page.locator(componentSelector);

// Assert
await expect(component).not.toHaveAttribute('href');
});
});
});
});

0 comments on commit e34c9c5

Please sign in to comment.