Skip to content

Commit 15bcdfb

Browse files
committed
fix: ♻️ Fixing review comments
1 parent 0fc28b5 commit 15bcdfb

6 files changed

Lines changed: 48 additions & 92 deletions

File tree

packages/eds-core-react/src/components/next/Radio/Radio.stories.tsx

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,19 @@ const Wrapper = ({
134134
</div>
135135
)
136136

137-
const AllVariants = () => (
137+
const AllVariants = ({ prefix = 'default' }: { prefix?: string }) => (
138138
<>
139-
<Radio label="Unchecked" name="all-variants" value="unchecked" />
140-
<Radio label="Checked" name="all-variants" value="checked" defaultChecked />
141-
<Radio label="Disabled" name="all-variants-disabled" disabled />
139+
<Radio label="Unchecked" name={`${prefix}-unchecked`} value="unchecked" />
140+
<Radio
141+
label="Checked"
142+
name={`${prefix}-checked`}
143+
value="checked"
144+
defaultChecked
145+
/>
146+
<Radio label="Disabled" name={`${prefix}-disabled`} disabled />
142147
<Radio
143148
label="Disabled checked"
144-
name="all-variants-disabled-checked"
149+
name={`${prefix}-disabled-checked`}
145150
disabled
146151
defaultChecked
147152
/>
@@ -154,13 +159,13 @@ export const Introduction: StoryFn<RadioProps> = (args) => {
154159

155160
export const Spacious: StoryFn<RadioProps> = () => (
156161
<Wrapper data-density="spacious">
157-
<AllVariants />
162+
<AllVariants prefix="spacious" />
158163
</Wrapper>
159164
)
160165

161166
export const Comfortable: StoryFn<RadioProps> = () => (
162167
<Wrapper data-density="comfortable">
163-
<AllVariants />
168+
<AllVariants prefix="comfortable" />
164169
</Wrapper>
165170
)
166171

@@ -216,6 +221,14 @@ export const GroupedRadio: StoryFn<RadioProps> = () => (
216221
</Wrapper>
217222
</fieldset>
218223
)
224+
GroupedRadio.parameters = {
225+
docs: {
226+
description: {
227+
story:
228+
'Radio buttons with the same `name` form a group. Use arrow keys (↑↓ or ←→) to navigate and select within the group.',
229+
},
230+
},
231+
}
219232

220233
export const WithoutVisibleLabel: StoryFn<RadioProps> = () => (
221234
<div style={{ display: 'flex', gap: '32px', alignItems: 'flex-start' }}>

packages/eds-core-react/src/components/next/Radio/Radio.test.tsx

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -104,21 +104,6 @@ describe('Radio (next)', () => {
104104
)
105105
expect(await axe(container)).toHaveNoViolations()
106106
})
107-
108-
it('connects helperMessage to input via aria-describedby', () => {
109-
render(
110-
<Radio
111-
label="Test Label"
112-
name="test"
113-
helperMessage="Helper text for a11y"
114-
/>,
115-
)
116-
117-
const radio = screen.getByRole('radio')
118-
const helperMessage = screen.getByText('Helper text for a11y')
119-
120-
expect(radio).toHaveAttribute('aria-describedby', helperMessage.id)
121-
})
122107
})
123108

124109
describe('Interaction', () => {
@@ -170,26 +155,6 @@ describe('Radio (next)', () => {
170155
})
171156
})
172157

173-
describe('Field props', () => {
174-
it('supports indicator prop for required/optional text', () => {
175-
render(<Radio label="Test Label" name="test" indicator="(Required)" />)
176-
177-
expect(screen.getByText('(Required)')).toBeInTheDocument()
178-
})
179-
180-
it('supports helperMessage prop', () => {
181-
render(
182-
<Radio
183-
label="Test Label"
184-
name="test"
185-
helperMessage="This is a helper message"
186-
/>,
187-
)
188-
189-
expect(screen.getByText('This is a helper message')).toBeInTheDocument()
190-
})
191-
})
192-
193158
describe('Radio group behavior', () => {
194159
it('only one radio in a group can be selected', () => {
195160
render(

packages/eds-core-react/src/components/next/Radio/Radio.tsx

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,32 +7,22 @@ import {
77
import { Field } from '../Field'
88
import { Icon } from '../Icon'
99
import type { RadioProps } from './Radio.types'
10-
import './radio.css'
1110

1211
const classNames = (...classes: (string | boolean | undefined)[]) =>
1312
classes.filter(Boolean).join(' ')
1413

1514
export const Radio = forwardRef<HTMLInputElement, RadioProps>(function Radio(
16-
{
17-
label,
18-
disabled = false,
19-
indicator,
20-
helperMessage,
21-
id: providedId,
22-
...rest
23-
},
15+
{ label, disabled = false, id: providedId, ...rest },
2416
ref,
2517
) {
2618
const generatedId = useId()
2719
const inputId = providedId ?? generatedId
28-
const helperMessageId = `${inputId}-helper`
2920

3021
const radioInput = (
3122
<>
3223
<input
3324
type="radio"
3425
id={inputId}
35-
aria-describedby={helperMessage ? helperMessageId : undefined}
3626
className="eds-radio__input"
3727
disabled={disabled}
3828
ref={ref}
@@ -65,14 +55,7 @@ export const Radio = forwardRef<HTMLInputElement, RadioProps>(function Radio(
6555
data-space-proportions="squished"
6656
>
6757
{radioInput}
68-
<Field.Label htmlFor={inputId} indicator={indicator}>
69-
{label}
70-
</Field.Label>
71-
{helperMessage && (
72-
<Field.HelperMessage id={helperMessageId}>
73-
{helperMessage}
74-
</Field.HelperMessage>
75-
)}
58+
<Field.Label htmlFor={inputId}>{label}</Field.Label>
7659
</Field>
7760
)
7861
}

packages/eds-core-react/src/components/next/Radio/Radio.types.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,4 @@ import type { InputHTMLAttributes, ReactNode } from 'react'
33
export type RadioProps = {
44
/** Label for the radio button */
55
label?: ReactNode
6-
/** If true, the radio button will be disabled */
7-
disabled?: boolean
8-
/** Indicator text shown after the label, e.g. "(Required)" or "(Optional)" */
9-
indicator?: string
10-
/** Helper message shown below the radio button, useful for additional context */
11-
helperMessage?: ReactNode
126
} & Omit<InputHTMLAttributes<HTMLInputElement>, 'type'>

packages/eds-core-react/src/components/next/Radio/radio.css

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,37 @@
22
.eds-radio {
33
--_radio-icon-color: var(--eds-color-bg-fill-emphasis-default);
44
--_radio-hover-color: var(--eds-color-bg-fill-muted-default);
5+
--_radio-icon-size: var(--eds-sizing-icon-lg);
6+
/* Touch target: 36px spacious, 28px comfortable */
7+
--_radio-touch-target: 2.25rem;
58
cursor: pointer;
69
position: relative;
710
box-sizing: border-box;
811
}
912

13+
[data-density='comfortable'] .eds-radio {
14+
--_radio-touch-target: 1.75rem;
15+
}
16+
1017
.eds-radio--standalone {
1118
display: inline-flex;
1219
align-items: center;
1320
justify-content: center;
1421
position: relative;
15-
padding: 0.375rem; /* 6px for spacious: 24px icon + 12px = 36px */
22+
/* Padding calculated from touch target and icon size */
23+
padding: calc((var(--_radio-touch-target) - var(--_radio-icon-size)) / 2);
1624
}
1725

1826
.eds-field.eds-radio {
19-
/* Gap compensates for icon-wrapper negative margin (-4.8px) */
20-
gap: 0.925rem;
27+
/*
28+
* Gap must compensate for icon-wrapper negative margin (-4.8px inline).
29+
* This negative margin matches Figma's icon container padding for optical alignment.
30+
* Compensation: 4.8px (margin) + 2px (visual balance) ≈ 6.8px (0.425rem)
31+
*/
32+
--_radio-gap-compensation: 0.425rem;
33+
gap: calc(
34+
var(--eds-generic-gap-horizontal) + var(--_radio-gap-compensation)
35+
);
2136
padding-inline: var(--eds-selectable-space-horizontal);
2237
padding-block: var(--eds-selectable-space-vertical);
2338
width: auto;
@@ -89,21 +104,17 @@
89104
outline: none;
90105
}
91106

92-
.eds-radio:has(.eds-radio__input:focus-visible)::after {
93-
content: '';
94-
position: absolute;
95-
inset: -2px;
96-
border: 1px solid var(--eds-color-border-focus);
97-
border-radius: 6px;
98-
pointer-events: none;
107+
.eds-radio:has(.eds-radio__input:focus-visible) {
108+
outline: var(--eds-sizing-stroke-thin) solid var(--eds-color-border-focus);
109+
outline-offset: var(--eds-sizing-stroke-thick);
110+
border-radius: var(--eds-spacing-border-radius-rounded);
99111
}
100112

101-
.eds-radio--standalone:has(.eds-radio__input:focus-visible)::after {
113+
.eds-radio--standalone:has(.eds-radio__input:focus-visible) {
102114
border-radius: var(--eds-spacing-border-radius-pill);
103115
}
104116

105117
.eds-radio__icon {
106-
position: relative;
107118
pointer-events: none;
108119
fill: var(--_radio-icon-color);
109120
flex-shrink: 0;
@@ -138,14 +149,4 @@
138149
color: var(--eds-color-border-neutral-medium);
139150
cursor: not-allowed;
140151
}
141-
142-
.eds-field.eds-radio[data-color-appearance='accent'] {
143-
--_eds-field-helper-color: var(--eds-color-text-neutral-subtle);
144-
}
145-
146-
/* Comfortable density for standalone radio */
147-
/* Icon is 20px in comfortable (--eds-sizing-icon-lg: 1.25rem) */
148-
[data-density='comfortable'] .eds-radio--standalone {
149-
padding: 0.25rem; /* 4px: 20px icon + 8px = 28px */
150-
}
151152
}
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* EDS 2.0 Next Components - CSS */
2+
/* Define layer order - eds-components has lowest priority for easy user overrides */
3+
@layer eds-components;
4+
25
@import '@equinor/eds-tokens/css/variables';
36
@import './Field/field.css';
47
@import './Radio/radio.css';
5-
6-
/* Define layer order - eds-components has lowest priority for easy user overrides */
7-
@layer eds-components;

0 commit comments

Comments
 (0)