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.

Sign up for GitHub

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

Jump to bottom

feat(RadioGroup): improve radio-group components a11y #196

Merged
merged 2 commits into from
Jan 20, 2023
Merged

feat(RadioGroup): improve radio-group components a11y #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
951f74a
feat: improve radio-group components a11y
amje Jun 15, 2022
4b5df18
feat: add aria-disabled prop
amje Jan 20, 2023
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
10 changes: 8 additions & 2 deletions src/components/RadioButton/RadioButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,16 @@ export const RadioButton = React.forwardRef<HTMLDivElement, RadioButtonProps>(fu
[],
);

const {optionsProps} = useRadioGroup({...props, options});
const {containerProps, optionsProps} = useRadioGroup({...props, options});

return (
<div ref={ref} style={style} className={b({size, width}, className)} data-qa={qa}>
<div
{...containerProps}
ref={ref}
style={style}
className={b({size, width}, className)}
data-qa={qa}
>
<div
ref={plateRef}
className={b('plate')}
Expand Down
6 changes: 4 additions & 2 deletions src/components/RadioButton/__stories__/RadioButton.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ const options: RadioButtonOption[] = [
{value: 'Value 3', content: 'Value 3'},
];

const DefaultTemplate: Story<RadioButtonProps> = (args) => <RadioButton {...args} />;
const DefaultTemplate: Story<RadioButtonProps> = (args) => {
const [value, setValue] = React.useState<string>(options[0].value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use storybook controls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it able to set control value from callback? Otherwise example wont be interactive.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, storybook is not intended to be interactive, it is more like UI snapshots than demos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's kinda weird when you open a demo and it's not working properly. And the only way to change the value is to do it by hand.

return <RadioButton {...args} value={value} onUpdate={setValue} />;
};
export const Default = DefaultTemplate.bind({});
Default.args = {
value: options[0].value,
options,
};

Expand Down
10 changes: 8 additions & 2 deletions src/components/RadioGroup/RadioGroup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,16 @@ export const RadioGroup = React.forwardRef<HTMLDivElement, RadioGroupProps>(func
}));
}

const {optionsProps} = useRadioGroup({...props, options});
const {containerProps, optionsProps} = useRadioGroup({...props, options});

return (
<div ref={ref} style={style} className={b({size, direction}, className)} data-qa={qa}>
<div
{...containerProps}
ref={ref}
style={style}
className={b({size, direction}, className)}
data-qa={qa}
>
{optionsProps.map((optionProps) => (
<Radio
{...optionProps}
Expand Down
6 changes: 4 additions & 2 deletions src/components/RadioGroup/__stories__/RadioGroup.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ const options: RadioGroupOption[] = [
{value: 'Value 3', content: 'Value 3'},
];

const DefaultTemplate: Story<RadioGroupProps> = (args) => <RadioGroup {...args} />;
const DefaultTemplate: Story<RadioGroupProps> = (args) => {
const [value, setValue] = React.useState<string>(options[0].value);
return <RadioGroup {...args} value={value} onUpdate={setValue} />;
};
export const Default = DefaultTemplate.bind({});
Default.args = {
value: options[0].value,
options,
};

Expand Down
4 changes: 4 additions & 0 deletions src/components/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ export interface ControlGroupProps {
defaultValue?: string;
onUpdate?: (value: string) => void;
onChange?: (event: React.ChangeEvent<HTMLInputElement>) => void;
onFocus?: (event: React.FocusEvent<HTMLInputElement>) => void;
onBlur?: (event: React.FocusEvent<HTMLInputElement>) => void;
disabled?: boolean;
options?: ControlGroupOption[];
'aria-label'?: string;
'aria-labelledby'?: string;
}
23 changes: 21 additions & 2 deletions src/components/utils/useRadioGroup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,17 @@ import {ControlGroupProps} from '../types';
import {useUniqId} from './useUniqId';

export function useRadioGroup(props: ControlGroupProps) {
const {name, value, defaultValue, options = [], disabled, onUpdate, onChange} = props;
const {
name,
value,
defaultValue,
options = [],
disabled,
onUpdate,
onChange,
onFocus,
onBlur,
} = props;

const controlId = useUniqId();
const [valueState, setValueState] = React.useState(
Expand All @@ -28,14 +38,23 @@ export function useRadioGroup(props: ControlGroupProps) {
[isControlled, onUpdate, onChange],
);

const containerProps = {
role: 'radiogroup',
'aria-disabled': disabled,
'aria-label': props['aria-label'],
'aria-labelledby': props['aria-labelledby'],
amje marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +44 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

if aria-label set, aria-labelledby should not be setted

Copy link
Contributor

Choose a reason for hiding this comment

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

and vice versa

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 i was wrong

};

const optionsProps = options.map((option) => ({
name: name || controlId,
value: String(option.value),
content: option.content,
checked: currentValue === String(option.value),
disabled: disabled || option.disabled,
onChange: handleChange,
onFocus: onFocus,
onBlur: onBlur,
}));

return {optionsProps};
return {containerProps, optionsProps};
}