Separate ListRow trailing control and fail fast on out-of-range Slider
ListRow with onClick now renders a container whose leading slot, title and meta form one button, with the trailing slot as a sibling outside it, so a trailing toggle or action is never nested in the row button and never triggers the row's action. Slider throws when value is outside [min, max] (including NaN) instead of rendering a fill that disagrees with the clamped native input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -11,18 +11,36 @@
|
|||||||
}
|
}
|
||||||
|
|
||||||
.interactive {
|
.interactive {
|
||||||
min-width: 44px;
|
padding: 0;
|
||||||
border: 0;
|
|
||||||
border-radius: var(--radius-md);
|
border-radius: var(--radius-md);
|
||||||
background: transparent;
|
|
||||||
font: inherit;
|
|
||||||
cursor: pointer;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
.interactive:hover {
|
.interactive:hover {
|
||||||
background: var(--color-neutral-200);
|
background: var(--color-neutral-200);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.main {
|
||||||
|
box-sizing: border-box;
|
||||||
|
display: flex;
|
||||||
|
align-items: center;
|
||||||
|
gap: var(--space-3);
|
||||||
|
flex: 1 1 auto;
|
||||||
|
min-width: 44px;
|
||||||
|
min-height: 44px;
|
||||||
|
padding: 14px 16px;
|
||||||
|
border: 0;
|
||||||
|
border-radius: var(--radius-md);
|
||||||
|
background: transparent;
|
||||||
|
color: inherit;
|
||||||
|
font: inherit;
|
||||||
|
text-align: left;
|
||||||
|
cursor: pointer;
|
||||||
|
}
|
||||||
|
|
||||||
|
.interactive > .trailing {
|
||||||
|
padding-right: 16px;
|
||||||
|
}
|
||||||
|
|
||||||
.leading,
|
.leading,
|
||||||
.trailing {
|
.trailing {
|
||||||
display: inline-flex;
|
display: inline-flex;
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { fireEvent, render, screen } from '@testing-library/react'
|
|||||||
import { describe, expect, it, vi } from 'vitest'
|
import { describe, expect, it, vi } from 'vitest'
|
||||||
import { expectMinHitArea } from '../test/hitArea.ts'
|
import { expectMinHitArea } from '../test/hitArea.ts'
|
||||||
import { ListRow } from './ListRow.tsx'
|
import { ListRow } from './ListRow.tsx'
|
||||||
|
import { Toggle } from './Toggle.tsx'
|
||||||
|
|
||||||
describe('ListRow', () => {
|
describe('ListRow', () => {
|
||||||
it('renders leading, title, meta and trailing slots without being interactive', () => {
|
it('renders leading, title, meta and trailing slots without being interactive', () => {
|
||||||
@@ -28,4 +29,29 @@ describe('ListRow', () => {
|
|||||||
fireEvent.click(row)
|
fireEvent.click(row)
|
||||||
expect(onClick).toHaveBeenCalledOnce()
|
expect(onClick).toHaveBeenCalledOnce()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('keeps a trailing toggle outside the row button so each control fires only its own action', () => {
|
||||||
|
const onClick = vi.fn()
|
||||||
|
const onCheckedChange = vi.fn()
|
||||||
|
render(
|
||||||
|
<ListRow
|
||||||
|
title="Dinner reminders"
|
||||||
|
meta="Push at 4 pm"
|
||||||
|
trailing={<Toggle aria-label="Dinner reminders" checked={false} onCheckedChange={onCheckedChange} />}
|
||||||
|
onClick={onClick}
|
||||||
|
/>,
|
||||||
|
)
|
||||||
|
const row = screen.getByRole('button', { name: /^Dinner reminders/ })
|
||||||
|
const toggle = screen.getByRole('switch', { name: 'Dinner reminders' })
|
||||||
|
expect(row).not.toContainElement(toggle)
|
||||||
|
expect(row.querySelector('button')).toBeNull()
|
||||||
|
|
||||||
|
fireEvent.click(toggle)
|
||||||
|
expect(onCheckedChange).toHaveBeenCalledExactlyOnceWith(true)
|
||||||
|
expect(onClick).not.toHaveBeenCalled()
|
||||||
|
|
||||||
|
fireEvent.click(row)
|
||||||
|
expect(onClick).toHaveBeenCalledOnce()
|
||||||
|
expect(onCheckedChange).toHaveBeenCalledOnce()
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
+17
-8
@@ -8,29 +8,38 @@ export type ListRowProps = {
|
|||||||
leading?: ReactNode
|
leading?: ReactNode
|
||||||
// Trailing slot: chevron, toggle, count or action.
|
// Trailing slot: chevron, toggle, count or action.
|
||||||
trailing?: ReactNode
|
trailing?: ReactNode
|
||||||
// When set, the whole row is a button.
|
// When set, the leading slot, title and meta form one button. The trailing
|
||||||
|
// slot stays a sibling outside it, so a trailing toggle or action is never
|
||||||
|
// nested in the row's button and never triggers the row's action.
|
||||||
onClick?: () => void
|
onClick?: () => void
|
||||||
className?: string
|
className?: string
|
||||||
}
|
}
|
||||||
|
|
||||||
export function ListRow({ title, meta, leading, trailing, onClick, className }: ListRowProps) {
|
export function ListRow({ title, meta, leading, trailing, onClick, className }: ListRowProps) {
|
||||||
const content = (
|
const main = (
|
||||||
<>
|
<>
|
||||||
{leading && <span className={styles.leading}>{leading}</span>}
|
{leading && <span className={styles.leading}>{leading}</span>}
|
||||||
<span className={styles.body}>
|
<span className={styles.body}>
|
||||||
<span className={styles.title}>{title}</span>
|
<span className={styles.title}>{title}</span>
|
||||||
{meta && <span className={styles.meta}>{meta}</span>}
|
{meta && <span className={styles.meta}>{meta}</span>}
|
||||||
</span>
|
</span>
|
||||||
{trailing && <span className={styles.trailing}>{trailing}</span>}
|
|
||||||
</>
|
</>
|
||||||
)
|
)
|
||||||
const classes = [styles.row, className].filter(Boolean).join(' ')
|
const trailingSlot = trailing && <span className={styles.trailing}>{trailing}</span>
|
||||||
if (onClick) {
|
if (onClick) {
|
||||||
return (
|
return (
|
||||||
<button type="button" className={`${classes} ${styles.interactive}`} onClick={onClick}>
|
<div className={[styles.row, styles.interactive, className].filter(Boolean).join(' ')}>
|
||||||
{content}
|
<button type="button" className={styles.main} onClick={onClick}>
|
||||||
</button>
|
{main}
|
||||||
|
</button>
|
||||||
|
{trailingSlot}
|
||||||
|
</div>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
return <div className={classes}>{content}</div>
|
return (
|
||||||
|
<div className={[styles.row, className].filter(Boolean).join(' ')}>
|
||||||
|
{main}
|
||||||
|
{trailingSlot}
|
||||||
|
</div>
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -47,4 +47,23 @@ describe('Slider', () => {
|
|||||||
'max (1) must be greater than min (1)',
|
'max (1) must be greater than min (1)',
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('accepts values at both ends of the range', () => {
|
||||||
|
render(
|
||||||
|
<>
|
||||||
|
<Slider aria-label="At min" min={0} max={1} value={0} readOnly />
|
||||||
|
<Slider aria-label="At max" min={0} max={1} value={1} readOnly />
|
||||||
|
</>,
|
||||||
|
)
|
||||||
|
expect(screen.getByRole('slider', { name: 'At min' }).style.getPropertyValue('--slider-fill')).toBe('0%')
|
||||||
|
expect(screen.getByRole('slider', { name: 'At max' }).style.getPropertyValue('--slider-fill')).toBe('100%')
|
||||||
|
})
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
[2, 'value (2) must be within [0, 1]'],
|
||||||
|
[-0.1, 'value (-0.1) must be within [0, 1]'],
|
||||||
|
[Number.NaN, 'value (NaN) must be within [0, 1]'],
|
||||||
|
])('fails fast when the value %s is outside the range', (value, message) => {
|
||||||
|
expect(() => render(<Slider aria-label="Bad" min={0} max={1} value={value} readOnly />)).toThrow(message)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -12,6 +12,9 @@ export type SliderProps = Omit<ComponentPropsWithRef<'input'>, 'type' | 'value'
|
|||||||
// Name it with aria-label or aria-labelledby.
|
// Name it with aria-label or aria-labelledby.
|
||||||
export function Slider({ value, min, max, className, style, ...rest }: SliderProps) {
|
export function Slider({ value, min, max, className, style, ...rest }: SliderProps) {
|
||||||
if (!(max > min)) throw new Error(`Slider: max (${max}) must be greater than min (${min})`)
|
if (!(max > min)) throw new Error(`Slider: max (${max}) must be greater than min (${min})`)
|
||||||
|
if (!(value >= min && value <= max)) {
|
||||||
|
throw new Error(`Slider: value (${value}) must be within [${min}, ${max}]`)
|
||||||
|
}
|
||||||
const fill = `${((value - min) / (max - min)) * 100}%`
|
const fill = `${((value - min) / (max - min)) * 100}%`
|
||||||
return (
|
return (
|
||||||
<input
|
<input
|
||||||
|
|||||||
Reference in New Issue
Block a user