Skip to content

Commit

Permalink
feat(pie-checkbox): DSW-2182 moves checkbox label to a slot instead o…
Browse files Browse the repository at this point in the history
…f a prop
  • Loading branch information
dandel10n committed Jul 19, 2024
1 parent c68f230 commit d0d08ec
Show file tree
Hide file tree
Showing 10 changed files with 184 additions and 106 deletions.
30 changes: 6 additions & 24 deletions apps/pie-storybook/stories/pie-checkbox-group.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,30 +89,12 @@ const Template = ({
status=${ifDefined(status)}
?disabled="${disabled}"
>
<pie-checkbox
name="checkbox-one"
label="checkbox 1">
</pie-checkbox>
<pie-checkbox
name="checkbox-two"
label="checkbox 2 longer label">
</pie-checkbox>
<pie-checkbox
name="checkbox-three"
label="checkbox 3">
</pie-checkbox>
<pie-checkbox
name="checkbox-three"
label="checkbox 4">
</pie-checkbox>
<pie-checkbox
name="checkbox-three"
label="checkbox 5 even longer label">
</pie-checkbox>
<pie-checkbox
name="checkbox-three"
label="checkbox 6">
</pie-checkbox>
<pie-checkbox name="checkbox-one">checkbox 1</pie-checkbox>
<pie-checkbox name="checkbox-two">checkbox 2 longer label</pie-checkbox>
<pie-checkbox name="checkbox-three">checkbox 3</pie-checkbox>
<pie-checkbox name="checkbox-three">checkbox 4</pie-checkbox>
<pie-checkbox name="checkbox-three">checkbox 5 even longer label</pie-checkbox>
<pie-checkbox name="checkbox-three">checkbox 6</pie-checkbox>
</pie-checkbox-group>
</div>
`;
Expand Down
26 changes: 10 additions & 16 deletions apps/pie-storybook/stories/pie-checkbox.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,14 @@ import { CheckboxProps, defaultProps, statusTypes } from '@justeattakeaway/pie-c

import { action } from '@storybook/addon-actions';
import { type StoryMeta } from '../types';
import { createStory, type TemplateFunction } from '../utilities';
import { createStory, type TemplateFunction, sanitizeAndRenderHTML } from '../utilities';

type CheckboxStoryMeta = StoryMeta<CheckboxProps>;

const defaultArgs: CheckboxProps = {
...defaultProps,
name: 'name',
label: 'Label',
checked: false,
defaultChecked: false,
disabled: false,
indeterminate: false,
required: false,
slot: 'Label',
};

const checkboxStoryMeta: CheckboxStoryMeta = {
Expand All @@ -39,12 +34,9 @@ const checkboxStoryMeta: CheckboxStoryMeta = {
},
},

label: {
description: 'The visible label for the checkbox',
slot: {
description: 'Content to place as a checkbox label',
control: 'text',
defaultValue: {
summary: defaultArgs.label,
},
},

checked: {
Expand Down Expand Up @@ -118,14 +110,14 @@ export default checkboxStoryMeta;
const Template = ({
value,
name,
label,
checked,
defaultChecked,
disabled,
indeterminate,
required,
assistiveText,
status,
slot,
}: CheckboxProps) => {
function onChange (event: CustomEvent) {
action('change')({
Expand All @@ -137,7 +129,6 @@ const Template = ({
<pie-checkbox
.value="${value}"
name="${ifDefined(name)}"
label="${ifDefined(label)}"
?checked="${checked}"
?defaultChecked="${defaultChecked}"
?disabled="${disabled}"
Expand All @@ -146,6 +137,7 @@ const Template = ({
@change="${onChange}"
assistiveText="${ifDefined(assistiveText)}"
status=${ifDefined(status)}>
${sanitizeAndRenderHTML(slot)}
</pie-checkbox>
`;
};
Expand All @@ -160,6 +152,7 @@ const ExampleFormTemplate: TemplateFunction<CheckboxProps> = ({
required,
assistiveText,
status,
slot,
}: CheckboxProps) => {
function onChange (event: CustomEvent) {
action('change')({
Expand All @@ -172,15 +165,16 @@ const ExampleFormTemplate: TemplateFunction<CheckboxProps> = ({
<pie-checkbox
.value="${value}"
name="${ifDefined(name)}"
label="Pie Checkbox"
?checked="${checked}"
?defaultChecked="${defaultChecked}"
?disabled="${disabled}"
?indeterminate="${indeterminate}"
?required="${required}"
@change="${onChange}"
assistiveText="${ifDefined(assistiveText)}"
status=${ifDefined(status)}></pie-checkbox>
status=${ifDefined(status)}>
${sanitizeAndRenderHTML(slot)}
</pie-checkbox>
<button type="reset">Reset</button>
<button type="submit">Submit</button>
<script>
Expand Down
30 changes: 6 additions & 24 deletions packages/components/pie-checkbox-group/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,34 +87,16 @@ In your markup or JSX, you can then use these to set the properties for the `pie
```html
<!-- Native HTML -->
<pie-checkbox-group name="TESTNAME">
<pie-checkbox
name="my-checkbox-one"
label="Checkbox Label 1">
</pie-checkbox>
<pie-checkbox
name="my-checkbox-two"
label="Checkbox Label 2">
</pie-checkbox>
<pie-checkbox
name="my-checkbox-three"
label="Checkbox Label 3">
</pie-checkbox>
<pie-checkbox name="my-checkbox-one">Checkbox Label 1</pie-checkbox>
<pie-checkbox name="my-checkbox-two">Checkbox Label 2</pie-checkbox>
<pie-checkbox name="my-checkbox-three">Checkbox Label 3</pie-checkbox>
</pie-checkbox-group>

<!-- JSX -->
<PieCheckboxGroup name="TESTNAME">
<PieCheckbox
name="my-checkbox-one"
label="Checkbox Label 1">
</PieCheckbox>
<PieCheckbox
name="my-checkbox-two"
label="Checkbox Label 2">
</PieCheckbox>
<PieCheckbox
name="my-checkbox-three"
label="Checkbox Label 3">
</PieCheckbox>
<PieCheckbox name="my-checkbox-one">Checkbox Label 1</PieCheckbox>
<PieCheckbox name="my-checkbox-two">Checkbox Label 2</PieCheckbox>
<PieCheckbox name="my-checkbox-three">Checkbox Label 3</PieCheckbox>
</PieCheckboxGroup>
```

Expand Down
1 change: 1 addition & 0 deletions packages/components/pie-checkbox-group/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const assistiveTextId = 'assistive-text';

/**
* @tagname pie-checkbox-group
* @slot - Default slot
* @event {CustomEvent} pie-checkbox-group-disabled - triggered after the disabled state of the checkbox group changes.
*/
export class PieCheckboxGroup extends FormControlMixin(RtlMixin(LitElement)) implements CheckboxGroupProps {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,138 @@

import { test } from '@sand4rt/experimental-ct-web';
import percySnapshot from '@percy/playwright';
import { PieCheckboxGroup, CheckboxGroupProps } from '../../src/index.ts';
import type {
PropObject, WebComponentPropValues, WebComponentTestInput,
} from '@justeattakeaway/pie-webc-testing/src/helpers/defs.ts';
import {
getAllPropCombinations, splitCombinationsByPropertyValue,
} from '@justeattakeaway/pie-webc-testing/src/helpers/get-all-prop-combos.ts';
import { createTestWebComponent } from '@justeattakeaway/pie-webc-testing/src/helpers/rendering.ts';
import { WebComponentTestWrapper } from '@justeattakeaway/pie-webc-testing/src/helpers/components/web-component-test-wrapper/WebComponentTestWrapper.ts';
import { percyWidths } from '@justeattakeaway/pie-webc-testing/src/percy/breakpoints.ts';
import { setRTL } from '@justeattakeaway/pie-webc-testing/src/helpers/set-rtl-direction.ts';
import { PieAssistiveText } from '@justeattakeaway/pie-assistive-text';
import { PieCheckbox } from '@justeattakeaway/pie-checkbox';
import { PieCheckboxGroup } from '../../src/index.ts';
import { statusTypes } from '../../src/defs.ts';

test.describe('PieCheckboxGroup - Visual tests`', () => {
test('should display the PieCheckboxGroup component successfully', async ({ page, mount }) => {
await mount(PieCheckboxGroup, {
props: {} as CheckboxGroupProps,
});
const readingDirections = ['LTR', 'RTL'];

await percySnapshot(page, 'PieCheckboxGroup - Visual Test');
});
const props: PropObject = {
label: ['Group Label', ''],
status: statusTypes,
isInline: [true, false],
disabled: [true, false],
assistiveText: ['Assistive text', ''],
};

const renderTestPieCheckboxGroup = (propVals: WebComponentPropValues) => {
let attributes = '';

if (propVals.label) attributes += ` label="${propVals.label}"`;
if (propVals.assistiveText) attributes += ` assistiveText="${propVals.assistiveText}"`;
if (propVals.isInline) attributes += ` isInline="${propVals.isInline}"`;
if (propVals.disabled) attributes += ` disabled="${propVals.disabled}"`;

return `
<pie-checkbox-group ${attributes} status="${propVals.status}">
<pie-checkbox>Label</pie-checkbox>
<pie-checkbox checked>Label</pie-checkbox>
<pie-checkbox>Label</pie-checkbox>
</pie-checkbox-group>`;
};

const componentPropsMatrix: WebComponentPropValues[] = getAllPropCombinations(props);
const componentPropsMatrixByInlineState: Record<string, WebComponentPropValues[]> = splitCombinationsByPropertyValue(componentPropsMatrix, 'isInline');
const componentVariants: string[] = Object.keys(componentPropsMatrixByInlineState);

test.beforeEach(async ({ mount }, testInfo) => {
testInfo.setTimeout(testInfo.timeout + 40000);

// This ensures the checkbox component is registered in the DOM for each test.
// It appears to add them to a Playwright cache which we understand is required for the tests to work correctly.
const checkboxPropComponent = await mount(PieCheckboxGroup);
await checkboxPropComponent.unmount();

const assistiveTextComponent = await mount(PieAssistiveText);
await assistiveTextComponent.unmount();

const checkboxComponent = await mount(PieCheckbox);
await checkboxComponent.unmount();
});

componentVariants.forEach((variant) => test(`should render all prop variations for the isInline state: ${variant}`, async ({ page, mount }) => {
for (const combo of componentPropsMatrixByInlineState[variant]) {
const testComponent: WebComponentTestInput = createTestWebComponent(combo, renderTestPieCheckboxGroup);
const propKeyValues = `
isInline: ${testComponent.propValues.isInline},
disabled: ${testComponent.propValues.disabled},
label: ${testComponent.propValues.label ? 'with label' : 'no label'},
status: ${testComponent.propValues.status},
assistiveText: ${testComponent.propValues.assistiveText ? 'with assistive text' : 'no assistive text'}`;

await mount(
WebComponentTestWrapper,
{
props: { propKeyValues },
slots: {
component: testComponent.renderedString.trim(),
},
},
);
}

await percySnapshot(page, `PIE Checkbox Group - isInline: ${variant}`, percyWidths);
}));

for (const dir of readingDirections) {
test(`Assistive text and statuses - ${dir}`, async ({ mount, page }) => {
if (dir === 'RTL') {
setRTL(page);
}

// Assistive text with no status
let testComponent: WebComponentTestInput = createTestWebComponent({ assistiveText: 'Assistive text', label: 'String' }, renderTestPieCheckboxGroup);
let propKeyValues = `assistiveText: ${testComponent.propValues.assistiveText}, label: ${testComponent.propValues.label}`;

await mount(
WebComponentTestWrapper,
{
props: { propKeyValues },
slots: {
component: testComponent.renderedString.trim(),
},
},
);

// Error + assistive text
testComponent = createTestWebComponent({ assistiveText: 'Error text', label: 'Group label', status: 'error' }, renderTestPieCheckboxGroup);
propKeyValues = `assistiveText: ${testComponent.propValues.assistiveText}, label: ${testComponent.propValues.label}, status: ${testComponent.propValues.status}`;

await mount(
WebComponentTestWrapper,
{
props: { propKeyValues },
slots: {
component: testComponent.renderedString.trim(),
},
},
);

// Success + assistive text
testComponent = createTestWebComponent({ assistiveText: 'Success text', label: 'Group label', status: 'success' }, renderTestPieCheckboxGroup);
propKeyValues = `assistiveText: ${testComponent.propValues.assistiveText}, label: ${testComponent.propValues.label}, status: ${testComponent.propValues.status}`;

await mount(
WebComponentTestWrapper,
{
props: { propKeyValues },
slots: {
component: testComponent.renderedString.trim(),
},
},
);

await percySnapshot(page, `PIE Checkbox Group - Assistive text and statuses - ${dir}`, percyWidths);
});
}
15 changes: 6 additions & 9 deletions packages/components/pie-checkbox/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ import { PieCheckbox } from '@justeattakeaway/pie-checkbox/dist/react';
| `name` | `string` | - | The name of the checkbox (used as a key/value pair with `value`). This is required in order to work properly with forms. |
| `value` | `string` `'on'` | The value of the input (used as a key/value pair in HTML forms with `name`). If not passed falls back to the html default value "on". |
| `required` | `boolean` | `false` | If true, the checkbox is required to be checked before submitting the form. If it is not in checked state, the component validity state will be invalid. |
| `label` | `string` | `''` | Text associated with the checkbox. If there is no label to provide, make sure to pass aria-label, aria-labelledby or aria-describedby attributes instead. |
| `disabled` | `boolean` | `false` | Indicates whether or not the checkbox is disabled. |
| `checked` | `boolean` | `false` | Controls whether or not the checkbox is checked. |
| `defaultChecked` | `boolean` | `false` | Sets the default checked state for the checkbox. This does not directly set the initial checked state when the page loads, use `checked` for that. If the checkbox is inside a form which is reset, the `checked` state will be updated to match `defaultChecked`. |
Expand All @@ -89,16 +88,14 @@ In your markup or JSX, you can then use these to set the properties for the `pie

```html
<!-- Native HTML -->
<pie-checkbox
name="mycheckbox"
label="Checkbox Label">
</pie-checkbox>
<pie-checkbox name="mycheckbox">Label</pie-checkbox>

<!-- Without a label it is necessary to pass aria-label or aria-labeledby attributes to the component -->
<pie-checkbox name="mycheckbox" aria-label="Label"></pie-checkbox>

<!-- JSX -->
<PieCheckbox
name="mycheckbox"
label="Checkbox Label">
</PieCheckbox>
<PieCheckbox name="mycheckbox">Label</PieCheckbox>
<PieCheckbox name="mycheckbox" aria-label="Label"></PieCheckbox>
```

## Events
Expand Down
10 changes: 10 additions & 0 deletions packages/components/pie-checkbox/src/checkbox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@
top: 55%;
left: 14%;

:host-context([dir="rtl"]) & {
left: unset;
right: 50%;
}

border-right: 2px solid transparent;
border-bottom: 2px solid transparent;

Expand Down Expand Up @@ -177,6 +182,11 @@
width: 0;
height: 2px;
background-color: white;

:host-context([dir="rtl"]) & {
left: unset;
right: 14%;
}
}

.c-checkbox {
Expand Down
Loading

0 comments on commit d0d08ec

Please sign in to comment.