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

Radio group error #2423

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Implement error states on nimble-radio-group",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
12 changes: 10 additions & 2 deletions packages/nimble-components/src/radio-group/index.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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',
Expand Down
15 changes: 15 additions & 0 deletions packages/nimble-components/src/radio-group/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -23,6 +27,12 @@ export const styles = css`
flex-direction: row;
}

.label-container {
display: flex;
gap: ${smallPadding};
margin-bottom: ${smallPadding};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this introduces some spacing even when there is no label. I'm not sure how much we care about the no-label use case, but would it be better to apply the margin-bottom to ::slotted([slot="label"]), .error-icon instead?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I debated this myself. The thing I didn't want to have happen, though, is for the control with no label to shift when error-visible was toggled, which is why I decide to leave the space there all the time.

I'll leave the conversation open for others to weigh in.

}

slot[name='label'] {
font: ${controlLabelFont};
color: ${controlLabelFontColor};
Expand All @@ -31,4 +41,9 @@ export const styles = css`
:host([disabled]) slot[name='label'] {
color: ${controlLabelDisabledFontColor};
}

.error-icon {
margin-left: auto;
margin-right: ${smallPadding};
}
`;
37 changes: 37 additions & 0 deletions packages/nimble-components/src/radio-group/template.ts
Original file line number Diff line number Diff line change
@@ -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<RadioGroup>`
<template
role="radiogroup"
aria-disabled="${x => x.disabled}"
aria-readonly="${x => x.readOnly}"
@click="${(x, c) => x.clickHandler(c.event as MouseEvent)}"
@keydown="${(x, c) => x.keydownHandler(c.event as KeyboardEvent)}"
@focusout="${(x, c) => x.focusOutHandler(c.event as FocusEvent)}"
>
<div class="label-container">
<slot name="label"></slot>
<${iconExclamationMarkTag}
severity="error"
class="error-icon"
></${iconExclamationMarkTag}>
</div>
<div
class="positioning-region ${x => (x.orientation === Orientation.horizontal ? 'horizontal' : 'vertical')}"
part="positioning-region"
>
<slot
${slotted({
property: 'slottedRadioButtons',
filter: elements('[role=radio]')
})}
></slot>
${errorTextTemplate}
</div>
</template>
`;
Loading