Skip to content
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

feat(pie-checkbox-group): DSW-1937 a11y improvements #1630

Merged
merged 4 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/heavy-moles-appear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@justeattakeaway/pie-assistive-text": minor
---

[Added] - New `isVisuallyHidden` prop.
7 changes: 7 additions & 0 deletions .changeset/hot-dingos-joke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@justeattakeaway/pie-checkbox-group": minor
---

[Added] - New `pie-checkbox-group-error` custom event dispatcher.
[Added] - When in error state component sets `assistiveText` prop for slotted PIE Checkboxes.
[Changed] - Limited `_slottedChildren` query to pie-checkbox custom elements.
6 changes: 6 additions & 0 deletions .changeset/swift-emus-wonder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@justeattakeaway/pie-checkbox": minor
---

[Added] - `visuallyHiddenError` private state and logic to hide pie-assistive-text visually.
[Added] - Event listener for `pie-checkbox-group-error` custom event.
5 changes: 5 additions & 0 deletions .changeset/two-lemons-appear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"pie-docs": patch
---

[Added] - New prop for PIE Assistive Text component props json.
14 changes: 14 additions & 0 deletions apps/pie-docs/src/components/assistive-text/code/props.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,20 @@
"default"
]
}
],
[
"isVisuallyHidden",
{
"type": "code",
"item": ["true", "false"]
},
"If true, hides the component visually but leaves it accessible for a11y technologies.",
{
"type": "code",
"item": [
false
xander-marjoram marked this conversation as resolved.
Show resolved Hide resolved
]
}
]
]
}
5 changes: 3 additions & 2 deletions apps/pie-storybook/stories/pie-checkbox.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import { html } from 'lit';
import { ifDefined } from 'lit/directives/if-defined.js';
/* eslint-disable import/no-duplicates */
import '@justeattakeaway/pie-checkbox';
import { CheckboxProps, defaultProps, statusTypes } from '@justeattakeaway/pie-checkbox';
import { CheckboxProps as CheckboxBaseProps, defaultProps, statusTypes } from '@justeattakeaway/pie-checkbox';
dandel10n marked this conversation as resolved.
Show resolved Hide resolved
/* eslint-enable import/no-duplicates */

import { action } from '@storybook/addon-actions';
import { type StoryMeta } from '../types';
import { type StoryMeta, SlottedComponentProps } from '../types';
dandel10n marked this conversation as resolved.
Show resolved Hide resolved
import { createStory, type TemplateFunction, sanitizeAndRenderHTML } from '../utilities';

type CheckboxProps = SlottedComponentProps<CheckboxBaseProps>;
type CheckboxStoryMeta = StoryMeta<CheckboxProps>;

const defaultArgs: CheckboxProps = {
Expand Down
8 changes: 3 additions & 5 deletions packages/components/pie-assistive-text/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@

1. [Introduction](#pie-assistive-text)
2. [Installation](#installation)
3. [Importing the component](#importing-the-component)
4. [Peer Dependencies](#peer-dependencies)
5. [Props](#props)
6. [Slots](#slots)
6. [Contributing](#contributing)
3. [Documentation](#documentation)
4. [Questions](#questions)
5. [Contributing](#contributing)

## pie-assistive-text

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,7 @@
display: inline-flex;
}
}

.c-assistiveText--isVisuallyHidden {
@include p.visually-hidden;
}
6 changes: 6 additions & 0 deletions packages/components/pie-assistive-text/src/defs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,16 @@ export interface AssistiveTextProps {
* What variant the assistive text should be such as default, error or success.
*/
variant?: typeof variants[number];

/**
* If true, hides the component visually but leaves it accessible for a11y technologies.
*/
isVisuallyHidden?: boolean;
}

export type DefaultProps = ComponentDefaultProps<AssistiveTextProps>;

export const defaultProps: DefaultProps = {
variant: 'default',
isVisuallyHidden: false,
};
23 changes: 16 additions & 7 deletions packages/components/pie-assistive-text/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {

import { property } from 'lit/decorators.js';
import { validPropertyValues, defineCustomElement } from '@justeattakeaway/pie-webc-core';
import { ifDefined } from 'lit/directives/if-defined.js';
import { classMap } from 'lit/directives/class-map.js';
import '@justeattakeaway/pie-icons-webc/dist/IconAlertCircle.js';
import '@justeattakeaway/pie-icons-webc/dist/IconCheckCircle.js';

Expand All @@ -23,12 +23,15 @@ const componentSelector = 'pie-assistive-text';
export class PieAssistiveText extends LitElement implements AssistiveTextProps {
@property({ type: String })
@validPropertyValues(componentSelector, variants, defaultProps.variant)
public variant?: AssistiveTextProps['variant'] = defaultProps.variant;
public variant = defaultProps.variant;

@property({ type: Boolean })
public isVisuallyHidden = defaultProps.isVisuallyHidden;

/**
* Renders the assistive-text icon content.
* @private
*/
* Renders the assistive-text icon content.
* @private
*/
private renderIcon (): TemplateResult {
const { variant } = this;
return html`
Expand All @@ -39,13 +42,19 @@ export class PieAssistiveText extends LitElement implements AssistiveTextProps {
render () {
const {
variant,
isVisuallyHidden,
} = this;

const classes = {
'c-assistiveText': true,
'c-assistiveText--isVisuallyHidden': isVisuallyHidden,
};

return html`
<p
class="c-assistiveText"
class="${classMap(classes)}"
data-test-id="pie-assistive-text"
variant=${ifDefined(variant)}>
variant=${variant}>
${this.renderIcon()}
<slot></slot>
</p>`;
Expand Down
1 change: 1 addition & 0 deletions packages/components/pie-checkbox-group/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ In your markup or JSX, you can then use these to set the properties for the `pie
| Event | Type | Description |
|-------|------|-------------|
| `pie-checkbox-group-disabled` | `CustomEvent` | Triggered after the disabled state of the checkbox group changes. |
| `pie-checkbox-group-error` | `CustomEvent` | Triggered after the state of the checkbox group changes to error. |

## Contributing

Expand Down
1 change: 1 addition & 0 deletions packages/components/pie-checkbox-group/src/defs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export interface CheckboxGroupProps {
* @constant
*/
export const ON_CHECKBOX_GROUP_DISABLED = 'pie-checkbox-group-disabled';
export const ON_CHECKBOX_GROUP_ERROR = 'pie-checkbox-group-error';

export type DefaultProps = ComponentDefaultProps<CheckboxGroupProps, 'status' | 'disabled' | 'isInline'>;

Expand Down
23 changes: 14 additions & 9 deletions packages/components/pie-checkbox-group/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { classMap } from 'lit/directives/class-map.js';
import styles from './checkbox-group.scss?inline';
import {
ON_CHECKBOX_GROUP_DISABLED,
ON_CHECKBOX_GROUP_ERROR,
CheckboxGroupProps,
defaultProps,
statusTypes,
Expand All @@ -29,6 +30,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.
* @event {CustomEvent} pie-checkbox-group-error - triggered after the state of the checkbox group changes to error.
*/
export class PieCheckboxGroup extends FormControlMixin(RtlMixin(LitElement)) implements CheckboxGroupProps {
@property({ type: String })
Expand All @@ -50,20 +52,23 @@ export class PieCheckboxGroup extends FormControlMixin(RtlMixin(LitElement)) imp
@property({ type: Boolean, reflect: true })
public disabled = defaultProps.disabled;

@queryAssignedElements({}) _slottedChildren!: Array<HTMLElement>;
@queryAssignedElements({ selector: 'pie-checkbox' }) _slottedChildren!: Array<HTMLElement>;
xander-marjoram marked this conversation as resolved.
Show resolved Hide resolved

private _handleDisabled () : void {
jamieomaguire marked this conversation as resolved.
Show resolved Hide resolved
if (this._slottedChildren) {
this._slottedChildren.forEach((child) => {
child.dispatchEvent(new CustomEvent(ON_CHECKBOX_GROUP_DISABLED, { bubbles: false, composed: false, detail: { disabled: this.disabled } }));
});
}
this._slottedChildren?.forEach((child) => child.dispatchEvent(new CustomEvent(ON_CHECKBOX_GROUP_DISABLED, {
bubbles: false, composed: false, detail: { disabled: this.disabled },
})));
}

private _handleStatus () : void {
if (this._slottedChildren) {
this._slottedChildren.forEach((child) => child.setAttribute('status', this.status));
}
this._slottedChildren?.forEach((child) => {
child.setAttribute('status', this.status);

if (this.status === 'error' && this.assistiveText) {
child.setAttribute('assistiveText', this.assistiveText);
child.dispatchEvent(new CustomEvent(ON_CHECKBOX_GROUP_ERROR, { bubbles: false, composed: false, detail: { error: true } }));
}
});
}

protected updated (_changedProperties: PropertyValues<this>): void {
Expand Down
14 changes: 11 additions & 3 deletions packages/components/pie-checkbox/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { CheckboxProps, defaultProps, statusTypes } from './defs';
export * from './defs';

const componentSelector = 'pie-checkbox';
const assistiveTextIdValue = 'assistive-text';
const assistiveTextId = 'assistive-text';

/**
* @tagname pie-checkbox
Expand All @@ -34,6 +34,9 @@ export class PieCheckbox extends FormControlMixin(RtlMixin(LitElement)) implemen
@state()
private disabledByParent = false;

@state()
private visuallyHiddenError = false;

@property({ type: String })
public value = defaultProps.value;

Expand Down Expand Up @@ -69,12 +72,14 @@ export class PieCheckbox extends FormControlMixin(RtlMixin(LitElement)) implemen
super.connectedCallback();

this.addEventListener('pie-checkbox-group-disabled', (e: CustomEventInit) => { this.disabledByParent = e.detail.disabled; });
this.addEventListener('pie-checkbox-group-error', (e: CustomEventInit) => { this.visuallyHiddenError = e.detail.error; });
}

disconnectedCallback () : void {
super.disconnectedCallback();

this.removeEventListener('pie-checkbox-group-disabled', (e: CustomEventInit) => { this.disabledByParent = e.detail.disabled; });
this.removeEventListener('pie-checkbox-group-error', (e: CustomEventInit) => { this.visuallyHiddenError = e.detail.error; });
}

/**
Expand Down Expand Up @@ -149,6 +154,7 @@ export class PieCheckbox extends FormControlMixin(RtlMixin(LitElement)) implemen
name,
disabled,
disabledByParent,
visuallyHiddenError,
required,
indeterminate,
assistiveText,
Expand All @@ -174,7 +180,8 @@ export class PieCheckbox extends FormControlMixin(RtlMixin(LitElement)) implemen
?disabled=${componentDisabled}
?required=${required}
.indeterminate=${indeterminate}
aria-describedby=${ifDefined(assistiveText ? assistiveTextIdValue : undefined)}
aria-invalid=${status === 'error' ? 'true' : 'false'}
aria-describedby=${ifDefined(assistiveText ? assistiveTextId : undefined)}
@change=${this.handleChange}
data-test-id="checkbox-input"
/>
Expand All @@ -192,8 +199,9 @@ export class PieCheckbox extends FormControlMixin(RtlMixin(LitElement)) implemen
</label>
${assistiveText ? html`
<pie-assistive-text
id="${assistiveTextIdValue}"
id="${assistiveTextId}"
variant=${status}
?isVisuallyHidden=${visuallyHiddenError}
data-test-id="pie-checkbox-assistive-text">
${assistiveText}
</pie-assistive-text>` : nothing}
Expand Down
Loading