diff --git a/change/@ni-nimble-components-e5bfd621-8ece-4d9c-a6ea-eb128a70e8ac.json b/change/@ni-nimble-components-e5bfd621-8ece-4d9c-a6ea-eb128a70e8ac.json new file mode 100644 index 0000000000..5d44c0b333 --- /dev/null +++ b/change/@ni-nimble-components-e5bfd621-8ece-4d9c-a6ea-eb128a70e8ac.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Implement error states on nimble-radio-group", + "packageName": "@ni/nimble-components", + "email": "20542556+mollykreis@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/nimble-components/src/radio-group/index.ts b/packages/nimble-components/src/radio-group/index.ts index deeb9f87f0..fee994518a 100644 --- a/packages/nimble-components/src/radio-group/index.ts +++ b/packages/nimble-components/src/radio-group/index.ts @@ -1,10 +1,12 @@ import { RadioGroup as FoundationRadioGroup, - radioGroupTemplate as template, DesignSystem } from '@microsoft/fast-foundation'; import { Orientation } from '@microsoft/fast-web-utilities'; +import { attr } from '@microsoft/fast-element'; import { styles } from './styles'; +import { template } from './template'; +import type { ErrorPattern } from '../patterns/error/types'; declare global { interface HTMLElementTagNameMap { @@ -17,7 +19,13 @@ export { Orientation }; /** * A nimble-styled grouping element for radio buttons */ -export class RadioGroup extends FoundationRadioGroup {} +export class RadioGroup extends FoundationRadioGroup implements ErrorPattern { + @attr({ attribute: 'error-text' }) + public errorText?: string; + + @attr({ attribute: 'error-visible', mode: 'boolean' }) + public errorVisible = false; +} const nimbleRadioGroup = RadioGroup.compose({ baseName: 'radio-group', diff --git a/packages/nimble-components/src/radio-group/styles.ts b/packages/nimble-components/src/radio-group/styles.ts index 37d2ac8d93..3fc6f5f5a0 100644 --- a/packages/nimble-components/src/radio-group/styles.ts +++ b/packages/nimble-components/src/radio-group/styles.ts @@ -4,15 +4,19 @@ import { controlLabelDisabledFontColor, controlLabelFont, controlLabelFontColor, + smallPadding, standardPadding } from '../theme-provider/design-tokens'; +import { styles as errorStyles } from '../patterns/error/styles'; export const styles = css` ${display('inline-block')} + ${errorStyles} .positioning-region { display: flex; gap: ${standardPadding}; + position: relative; } :host([orientation='vertical']) .positioning-region { @@ -23,6 +27,12 @@ export const styles = css` flex-direction: row; } + .label-container { + display: flex; + gap: ${smallPadding}; + margin-bottom: ${smallPadding}; + } + slot[name='label'] { font: ${controlLabelFont}; color: ${controlLabelFontColor}; @@ -31,4 +41,9 @@ export const styles = css` :host([disabled]) slot[name='label'] { color: ${controlLabelDisabledFontColor}; } + + .error-icon { + margin-left: auto; + margin-right: ${smallPadding}; + } `; diff --git a/packages/nimble-components/src/radio-group/template.ts b/packages/nimble-components/src/radio-group/template.ts new file mode 100644 index 0000000000..4d538f2490 --- /dev/null +++ b/packages/nimble-components/src/radio-group/template.ts @@ -0,0 +1,37 @@ +import { elements, html, slotted } from '@microsoft/fast-element'; +import { Orientation } from '@microsoft/fast-web-utilities'; +import type { RadioGroup } from '.'; +import { errorTextTemplate } from '../patterns/error/template'; +import { iconExclamationMarkTag } from '../icons/exclamation-mark'; + +/* eslint-disable @typescript-eslint/indent */ +export const template = html` + +`; diff --git a/packages/nimble-components/src/radio-group/tests/radio-group.foundation.spec.ts b/packages/nimble-components/src/radio-group/tests/radio-group.foundation.spec.ts new file mode 100644 index 0000000000..cdf0d1b8eb --- /dev/null +++ b/packages/nimble-components/src/radio-group/tests/radio-group.foundation.spec.ts @@ -0,0 +1,505 @@ +// Based on tests in FAST repo: https://github.com/microsoft/fast/blob/913c27e7e8503de1f7cd50bdbc9388134f52ef5d/packages/web-components/fast-foundation/src/radio-group/radio-group.spec.ts + +import { DOM } from '@microsoft/fast-element'; +import { + Radio, + radioTemplate as itemTemplate +} from '@microsoft/fast-foundation'; +import { Orientation } from '@microsoft/fast-web-utilities'; +import { RadioGroup } from '..'; +import { fixture } from '../../utilities/tests/fixture'; +import { template } from '../template'; +import { radioTag } from '../../radio'; + +// eslint-disable-next-line @typescript-eslint/naming-convention +const FASTRadioGroup = RadioGroup.compose({ + baseName: 'radio-group', + template +}); + +// TODO: Need to add tests for keyboard handling & focus management +describe('Radio Group', () => { + // eslint-disable-next-line @typescript-eslint/naming-convention + const FASTRadio = Radio.compose({ + baseName: 'radio', + template: itemTemplate + }); + + async function setup(): Promise<{ + element: RadioGroup, + connect: () => Promise, + disconnect: () => Promise, + parent: HTMLElement, + radio1: Radio, + radio2: Radio, + radio3: Radio + }> { + const { element, connect, disconnect, parent } = await fixture([ + FASTRadioGroup(), + FASTRadio() + ]); + + const radio1 = document.createElement(radioTag); + const radio2 = document.createElement(radioTag); + const radio3 = document.createElement(radioTag); + + radio1.className = 'one'; + radio2.className = 'two'; + radio3.className = 'three'; + + element.appendChild(radio1); + element.appendChild(radio2); + element.appendChild(radio3); + + return { element, connect, disconnect, parent, radio1, radio2, radio3 }; + } + + it('should have a role of `radiogroup`', async () => { + const { element, connect, disconnect } = await setup(); + + await connect(); + + expect(element.getAttribute('role')).toEqual('radiogroup'); + + await disconnect(); + }); + + it("should set a `horizontal` class on the 'positioning-region' when an orientation of `horizontal` is provided", async () => { + const { element, connect, disconnect } = await setup(); + + element.orientation = Orientation.horizontal; + + await connect(); + + expect( + element.shadowRoot + ?.querySelector('.positioning-region') + ?.classList.contains('horizontal') + ).toBeTrue(); + + await disconnect(); + }); + + it("should set a `vertical` class on the 'positioning-region' when an orientation of `vertical` is provided", async () => { + const { element, connect, disconnect } = await setup(); + + element.orientation = Orientation.vertical; + + await connect(); + + expect( + element.shadowRoot + ?.querySelector('.positioning-region') + ?.classList.contains('vertical') + ).toBeTrue(); + + await disconnect(); + }); + + it("should set a default class on the 'positioning-region' of `horizontal` when no orientation is provided", async () => { + const { element, connect, disconnect } = await setup(); + + await connect(); + + expect( + element.shadowRoot + ?.querySelector('.positioning-region') + ?.classList.contains('horizontal') + ).toBeTrue(); + + await disconnect(); + }); + + it('should set the `aria-disabled` attribute equal to the `disabled` value', async () => { + const { element, connect, disconnect } = await setup(); + + element.disabled = true; + + await connect(); + + expect(element.getAttribute('aria-disabled')).toBe('true'); + + element.disabled = false; + + await DOM.nextUpdate(); + + expect(element.getAttribute('aria-disabled')).toBe('false'); + + await disconnect(); + }); + + it('should NOT set a default `aria-disabled` value when `disabled` is not defined', async () => { + const { element, connect, disconnect } = await setup(); + + await connect(); + + expect(element.getAttribute('aria-disabled')).toBe(null); + + await disconnect(); + }); + + it('should set all child radio elements to disabled when the`disabled` is passed', async () => { + const { element, connect, disconnect } = await setup(); + element.disabled = true; + + await connect(); + await DOM.nextUpdate(); + + expect(element.querySelector('.one')!.disabled).toBeTrue(); + expect(element.querySelector('.two')!.disabled).toBeTrue(); + expect(element.querySelector('.three')!.disabled).toBeTrue(); + + expect( + element.querySelector('.one')?.getAttribute('aria-disabled') + ).toBe('true'); + expect( + element.querySelector('.two')?.getAttribute('aria-disabled') + ).toBe('true'); + expect( + element.querySelector('.three')?.getAttribute('aria-disabled') + ).toBe('true'); + + await disconnect(); + }); + + it('should set the `aria-readonly` attribute equal to the `readonly` value', async () => { + const { element, connect, disconnect } = await fixture(FASTRadioGroup()); + + element.readOnly = true; + + await connect(); + + expect(element.getAttribute('aria-readonly')).toBe('true'); + + element.readOnly = false; + + await DOM.nextUpdate(); + + expect(element.getAttribute('aria-readonly')).toBe('false'); + + await disconnect(); + }); + + it('should NOT set a default `aria-readonly` value when `readonly` is not defined', async () => { + const { element, connect, disconnect } = await fixture(FASTRadioGroup()); + + await connect(); + + expect(element.getAttribute('aria-readonly')).toBe(null); + + await disconnect(); + }); + + it('should set all child radio elements to readonly when the`readonly` is passed', async () => { + const { element, connect, disconnect } = await setup(); + element.readOnly = true; + + await connect(); + await DOM.nextUpdate(); + + expect(element.querySelector('.one')!.readOnly).toBeTrue(); + expect(element.querySelector('.two')!.readOnly).toBeTrue(); + expect(element.querySelector('.three')!.readOnly).toBeTrue(); + + expect( + element.querySelector('.one')?.getAttribute('aria-readonly') + ).toBe('true'); + expect( + element.querySelector('.two')?.getAttribute('aria-readonly') + ).toBe('true'); + expect( + element.querySelector('.three')?.getAttribute('aria-readonly') + ).toBe('true'); + + await disconnect(); + }); + + it('should set tabindex of 0 to a child radio with a matching `value`', async () => { + const { element, connect, disconnect } = await fixture([ + FASTRadioGroup(), + FASTRadio() + ]); + + element.value = 'baz'; + + const radio1 = document.createElement(radioTag); + const radio2 = document.createElement(radioTag); + const radio3 = document.createElement(radioTag); + + radio1.className = 'one'; + radio2.className = 'two'; + radio3.className = 'three'; + + radio1.value = 'foo'; + radio2.value = 'bar'; + radio3.value = 'baz'; + + element.appendChild(radio1); + element.appendChild(radio2); + element.appendChild(radio3); + + await connect(); + await DOM.nextUpdate(); + + expect( + element.querySelectorAll(radioTag)[2]!.getAttribute('tabindex') + ).toBe('0'); + + await disconnect(); + }); + + it('should NOT set tabindex of 0 to a child radio if its value does not match the radiogroup `value`', async () => { + const { element, connect, disconnect } = await fixture([ + FASTRadioGroup(), + FASTRadio() + ]); + + element.value = 'baz'; + + const radio1 = document.createElement(radioTag); + const radio2 = document.createElement(radioTag); + const radio3 = document.createElement(radioTag); + + radio1.className = 'one'; + radio2.className = 'two'; + radio3.className = 'three'; + + radio1.value = 'foo'; + radio2.value = 'bar'; + radio3.value = 'baz'; + + element.appendChild(radio1); + element.appendChild(radio2); + element.appendChild(radio3); + + await connect(); + await DOM.nextUpdate(); + + expect( + element.querySelectorAll(radioTag)[0]!.getAttribute('tabindex') + ).toBe('-1'); + expect( + element.querySelectorAll(radioTag)[1]!.getAttribute('tabindex') + ).toBe('-1'); + + await disconnect(); + }); + + it('should set a child radio with a matching `value` to `checked`', async () => { + const { element, connect, disconnect } = await fixture([ + FASTRadioGroup(), + FASTRadio() + ]); + + element.value = 'baz'; + + const radio1 = document.createElement(radioTag); + const radio2 = document.createElement(radioTag); + const radio3 = document.createElement(radioTag); + + radio1.className = 'one'; + radio2.className = 'two'; + radio3.className = 'three'; + + radio1.value = 'foo'; + radio2.value = 'bar'; + radio3.value = 'baz'; + + element.appendChild(radio1); + element.appendChild(radio2); + element.appendChild(radio3); + + await connect(); + await DOM.nextUpdate(); + + expect(element.querySelectorAll(radioTag)[2]!.checked).toBeTrue(); + expect( + element.querySelectorAll(radioTag)[2]!.getAttribute('aria-checked') + ).toBe('true'); + + await disconnect(); + }); + + it('should set a child radio with a matching `value` to `checked` when value changes', async () => { + const { element, connect, disconnect } = await fixture([ + FASTRadioGroup(), + FASTRadio() + ]); + + element.value = 'baz'; + + const radio1 = document.createElement(radioTag); + const radio2 = document.createElement(radioTag); + const radio3 = document.createElement(radioTag); + + radio1.className = 'one'; + radio2.className = 'two'; + radio3.className = 'three'; + + radio1.value = 'foo'; + radio2.value = 'bar'; + radio3.value = 'baz'; + + element.appendChild(radio1); + element.appendChild(radio2); + element.appendChild(radio3); + + await connect(); + await DOM.nextUpdate(); + + element.value = 'foo'; + + await DOM.nextUpdate(); + + expect(element.querySelectorAll(radioTag)[0]!.checked).toBeTrue(); + expect( + element.querySelectorAll(radioTag)[0]!.getAttribute('aria-checked') + ).toBe('true'); + + await disconnect(); + }); + + it('should mark the last radio defaulted to checked as checked, the rest should not be checked', async () => { + const { element, connect, disconnect } = await fixture([ + FASTRadioGroup(), + FASTRadio() + ]); + + const radio1 = document.createElement(radioTag); + const radio2 = document.createElement(radioTag); + const radio3 = document.createElement(radioTag); + + radio1.className = 'one'; + radio2.className = 'two'; + radio3.className = 'three'; + + radio1.value = 'foo'; + radio2.value = 'bar'; + radio3.value = 'baz'; + + radio2.setAttribute('checked', ''); + radio3.setAttribute('checked', ''); + + element.appendChild(radio1); + element.appendChild(radio2); + element.appendChild(radio3); + + await connect(); + await DOM.nextUpdate(); + + const radios: NodeList = element.querySelectorAll(radioTag); + expect((radios[2] as HTMLInputElement).checked).toBeTrue(); + expect((radios[1] as HTMLInputElement).checked).toBeFalse(); + + await disconnect(); + }); + + it('should mark radio matching value on radio-group over any checked attributes', async () => { + const { element, connect, disconnect } = await fixture([ + FASTRadioGroup(), + FASTRadio() + ]); + + element.value = 'bar'; + + const radio1 = document.createElement(radioTag); + const radio2 = document.createElement(radioTag); + const radio3 = document.createElement(radioTag); + + radio1.className = 'one'; + radio2.className = 'two'; + radio3.className = 'three'; + + radio1.value = 'foo'; + radio2.value = 'bar'; + radio3.value = 'baz'; + + radio2.setAttribute('checked', ''); + radio3.setAttribute('checked', ''); + + element.appendChild(radio1); + element.appendChild(radio2); + element.appendChild(radio3); + + await connect(); + await DOM.nextUpdate(); + + const radios: NodeList = element.querySelectorAll(radioTag); + expect((radios[1] as HTMLInputElement).checked).toBeTrue(); + + // radio-group explicitly sets non-matching radio's checked to false if a value match was found, + // but the attribute should still persist. + expect( + (radios[2] as HTMLInputElement).hasAttribute('checked') + ).toBeTrue(); + expect((radios[2] as HTMLInputElement).checked).toBeFalse(); + + await disconnect(); + }); + + it('should NOT set a child radio to `checked` if its value does not match the radiogroup `value`', async () => { + const { element, connect, disconnect } = await fixture([ + FASTRadioGroup(), + FASTRadio() + ]); + + element.value = 'baz'; + + const radio1 = document.createElement(radioTag); + const radio2 = document.createElement(radioTag); + const radio3 = document.createElement(radioTag); + + radio1.className = 'one'; + radio2.className = 'two'; + radio3.className = 'three'; + + radio1.value = 'foo'; + radio2.value = 'bar'; + radio3.value = 'baz'; + + element.appendChild(radio1); + element.appendChild(radio2); + element.appendChild(radio3); + + await connect(); + await DOM.nextUpdate(); + + expect(element.querySelectorAll(radioTag)[0]!.checked).toBeFalse(); + expect( + element.querySelectorAll(radioTag)[0]!.getAttribute('aria-checked') + ).toBe('false'); + + expect(element.querySelectorAll(radioTag)[1]!.checked).toBeFalse(); + expect( + element.querySelectorAll(radioTag)[1]!.getAttribute('aria-checked') + ).toBe('false'); + + await disconnect(); + }); + + it('should allow resetting of elements by the parent form', async () => { + const { element, connect, disconnect, parent, radio1, radio2, radio3 } = await setup(); + + radio2.setAttribute('checked', ''); + + const form = document.createElement('form'); + form.appendChild(element); + parent.appendChild(form); + + await connect(); + + radio1.checked = true; + + expect(!!radio1.checked).toBeTrue(); + expect(!!radio2.checked).toBeFalse(); + expect(!!radio3.checked).toBeFalse(); + + form.reset(); + + expect(!!radio1.checked).toBeFalse(); + expect(!!radio2.checked).toBeTrue(); + expect(!!radio3.checked).toBeFalse(); + + await disconnect(); + }); +}); diff --git a/packages/storybook/src/nimble/radio-group/radio-group-matrix.stories.ts b/packages/storybook/src/nimble/radio-group/radio-group-matrix.stories.ts index 3f43b7c573..ce58d37df9 100644 --- a/packages/storybook/src/nimble/radio-group/radio-group-matrix.stories.ts +++ b/packages/storybook/src/nimble/radio-group/radio-group-matrix.stories.ts @@ -1,6 +1,7 @@ import type { StoryFn, Meta } from '@storybook/html'; import { html, ViewTemplate } from '@microsoft/fast-element'; import { Orientation } from '@microsoft/fast-web-utilities'; +import { standardPadding } from '../../../../nimble-components/src/theme-provider/design-tokens'; import { radioTag } from '../../../../nimble-components/src/radio'; import { radioGroupTag } from '../../../../nimble-components/src/radio-group'; import { @@ -8,7 +9,12 @@ import { sharedMatrixParameters, createMatrixThemeStory } from '../../utilities/matrix'; -import { disabledStates, DisabledState } from '../../utilities/states'; +import { + disabledStates, + DisabledState, + errorStates, + ErrorState +} from '../../utilities/states'; import { createStory } from '../../utilities/storybook'; import { hiddenWrapper } from '../../utilities/hidden'; import { textCustomizationWrapper } from '../../utilities/text-customization'; @@ -30,19 +36,23 @@ export default metadata; const component = ( [disabledName, disabled]: DisabledState, - [orientationName, orientation]: OrientationState + [orientationName, orientation]: OrientationState, + [errorName, errorVisible, errorText]: ErrorState ): ViewTemplate => html`<${radioGroupTag} orientation="${() => orientation}" ?disabled="${() => disabled}" + ?error-visible="${() => errorVisible}" + error-text="${() => errorText}" value="1" + style="width: 250px; margin: var(${standardPadding.cssCustomProperty});" > - + <${radioTag} value="1">Option 1 <${radioTag} value="2">Option 2 `; export const radioGroupThemeMatrix: StoryFn = createMatrixThemeStory( - createMatrix(component, [disabledStates, orientationStates]) + createMatrix(component, [disabledStates, orientationStates, errorStates]) ); export const hiddenRadioGroup: StoryFn = createStory( diff --git a/packages/storybook/src/nimble/radio-group/radio-group.stories.ts b/packages/storybook/src/nimble/radio-group/radio-group.stories.ts index 98825d3fb5..d9c31a51e0 100644 --- a/packages/storybook/src/nimble/radio-group/radio-group.stories.ts +++ b/packages/storybook/src/nimble/radio-group/radio-group.stories.ts @@ -8,6 +8,8 @@ import { apiCategory, createUserSelectedThemeStory, disabledDescription, + errorTextDescription, + errorVisibleDescription, slottedLabelDescription } from '../../utilities/storybook'; @@ -19,6 +21,8 @@ interface RadioGroupArgs { value: string; buttons: undefined; change: undefined; + errorVisible: boolean; + errorText: string; } interface RadioArgs { @@ -49,6 +53,9 @@ export const radioGroup: StoryObj = { ?disabled="${x => x.disabled}" name="${x => x.name}" value="${x => x.value}" + ?error-visible="${x => x.errorVisible}" + error-text="${x => x.errorText}" + style="min-width: 200px;" > <${radioTag} value="apple">Apple @@ -61,7 +68,9 @@ export const radioGroup: StoryObj = { orientation: Orientation.horizontal, disabled: false, name: 'fruit', - value: 'none' + value: 'none', + errorVisible: false, + errorText: 'Value is invalid' }, argTypes: { value: { @@ -105,6 +114,16 @@ export const radioGroup: StoryObj = { 'Event emitted when the user selects a new value in the radio group.', table: { category: apiCategory.events }, control: false + }, + errorText: { + name: 'error-text', + description: errorTextDescription, + table: { category: apiCategory.attributes } + }, + errorVisible: { + name: 'error-visible', + description: errorVisibleDescription, + table: { category: apiCategory.attributes } } } }; diff --git a/packages/storybook/src/utilities/storybook.ts b/packages/storybook/src/utilities/storybook.ts index a54e48010e..9a79b83c0c 100644 --- a/packages/storybook/src/utilities/storybook.ts +++ b/packages/storybook/src/utilities/storybook.ts @@ -195,7 +195,7 @@ export const placeholderDescription = (options: { }): string => `Placeholder text to display when no value has been entered in the ${options.componentName}.`; export const errorTextDescription = 'A message to be displayed explaining why the value is invalid. Only visible when `error-visible` is set.'; -export const errorVisibleDescription = 'When set to `true`, the `error-text` message will be displayed.'; +export const errorVisibleDescription = 'When set to `true`, an error indicator will be displayed within the control and the `error-text` message will be displayed.'; export const dropdownPositionDescription = (options: { componentName: string