Skip to content

Review 5143

Cindy Zhang edited this page Aug 23, 2026 · 1 revision

#5143 — fix(RadioListItem): make the whole row a click target

freddymeta · merged · view on GitHub

Verdict: approve · 2 rounds

These records predate the Review Presentation format for the most part, so the problem/solution/API/theme-target/breaking slots are only filled where the review itself carried them. Reviews from 2026-08-23 on use the full template.

Round 1 — dismissed · 2026-08-17

Thanks — checked the row in Chrome and the description, empty area, keyboard and nested button all behave.

Two before merge. The disabled label now dims twice: labelDisabled stacks with the Item's own 0.5 opacity, so it lands at 163,163,163 where the equivalent CheckboxListItem row stays readable. Drop the extra dim and let the Item carry it.

And the switch to fireEvent.click gives up the thing this PR relies on — userEvent refuses to click through pointer-events: none, fireEvent doesn't. Delete the guard and that test still passes.

[Reviewed by Robohands]

Round 2 — approve · 2026-08-19

Thanks — both fixes hold up. The disabled row now dims exactly like CheckboxListItem (both land on rgb(139,139,139)), and the disabled test proves the guard again: I deleted Item's pointer-events: none locally and both tests fail.

Approving, but themingTargets is red so you won't be able to merge until it's green. The row passes size/selected/disabled to themeProps, but the doc still lists astryx-radio-list-item bare. One line, mirroring multi-selector-option:

{className: 'astryx-radio-list-item', visualProps: ['size'], states: ['selected', 'disabled']},

[Reviewed by Robohands, edited by the maintainer]

Clone this wiki locally