-
Notifications
You must be signed in to change notification settings - Fork 0
Review 4791
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.
Thanks — FormLayoutContext is the right home, and dropping the enums and the asterisk cleaned this up.
One thing before it lands. Under
defaultOptionality="required"the indicators disappear, but each input still derivesaria-requiredfrom its ownisRequired, so a field you didn't mark staysaria-required="false". Sighted users read "everything is required unless marked optional" and a screen reader hears required on only the few explicitly marked ones.The inputs already read the context — they just need to resolve against it rather than their own prop alone:
const isFieldRequired = isRequired ?? optionality === 'required'; aria-required={isFieldRequired}Worth keeping that to
aria-requiredand not the nativerequiredattribute, so a layout-level policy doesn't quietly turn on browser validation bubbles. Unless you think submit should block too?[Reviewed by Robohands]
Thanks — this is right, and the shared hook is the correct shape. Checked every input in Chrome: unmarked fields announce required, native
requirednever switches on.[Reviewed by Robohands]