From 1b7a99668becaf4125b005190f4ebc2187bf3b90 Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Thu, 12 Sep 2024 16:45:18 -0500 Subject: [PATCH 1/7] Create Required Inputs HLD --- specs/required-inputs/README.md | 66 +++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 specs/required-inputs/README.md diff --git a/specs/required-inputs/README.md b/specs/required-inputs/README.md new file mode 100644 index 0000000000..dc8110010e --- /dev/null +++ b/specs/required-inputs/README.md @@ -0,0 +1,66 @@ +# Required Input Controls + +## Problem Statement + +Input controls are often used in forms or property editor dialogs. Often the user is required to provide an input to the control before the form can be submitted or before the property changes can be saved. + +We need to provide a standardized way for the application to let the end-user know that a control is required. + +Initially, we need this for: +- `nimble-combobox` +- `nimble-number-field` +- `nimble-radio`/`nimble-radio-group` +- `nimble-select` +- `nimble-text-area` +- `nimble-text-field` + +## Links To Relevant Work Items and Reference Material + +- [Nimble issue](https://github.com/ni/nimble/issues/2100) +- [AzDO Feature](https://ni.visualstudio.com/DevCentral/_workitems/edit/2732543) +- [Figma design](https://www.figma.com/design/PO9mFOu5BCl8aJvFchEeuN/Nimble_Components?node-id=1295-47481) + +## Implementation / Design + +### API + +Our FAST base components provide built-in forms support via the [same API](https://www.w3schools.com/tags/att_input_required.asp) exposed by the native HTML `input`: + +- `required`: boolean attribute whose presence indicates that a value must be provided to submit + + For radio buttons/groups, only the button elements expose the `required` attribute. If any of the radio buttons in a group are marked required, the group is treated as required. + +### Visuals + +We will update our templates so that setting `required` causes a red asterisk to be displayed after the label, as per the visual design. For radio buttons/groups, the asterisk is on the group label, not individual button labels. + +### Form validation + +#### Vanilla HTML + +We would expect this to already work, since the FAST components we derive from provide forms support. Unfortunately, this support is broken for `nimble-combobox` and `nimble-radio`. + +- `nimble-combobox`: All that is needed is to forward the `required` attribute to the `input` in the template (which we have already forked from FAST). We will make this change. +- `nimble-radio`: + - `required` on a radio button does not take other buttons in the group into account. It will result in a validation error if _that specific_ radio is unchecked. + - Validity is only updated when the value or checked state changes, so if the radio button is initially unchecked and remains unchecked, it will remain marked valid, even though it violates the `required` constraint. + +Because the `nimble-radio` support has never worked, would take significant work to fix, and FAST is no longer accepting submissions to the archive branch we use, we **will not fix radio button forms support** as part of this feature. + +Forms support already works for `nimble-number-field`, `nimble-select`, `nimble-text-area`, and `nimble-text-field`. + +#### Angular + +The `required` attribute only plays a role in validation for template-driven forms (with reactive forms, it is only used for accessibility purposes). Nimble inputs already support setting the `required` attribute via template, but we will add it to the directive wrappers for completeness. + +#### Blazor + +Typically, form validation is handled via a `DataAnnotationsValidator`, and an input is treated as required if it is bound to a model property annotated with `RequiredAttribute` (i.e. `[Required]`). The Nimble component's `required` attribute is still needed to turn on the visual affordance, though, so we will add it to the Razor API. + +## Alternative Implementations / Designs + +None + +## Open Issues + +None From fc13317833ffb935b46e2a7d5cb2a53591d6d36d Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Thu, 12 Sep 2024 16:49:16 -0500 Subject: [PATCH 2/7] Add link to FAST issue --- specs/required-inputs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/required-inputs/README.md b/specs/required-inputs/README.md index dc8110010e..e6b2ab0fed 100644 --- a/specs/required-inputs/README.md +++ b/specs/required-inputs/README.md @@ -42,7 +42,7 @@ We would expect this to already work, since the FAST components we derive from p - `nimble-combobox`: All that is needed is to forward the `required` attribute to the `input` in the template (which we have already forked from FAST). We will make this change. - `nimble-radio`: - - `required` on a radio button does not take other buttons in the group into account. It will result in a validation error if _that specific_ radio is unchecked. + - `required` on a radio button [does not take other buttons in the group into account](https://github.com/microsoft/fast/issues/6866). It will result in a validation error if _that specific_ radio is unchecked. - Validity is only updated when the value or checked state changes, so if the radio button is initially unchecked and remains unchecked, it will remain marked valid, even though it violates the `required` constraint. Because the `nimble-radio` support has never worked, would take significant work to fix, and FAST is no longer accepting submissions to the archive branch we use, we **will not fix radio button forms support** as part of this feature. From 4d8c2ce839f4eaa91e828f788bf50fa075529908 Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Mon, 16 Sep 2024 13:10:11 -0500 Subject: [PATCH 3/7] Clarify radio button behavior --- specs/required-inputs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/required-inputs/README.md b/specs/required-inputs/README.md index e6b2ab0fed..9ebc51419e 100644 --- a/specs/required-inputs/README.md +++ b/specs/required-inputs/README.md @@ -28,7 +28,7 @@ Our FAST base components provide built-in forms support via the [same API](https - `required`: boolean attribute whose presence indicates that a value must be provided to submit - For radio buttons/groups, only the button elements expose the `required` attribute. If any of the radio buttons in a group are marked required, the group is treated as required. + For radio buttons/groups, only the button elements expose the `required` attribute. If any of the radio buttons in a group are marked required, the group is treated as required. It does not matter if the button with `required` is disabled, hidden, etc. It is effectively as if the attribute is on the group of buttons, rather than a specific one. This matches the behavior of the native `input` with `type="radio"`. ### Visuals From f3941ebcfdec64b6738be4275528d4754a103918 Mon Sep 17 00:00:00 2001 From: m-akinc <7282195+m-akinc@users.noreply.github.com> Date: Tue, 17 Sep 2024 17:33:48 -0500 Subject: [PATCH 4/7] Update specs/required-inputs/README.md Co-authored-by: Jesse Attas --- specs/required-inputs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/required-inputs/README.md b/specs/required-inputs/README.md index 9ebc51419e..e3582b531b 100644 --- a/specs/required-inputs/README.md +++ b/specs/required-inputs/README.md @@ -28,7 +28,7 @@ Our FAST base components provide built-in forms support via the [same API](https - `required`: boolean attribute whose presence indicates that a value must be provided to submit - For radio buttons/groups, only the button elements expose the `required` attribute. If any of the radio buttons in a group are marked required, the group is treated as required. It does not matter if the button with `required` is disabled, hidden, etc. It is effectively as if the attribute is on the group of buttons, rather than a specific one. This matches the behavior of the native `input` with `type="radio"`. +For radio buttons/groups, only the button elements expose the `required` attribute. If any of the radio buttons in a group are marked required, the group is treated as required. It does not matter if the button with `required` is disabled, hidden, etc. It is effectively as if the attribute is on the group of buttons, rather than a specific one. This matches the behavior of the native `input` with `type="radio"`. ### Visuals From 7040c53120618e5557d8079bddae40f09b87aba5 Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Wed, 18 Sep 2024 19:26:00 -0500 Subject: [PATCH 5/7] Addressed Jesse's feedback --- specs/required-inputs/README.md | 48 ++++++++++++++++++++++-- specs/required-inputs/missing-value.png | Bin 0 -> 5573 bytes 2 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 specs/required-inputs/missing-value.png diff --git a/specs/required-inputs/README.md b/specs/required-inputs/README.md index e3582b531b..1746fbae7a 100644 --- a/specs/required-inputs/README.md +++ b/specs/required-inputs/README.md @@ -14,6 +14,8 @@ Initially, we need this for: - `nimble-text-area` - `nimble-text-field` +Any other controls that could possibly be considered inputs are out of scope. + ## Links To Relevant Work Items and Reference Material - [Nimble issue](https://github.com/ni/nimble/issues/2100) @@ -32,22 +34,52 @@ For radio buttons/groups, only the button elements expose the `required` attribu ### Visuals -We will update our templates so that setting `required` causes a red asterisk to be displayed after the label, as per the visual design. For radio buttons/groups, the asterisk is on the group label, not individual button labels. +We will update our templates so that setting `required` causes a red asterisk to be displayed after the label, as per the visual design. For radio buttons/groups, the asterisk is on the group label, not individual button labels. We have already forked some of the templates from FAST, but we will now need to fork the ones for `nimble-number-field`, `nimble-radio-group`, and `nimble-text-field`. + +This will be implemented using a styled span containing the text "*". The span must be outside of the `label` element so that it is not included in the accessible name. We will wrap the two in a `div` so that we can force a horizontal layout (`flex-direction: row`). This structure will also ensure that whatever the wrapping/ellipsizing behavior of the label is, the asterisk will always appear at the far right: + +```html +
+ + ${when(x => x.required, html` + * + `)} +
+``` + +Note that for text-field, text-area, and number-field, we will also remove the visual if the control has the `readonly` attribute set, since [validation is not enforced on a readonly input](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/readonly#attribute_interactions). This doesn't apply to select, combobox, or radio buttons, because they do not support `readonly`. + +Styles will be shared. ### Form validation #### Vanilla HTML -We would expect this to already work, since the FAST components we derive from provide forms support. Unfortunately, this support is broken for `nimble-combobox` and `nimble-radio`. +When the user attempts to submit a form and a `required` input is missing a value, a browser-specific indicator reports the validation error: + +![Missing value indicator](missing-value.png) -- `nimble-combobox`: All that is needed is to forward the `required` attribute to the `input` in the template (which we have already forked from FAST). We will make this change. +We would expect this to already work, since the FAST components we derive from provide forms support. Unfortunately, this support is broken or incomplete for multiple components: + +- `nimble-combobox`: + - In Chrome/Edge, a missing required value properly blocks form submission and updates the control's `validity` flags, but the visual indicator of the validation error is not shown, and instead an error is printed to the console: "An invalid form control with name='' is not focusable." Presumably, something is trying to focus the host element, though it delegates focus to the shadow DOM. The error comes from a call to [`form.requestSubmit()` in the FAST button code](https://github.com/microsoft/fast/blob/913c27e7e8503de1f7cd50bdbc9388134f52ef5d/packages/web-components/fast-foundation/src/button/button.ts#L221), which we cannot debug into. Things work properly in Firefox. + - Accessibility: the control is not marked/announced as required. Forwarding `required` to the `input` in the template (which we have already forked) solves this. +- `nimble-text-area`: + - Has same Chrome/Edge issue as `nimble-combobox`. +- `nimble-select`: + - Validation incorrectly reports a missing value until a value change has occurred. I.e. the initial value is not seen. This happens because we try to mirror the initial value to the proxy native `select` element (within [a call to `super.slottedOptionsChanged()`](https://github.com/ni/nimble/blob/ddad57c4c97da9504f8146ad48668f290dae5301/packages/nimble-components/src/select/index.ts#L331)) before we have [mirrored the child option elements](https://github.com/ni/nimble/blob/ddad57c4c97da9504f8146ad48668f290dae5301/packages/nimble-components/src/select/index.ts#L346). The native `select` will reject/ignore setting `value` if it doesn't correspond to the value of one of its child options (of which it has none, at that point). Right after mirroring the child options, there is a call to `updateValue()`, but because the value has already been updated earlier, it [short-circuits](https://github.com/ni/nimble/blob/ddad57c4c97da9504f8146ad48668f290dae5301/packages/nimble-components/src/select/index.ts#L268), skipping the code path that would have updated the proxy. I suspect we can find a reasonable fix for this. + - Accessibility: the control is not marked/announced as required. The accessible control (i.e. the one defining the `role`) is the `nimble-select` itself. Since this is not a native input, the accessibility tree doesn't see/understand the `required` attribute on it. We need to additionally set `aria-required` on it (bound to `required`). - `nimble-radio`: - `required` on a radio button [does not take other buttons in the group into account](https://github.com/microsoft/fast/issues/6866). It will result in a validation error if _that specific_ radio is unchecked. - Validity is only updated when the value or checked state changes, so if the radio button is initially unchecked and remains unchecked, it will remain marked valid, even though it violates the `required` constraint. +Forms support is complete and functional for `nimble-number-field` and `nimble-text-field`. + Because the `nimble-radio` support has never worked, would take significant work to fix, and FAST is no longer accepting submissions to the archive branch we use, we **will not fix radio button forms support** as part of this feature. -Forms support already works for `nimble-number-field`, `nimble-select`, `nimble-text-area`, and `nimble-text-field`. +We will file a Chromium bug for the issue affecting the `nimble-combobox` and `nimble-text-area` but **will not try to work around it.** #### Angular @@ -57,6 +89,14 @@ The `required` attribute only plays a role in validation for template-driven for Typically, form validation is handled via a `DataAnnotationsValidator`, and an input is treated as required if it is bound to a model property annotated with `RequiredAttribute` (i.e. `[Required]`). The Nimble component's `required` attribute is still needed to turn on the visual affordance, though, so we will add it to the Razor API. +## Documentation + +We will add documentation for the `required` attribute. No additional guidance is necessary. + +## Testing + +Visual matrix test cases will be added for the affected components. + ## Alternative Implementations / Designs None diff --git a/specs/required-inputs/missing-value.png b/specs/required-inputs/missing-value.png new file mode 100644 index 0000000000000000000000000000000000000000..c2161024272a650144e793db50f248d8132c4321 GIT binary patch literal 5573 zcma)=Wmr^EyM~95luk(ph9PC>&Y@-KknU~-lx_s1QyK(CkOrl5=008*i|2C9<*AgoLfXqNiR$ANJ-w-sg~E? zjE|?zLs|QG3Ff~SyVD&wK#=9m;w8rItO-I(Qyel!tct=)%CCj)wN-(Wm+tJAVni!SG@?{ zeFPk;h*1dDG&ZJnuWVG;Q%r1RK>d`RP3=zgH&QBMYwwAz#t_ePp;WgI(y{&J6kF-p zw`0gp!@+(nTIslOH|@x!s#)_Dr|w??;plihXm==*^UXO%`+o;^v6uIi`eOA2T$edU zv-o*`Q$S2ofifR5oT0VRz7f4xkz^^#Knki-#8JQU+wKt4t*rB=sTtexBtl69@!pRd z2aQ)NUPDPw`%54Y8_?XIm^@R9WJoN*hzF{tt^EOy+6KP0wM9xXwWB^g59vTy(Cl>{ z{5s$ImjB2^G4@7jwwJB6-E*)Q4Dx=P&6m__<1^JNm-oZ<$uLv+9D7#N76#$HF(oXd6v>CkCTZkt@Uye=(HtCLCSZtR=u5ob zkJdT4p@`mu|83K_{`O*rn^!|k5gc3_#j~rU9eg}^-yKOX{KhnZ>Hg}Fmn1Hbm4oAg ziH$R3qN=2%8VF6s1hcky?lOd7;3e-@wv6?GF}1ZXFv`Av--Jit@;EO}zeOx?w|q*x zm@alU)UP&+=W|^)Iy*c2qLc#mQeC|#CXX8Wu2W`pbFwy7XRGEx46P;NJ85}$n4BPS zr`VT3@7g2Bp8RPmJutYsAf*_tX@qzl5n)A2m|j~;!3Zxb>n2_sCjF3vBWv9mj6zD5 zJX_#jlvC7Nv}Ab;p$eKG%Xyxh1_6GG$*VdFU8@>)3Tr{ zuQ-_u!emtWp&0r$2)+Cgapq$85zqaS+egYvryB!ZSkjSr@xa*tf8%^gu_Nz{`%%gM zr4-yxMb`K_cg$8(Fvk-3Wzt=7E*RB_p|)ik&^i5zM95-CsthJ@h{f3 zw%0tFVhFWb?}k6Qe5el90miP)NBoyoxz+wRYmwWcD3vlaoyzF_anuypwy4v^D!lVp zPe12Pc;_Sc#olg4#Zq!Z-yJR4PnpwCS>arYD;o()B!8Vh#=OofDIy0<+%?+*0N1q=`&&i(PUX9J80R;d{*jm1MluRSVo z$vF}g=mWHMt4v-Yr}0mfBq|Fz4!VU%KE?fr)ou(z{NmBaI|*}Lm;5Tu_4RZ3hUt&27wu}!^yIF&7NA&zsBi>I;jh# zLR?B=Sd24*(#Hl(6FkN^BJYFYqc^>@Ip0xp8rUf;KP;W`$t^zFOHn-$Sn49SNcLGaE(lew}pJH>&#V3BX*Q}LAt%mbaI^`)P?)QJS=e$rI3`P?)AWAQ?~%`d{wDz zy2h=@*QGREv%7sHkk}{}Ckoo?PZ(40v z$spr6_H6TmD+g^>`Vi1zdolGndzGe{$M*-u3y|0 z^2=$mP>#oTjApv}yH?5~GNoDnsaK2rd`l_rFxFG0rW`#_Pe5XR>Ho4w$ zEF+===coPjg7%YGf~1=S9`qxDM?U)wo@B8*5>E(^b8HhD`f<`oM`VKU5^pIk?Tw)r zzCZ489rL^X^Ijn@9~U0ge=aAoi4yYhYr8F~#tUAXSg|L>d6NH$Xo+`I&ttuEI!BUD zneMN)INIOTg5vuWpzoHny#o4Xek*;Sukp2D?mG=U?z~pcUsW8yg_ALPHdK97yy`h( zwQ1qwo$SBP%cmGQ?xA>xPY9LGpcVXkEX;u{e#gx$$4YK-osouuwFKM!m1^@aSdNf} zi06(Oe5YxnvCZ!~tIw-1_C=CBIo$JXQ%TrkI~JQ>GSCR0R>Z2_?{d%jtkYdF%tW%Y zMynXc{Z|P*K))@~rXcPrCnr}SFJU(^uzwy_(mc*WPAkkHo|kF*@@1~*dw~uTDTyM5 zgmjf`fy`><1J^RJ#(L5p%*}#}GUxav3DYx~-1LWqBloiBBZ4(VSFMlCtonY6IDzyc zoEmh}7USHq)TfCjanuEj8T6ZGUwM6B@N+R07nS_qn4c*Oqx)4M<6eHw&Ewk+9{r-A zm$eoB96^v&m}XAD25@ISo?PfBA>(V@neDzP+$N41I@=m%1T)U3qH0Y=haeVDD)l~-nkPoK-+HK@>WCq#@omW|RtL&Z>; z{NqC{vO6-rO=v6#*52eg^GIDs+%fVO7sb{3#PEyHHWxQv%}Ou*=k3^1i_5nIi-&cy z;DvVc##RfONiI+1S467Zb_ZNat?nE0j8c_h*ccWl)1PoiVbT?b4fP6J$7S3O5`&YE>Cy%%8rK+2bc4 zm!Q6pdeT9{eP3?RyvRr5P|p18LJpsDh_BkG~zyN*3{r~_uC!4tAoScN@VyXCt zqC(@~%2Lc+CW&xmWffTM8qM=iGIBnYlN!?m6r6_uBB@XQC(iEi`VT^%}B zc3AeuZ!daD`hQU)^KSR}JNEN)wGa^Y2$+D~+2AEVkR|uL;Q`1)5u125yKx{yk76BTS>3-h(V!Hv)xkPw(MsL{S8g@zx1>ug_rLp=d4V9G&2TEB@%%2ot+Y2V zf)3S_UIobNaOEc`eiimPo2%MMOaSOoD)qQ=1AlOIG_AE&tZDCiS*&-r0_Eu^v=N`1 z_&^<eGGlh?0phE6T}4@{s_YT5P9Uw?Sk|FD~cr`{H;WM|!HC3vI+pon&<|SsTy;66Z!C@1M-2-I zV*gxC_|F+25eM1f{}C@_jg^J4m6!vEiXrA*i(9x9oC}1&Uj##bDPV6chquySbrclA z>)&GUh|@WZn+Gb5Tab?B0CTG?;QmS=)yRR(rE;@aY6R;P^&hvs(LiYrx1K1F-kxqT ztan-)h_K>kSz-Bmldk$rHP^}MY~poH&}oC?yzUw5qmdvG@0m4`HL>+{S>uiGSKQ8q zo>P?ahx_Z{ugW(CIKW?+Bc_z4QrZST9mwo2EKPT(%Ju7p1Ha;yR8|U9c{O>T79uw2 z^9Mzb%hO2uyG=QmK)F9$rVaWMAjy)Crvh5b+)SFLnb)s4N&qYf?{7pB-F{aG`qM2k zt$|@ViB51@6(sVCF>rXhYO2FSeZ%6Z_Q;pc$H!BK!Xh_EGI<4_0ZTDWgGM1cEOj<2 zg4VS7xs`YFui7KzOfxXKv1zG^$a^&O6a@<5p?O(2xDqmPhj%7@f5|FDMI1u>&c-1F zgDK3coS;`y+FS|L9Nz5ga1jR}C|imNUV=`htYR)=a6098 zjh~uRJy^b(!pGP(ENiQdBpMSId^Xh!zN^6c9#36;-K>qF{&YpdJ{^PT^0VNofU?Aq;rDAAeT&CU{lUn;4e7T;k*-# zV4&r=o&6soM$-8X^bt?Fv4B8ne6ld3&|DuU(CW$pApwVQM9b}V_UQY`pvPO=x~HD# z6Z+rP+Np^z-Taa~JqO=?!j0%jC*pg{jI3zN}{pwF$8`m)On>|Z?e)9&K>gs}-n3%w^TRy8xO&(`c z$`GZ$W4S6!UH-U_+TW#e2s@A&XW(imjeS(oar1Ak%|xA0xL{)8Et*L@+Do+vK6i(jrus^g)Qp8{}H#r-e^VX_A1ux(EFd z3Zx?og1_bY4mHXaa4RIR(=p1&MrBO$8UL<#-dd=!1iKt9HGxMD8&>F|J){?rf1cNo z__VPZVz-+q@6`BQ@;LJYt}>COivFnnfh0y1lbXhPMMWMKHnwCniCfvD)^k<;mUk6# zDxA$F;IzL^_J5jx{`pSLceFoWCD>L}s`V|a(K{3kMJg=lVn>6Q&F6OO zlL}4v0di=0*+Z*Mr`wUkXY<>7Fqvsv>;z@%@_1#1h9J@rC0o!{Kw<=ykW6JG4{d%Y zJ{C)fp9^A1dKg#>V0_-w^+t=T2i0ut`MT#LdV5O&hWny^Xo01mP3OH#_MK4`fip}#&lGV{CR ze5H-w&cx@pD*@M;ht9w2Uzm1bS+4g)+q5Vo(Dh-K2vPp(#G?_K=qbfsZ@XURmFRhf z`XTLQ!=HEA(P)*4#K{g5N~j|A>pVe^hyf#JT{9@x#=>BxlL(%|?QX!6reMklSi7 zcL>UjVwkXoIQc4N4{vMHcIPS#dvY%N zs9h-~e;q8h{dS3%#32N6O@8L-65>cMg10= z?!&JdGMqO{m6Lvs#nFliFu+p4Dm)S#IBZfNDx4SsvqjX56t_2aUH!5mMucX|Xl5)` zjIl_nl!IAOW|?w6++FfMB8v?pd)xpLS~i0!lQvjJqE%55rk4qLmOJzWExcI5_xi}W z0q%Lc{KsXM%lE>rh0Nw;wd-9>57hh=rAL-JfbT3LTk3N}zMYq@Oy?KI;Nae|pjE_< zxG`rVg|YC5R(JZt#h)oipqJ!XQyNG^e^lQ7m?{GCcq#h+=!c>C)xj_1=PpoPg*@Yo zp@7sIg2<7fbJdubn7E{)mvsX89y5X!EDzVmavMF!YtAWZJUB8=`f*<6mSYNGj_6Vg zy70}ft*gSoA_>JB^dxurfp?{YS}Oi0MDY#inTfDxxTb(@2~Q@9aKPG~tXx9xM{aR) z4<0rKt|XLJ9kf%ffTprx;)jZYn(n6>La3%{z>}pkW7Ro`(hC5a8P5@}o8z`S;oh4g zGw0k6 Date: Mon, 30 Sep 2024 14:36:20 -0500 Subject: [PATCH 6/7] Address feedback --- specs/required-inputs/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/specs/required-inputs/README.md b/specs/required-inputs/README.md index 1746fbae7a..acbf6791ef 100644 --- a/specs/required-inputs/README.md +++ b/specs/required-inputs/README.md @@ -26,17 +26,17 @@ Any other controls that could possibly be considered inputs are out of scope. ### API -Our FAST base components provide built-in forms support via the [same API](https://www.w3schools.com/tags/att_input_required.asp) exposed by the native HTML `input`: +Our FAST base components provide built-in forms support via the [same API](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/required) exposed by the native HTML `input`: - `required`: boolean attribute whose presence indicates that a value must be provided to submit -For radio buttons/groups, only the button elements expose the `required` attribute. If any of the radio buttons in a group are marked required, the group is treated as required. It does not matter if the button with `required` is disabled, hidden, etc. It is effectively as if the attribute is on the group of buttons, rather than a specific one. This matches the behavior of the native `input` with `type="radio"`. +For radio buttons and radio button groups, only the radio button elements expose the `required` attribute. If any of the radio buttons in a radio button group are marked required, the radio button group is treated as required. It does not matter if the radio button with `required` is disabled, hidden, etc. It is effectively as if the attribute is on the radio button group, rather than a specific one. This matches the behavior of the native `input` with `type="radio"`. ### Visuals We will update our templates so that setting `required` causes a red asterisk to be displayed after the label, as per the visual design. For radio buttons/groups, the asterisk is on the group label, not individual button labels. We have already forked some of the templates from FAST, but we will now need to fork the ones for `nimble-number-field`, `nimble-radio-group`, and `nimble-text-field`. -This will be implemented using a styled span containing the text "*". The span must be outside of the `label` element so that it is not included in the accessible name. We will wrap the two in a `div` so that we can force a horizontal layout (`flex-direction: row`). This structure will also ensure that whatever the wrapping/ellipsizing behavior of the label is, the asterisk will always appear at the far right: +This will be implemented using a new `nimble-icon-asterisk` icon ([named consistently with Font Awesome](https://fontawesome.com/icons/asterisk)) that follows the `label` element. We will wrap the two in a `div` so that we can force a horizontal layout (`flex-direction: row`). This structure will also ensure that whatever the wrapping/ellipsizing behavior of the label is, the asterisk will always appear at the far right: ```html
@@ -44,7 +44,7 @@ This will be implemented using a styled span containing the text "*". The span m ${when(x => x.required, html` - * + `)}
``` From 80175ffa7668025b080221fb20b3e935af11d6ab Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Mon, 30 Sep 2024 16:40:31 -0500 Subject: [PATCH 7/7] Trigger snapshot regen --- packages/storybook/.storybook/preview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storybook/.storybook/preview.js b/packages/storybook/.storybook/preview.js index 76bea5bd30..ebeb9eb6a3 100644 --- a/packages/storybook/.storybook/preview.js +++ b/packages/storybook/.storybook/preview.js @@ -105,5 +105,5 @@ configureActions({ depth: 1 }); -// Update the GUID on this line to trigger a turbosnap full rebuild: 1c1078d9-af89-4624-93d1-7e2639a09ea9 +// Update the GUID on this line to trigger a turbosnap full rebuild: 9236157e-5f33-411d-aede-26045b7d9d57 // See https://www.chromatic.com/docs/turbosnap/#full-rebuilds