Skip to content

Radio: Remove aria-required and aria-invalid #5760

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

Merged
merged 5 commits into from
Mar 14, 2025
Merged

Radio: Remove aria-required and aria-invalid #5760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b4097ca
Remove lint disable; removes `aria-invalid` and `aria-required` from …
TylerJDev Mar 11, 2025
58cfec8
Add changeset
TylerJDev Mar 11, 2025
d4e6b51
Remove `validationStatus` prop
TylerJDev Mar 11, 2025
7a51596
Update changeset bump
TylerJDev Mar 11, 2025
bc31fb5
Merge branch 'main' into tylerjdev/remove-lint-disables
TylerJDev Mar 13, 2025
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/red-lies-vanish.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": minor
---

Radio: Removes `aria-invalid` and `aria-required` from `Radio`, as they are not supported on the element's role.
12 changes: 0 additions & 12 deletions packages/react/src/Radio/Radio.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type {ChangeEventHandler, InputHTMLAttributes, ReactElement} from 'react'
import React, {useContext} from 'react'
import type {SxProp} from '../sx'
import type {FormValidationStatus} from '../utils/types/FormValidationStatus'
import {RadioGroupContext} from '../RadioGroup/RadioGroup'
import {clsx} from 'clsx'
import classes from './Radio.module.css'
Expand Down Expand Up @@ -35,10 +34,6 @@ export type RadioProps = {
* Indicates whether the radio button must be checked before the form can be submitted
*/
required?: boolean
/**
* Only used to inform ARIA attributes. Individual radio inputs do not have validation styles.
*/
validationStatus?: FormValidationStatus
Copy link
Member Author

@TylerJDev TylerJDev Mar 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We don't advertise this prop in our docs, and it doesn't have any usage outside of the aria-* props which are removed in this PR. The prop itself didn't add any functionality and I don't think this would need to be a major release. There's only one place (outside of Dotcom) that appears to utilize this prop.

Any thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

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

@TylerJDev if you feel like we can safely roll this out then I'm okay with that 👍 I think it would fall under a breaking change but I get if it doesn't feel worth it to migrate something we don't advertise and doesn't have usage.

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely! I think this is one of those things that didn't really do anything, and was never adopted anywhere, so we can probably get away with just removing it 😄

} & InputHTMLAttributes<HTMLInputElement> &
SxProp

Expand All @@ -54,7 +49,6 @@ const Radio = React.forwardRef<HTMLInputElement, RadioProps>(
onChange,
sx: sxProp = defaultSxProp,
required,
validationStatus,
value,
className,
...rest
Expand All @@ -77,7 +71,6 @@ const Radio = React.forwardRef<HTMLInputElement, RadioProps>(

if (sxProp !== defaultSxProp) {
return (
// eslint-disable-next-line github/a11y-role-supports-aria-props
<Box
as="input"
sx={sxProp}
Expand All @@ -89,8 +82,6 @@ const Radio = React.forwardRef<HTMLInputElement, RadioProps>(
checked={checked}
aria-checked={checked ? 'true' : 'false'}
required={required}
aria-required={required ? 'true' : 'false'}
aria-invalid={validationStatus === 'error' ? 'true' : 'false'}
onChange={handleOnChange}
className={clsx(className, sharedClasses.Input, classes.Radio)}
{...rest}
Expand All @@ -99,7 +90,6 @@ const Radio = React.forwardRef<HTMLInputElement, RadioProps>(
}

return (
// eslint-disable-next-line github/a11y-role-supports-aria-props
<input
type="radio"
value={value}
Expand All @@ -109,8 +99,6 @@ const Radio = React.forwardRef<HTMLInputElement, RadioProps>(
checked={checked}
aria-checked={checked ? 'true' : 'false'}
required={required}
aria-required={required ? 'true' : 'false'}
aria-invalid={validationStatus === 'error' ? 'true' : 'false'}
onChange={handleOnChange}
className={clsx(className, sharedClasses.Input, classes.Radio)}
{...rest}
Expand Down
30 changes: 0 additions & 30 deletions packages/react/src/__tests__/Radio.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,34 +150,4 @@ describe('Radio', () => {

expect(radio).toHaveAttribute('aria-checked', 'true')
})

it('renders an invalid aria state when validation prop indicates an error', () => {
const handleChange = jest.fn()
const {getByRole, rerender} = render(<Radio {...defaultProps} onChange={handleChange} />)

const radio = getByRole('radio') as HTMLInputElement

expect(radio).toHaveAttribute('aria-invalid', 'false')

rerender(<Radio {...defaultProps} onChange={handleChange} validationStatus="success" />)

expect(radio).toHaveAttribute('aria-invalid', 'false')

rerender(<Radio {...defaultProps} onChange={handleChange} validationStatus="error" />)

expect(radio).toHaveAttribute('aria-invalid', 'true')
})

it('renders an aria state indicating the field is required', () => {
const handleChange = jest.fn()
const {getByRole, rerender} = render(<Radio {...defaultProps} onChange={handleChange} />)

const radio = getByRole('radio') as HTMLInputElement

expect(radio).toHaveAttribute('aria-required', 'false')

rerender(<Radio {...defaultProps} onChange={handleChange} required />)

expect(radio).toHaveAttribute('aria-required', 'true')
})
})
Loading