fix(components/forms): legends for radio group and field group are properly detectable by screen readers

libs/components/forms/src/lib/modules/field-group/field-group.component.html

@@ -2,23 +2,24 @@
   class="sky-field-group"
   [attr.aria-describedby]="hintText ? hintTextEl.id : undefined"
 >
-  <span class="sky-field-group-legend-wrapper">
-    <legend
-      [ngClass]="{
-        'sky-screen-reader-only': headingHidden,
-        'sky-margin-inline-xs': helpPopoverContent
-      }"
-    >
-      @if (headingLevel === 3) {
-        <h3 [class]="headingClass">
-          {{ headingText }}
-        </h3>
-      } @else if (headingLevel === 4) {
-        <h4 [class]="headingClass">
-          {{ headingText }}
-        </h4>
+  <legend
+    class="sky-control-label"
+    [ngClass]="{
+      'sky-screen-reader-only': headingHidden,
+      'sky-margin-stacked-xs': !!hintText,
+      'sky-margin-stacked-sm': !hintText
+    }"
+  >
+    <span class="sky-margin-inline-xs">
+      @switch (headingLevel) {
+        @case (4) {
+          <h4 [class]="headingClass">{{ headingText }}</h4>
+        }
+        @default {
+          <h3 [class]="headingClass">{{ headingText }}</h3>
+        }
       }
-    </legend>
+    </span>
     @if (helpPopoverContent || helpKey) {
       <sky-help-inline
         class="sky-field-group-help-inline"
@@ -28,10 +29,12 @@ <h4 [class]="headingClass">
         [popoverContent]="helpPopoverContent"
       />
     }
-  </span>
+  </legend>
   <div #hintTextEl="skyId" skyId>
     @if (hintText) {
-      <div class="sky-field-group-hint-text sky-font-deemphasized">
+      <div
+        class="sky-field-group-hint-text sky-margin-stacked-lg sky-font-deemphasized"
+      >
         {{ hintText }}
       </div>
     }

libs/components/forms/src/lib/modules/field-group/field-group.component.scss

@@ -2,23 +2,11 @@
   display: block;
 }
 
-.sky-field-group-hint-text {
-  margin-top: var(--sky-margin-stacked-xs);
-}
-
-.sky-field-group-content {
-  margin-top: var(--sky-margin-stacked-sm);
-}
-
-h3,
-h4 {
-  margin: 0;
-}
-
-.sky-field-group-legend-wrapper {
-  display: inline-flex;
-}
+legend {
+  display: flex;
 
-sky-help-inline {
-  display: inline-flex;
+  h3,
+  h4 {
+    margin: 0;
+  }
 }

libs/components/forms/src/lib/modules/radio/radio-group.component.html

@@ -12,35 +12,43 @@
     headingText && ngControl?.errors ? errorId : undefined
   "
 >
-  <span class="sky-radio-group-label-wrapper">
-    @if (headingText) {
-      <legend
-        class="sky-control-label"
+  @if (headingText) {
+    <legend
+      class="sky-control-label"
+      [ngClass]="{
+        'sky-screen-reader-only': headingHidden,
+        'sky-margin-stacked-xs': !!hintText,
+        'sky-margin-stacked-sm': !hintText
+      }"
+    >
+      <span
+        class="sky-margin-inline-xs"
         [ngClass]="{
-          'sky-screen-reader-only': headingHidden,
-          'sky-margin-stacked-sm': !hintText,
-          'sky-control-label-required': isRequired,
-          'sky-margin-inline-xs': helpPopoverContent
+          'sky-control-label-required': required
         }"
       >
-        @if (headingLevel === 3) {
-          <h3 [class]="headingClass">
-            {{ headingText }}
-          </h3>
-        } @else if (headingLevel === 4) {
-          <h4 [class]="headingClass">
-            {{ headingText }}
-          </h4>
-        } @else if (headingLevel === 5) {
-          <h5 [class]="headingClass">
-            {{ headingText }}
-          </h5>
-        } @else {
-          <span [class]="'sky-radio-group-heading-text ' + headingClass">
-            {{ headingText }}
-          </span>
+        @switch (headingLevel) {
+          @case (3) {
+            <h3 [class]="headingClass">{{ headingText }}</h3>
+          }
+          @case (4) {
+            <h4 [class]="headingClass">{{ headingText }}</h4>
+          }
+          @case (5) {
+            <h5 [class]="headingClass">{{ headingText }}</h5>
+          }
+          @default {
+            <span [class]="'sky-radio-group-heading-text ' + headingClass">{{
+              headingText
+            }}</span>
+          }
         }
-      </legend>
+      </span>
+      @if (required) {
+        <span class="sky-screen-reader-only">{{
+          'skyux_form_group_required' | skyLibResources
+        }}</span>
+      }
       @if (helpPopoverContent || helpKey) {
         <sky-help-inline
           [helpKey]="helpKey"
@@ -49,12 +57,13 @@ <h5 [class]="headingClass">
           [popoverContent]="helpPopoverContent"
         />
       }
-    }
-  </span>
-
+    </legend>
+  }
   <div #hintTextEl="skyId" skyId>
     @if (hintText) {
-      <div class="sky-font-deemphasized sky-radio-group-hint-text">
+      <div
+        class="sky-font-deemphasized sky-radio-group-hint-text sky-margin-stacked-lg"
+      >
         {{ hintText }}
       </div>
     }

libs/components/forms/src/lib/modules/radio/radio-group.component.scss

@@ -5,14 +5,6 @@
   }
 }
 
-.sky-radio-group-hint-text {
-  margin-bottom: var(--sky-margin-stacked-lg);
-}
-
-.sky-radio-group-label-wrapper {
-  display: flex;
-}
-
 legend {
   h3,
   h4,

libs/components/forms/src/lib/modules/radio/radio-group.component.spec.ts

@@ -189,7 +189,9 @@ describe('Radio group component (reactive)', function () {
     tick();
 
     const radioGroupDiv = getRadioGroup(fixture);
-    const radioGroupLabel = getRadioGroupLabel(fixture);
+    const radioGroupLabel = fixture.nativeElement.querySelector(
+      'span.sky-margin-inline-xs',
+    );
     expect(radioGroupDiv?.getAttribute('required')).toBeNull();
     expect(radioGroupDiv?.getAttribute('aria-required')).toBeNull();
     expect(radioGroupLabel).not.toHaveCssClass('sky-control-label-required');
@@ -203,7 +205,9 @@ describe('Radio group component (reactive)', function () {
     tick();
 
     const radioGroupDiv = getRadioGroup(fixture);
-    const radioGroupLabel = getRadioGroupLabel(fixture);
+    const radioGroupLabel = fixture.nativeElement.querySelector(
+      'span.sky-margin-inline-xs',
+    );
     expect(radioGroupDiv?.getAttribute('required')).not.toBeNull();
     expect(radioGroupDiv?.getAttribute('aria-required')).toBe('true');
     expect(radioGroupLabel).toHaveCssClass('sky-control-label-required');

libs/components/forms/testing/src/field-group/field-group-harness.ts

@@ -21,8 +21,9 @@ export class SkyFieldGroupHarness extends SkyComponentHarness {
   public static hostSelector = 'sky-field-group';
 
   #getLegend = this.locatorFor('legend');
-  #getH3 = this.locatorForOptional('legend h3');
-  #getH4 = this.locatorForOptional('legend h4');
+  #getLegendH3 = this.locatorForOptional('legend h3');
+  #getLegendH4 = this.locatorForOptional('legend h4');
+  #getLegendHeading = this.locatorFor('legend h3,h4');
   #getHintText = this.locatorForOptional('.sky-field-group-hint-text');
 
   /**
@@ -40,7 +41,7 @@ export class SkyFieldGroupHarness extends SkyComponentHarness {
    * the text will still be returned.
    */
   public async getHeadingText(): Promise<string | undefined> {
-    return (await this.#getLegend()).text();
+    return (await this.#getLegendHeading()).text();
   }
 
   /**
@@ -72,7 +73,7 @@ export class SkyFieldGroupHarness extends SkyComponentHarness {
    * The semantic heading level used for the field group.
    */
   public async getHeadingLevel(): Promise<SkyFieldGroupHeadingLevel> {
-    const h3 = await this.#getH3();
+    const h3 = await this.#getLegendH3();
 
     return h3 ? 3 : 4;
   }
@@ -81,7 +82,7 @@ export class SkyFieldGroupHarness extends SkyComponentHarness {
    * The heading style used for the field group.
    */
   public async getHeadingStyle(): Promise<SkyFieldGroupHeadingStyle> {
-    const heading = (await this.#getH3()) || (await this.#getH4());
+    const heading = (await this.#getLegendH3()) || (await this.#getLegendH4());
 
     return (await heading?.hasClass('sky-font-heading-3')) ? 3 : 4;
   }
@@ -112,7 +113,7 @@ export class SkyFieldGroupHarness extends SkyComponentHarness {
   async #getHelpInline(): Promise<SkyHelpInlineHarness> {
     const harness = await this.locatorForOptional(
       SkyHelpInlineHarness.with({
-        ancestor: '.sky-field-group > .sky-field-group-legend-wrapper',
+        ancestor: '.sky-field-group > .sky-control-label',
       }),
     )();
 

libs/components/forms/testing/src/radio/fixtures/radio-harness-test.component.html

@@ -2,11 +2,11 @@
   <sky-radio-group
     data-sky-id="radio-group"
     formControlName="paymentMethod"
-    headingText="Payment method"
     [attr.class]="class"
     [headingHidden]="hideGroupHeading"
     [headingLevel]="headingLevel"
     [headingStyle]="headingStyle"
+    [headingText]="headingText"
     [helpKey]="helpKey"
     [helpPopoverContent]="helpPopoverContent"
     [helpPopoverTitle]="helpPopoverTitle"

libs/components/forms/testing/src/radio/fixtures/radio-harness-test.component.ts

@@ -32,6 +32,7 @@ export class RadioHarnessTestComponent {
   public cashHintText: string | undefined;
   public headingLevel: SkyRadioGroupHeadingLevel | undefined = 3;
   public headingStyle: SkyRadioGroupHeadingStyle = 3;
+  public headingText: string | undefined = 'Payment method';
   public helpKey: string | undefined;
   public helpPopoverContent: string | undefined;
   public helpPopoverTitle: string | undefined;

libs/components/forms/testing/src/radio/radio-group-harness.spec.ts

@@ -34,12 +34,17 @@ async function setupTest(options: { dataSkyId?: string } = {}): Promise<{
 }
 
 describe('Radio group harness', () => {
-  it('should get the heading text', async () => {
-    const { radioGroupHarness } = await setupTest();
+  it('should get the heading text when it is set', async () => {
+    const { radioGroupHarness, fixture } = await setupTest();
 
     await expectAsync(radioGroupHarness.getHeadingText()).toBeResolvedTo(
       'Payment method',
     );
+
+    fixture.componentInstance.headingText = undefined;
+    fixture.detectChanges();
+
+    await expectAsync(radioGroupHarness.getHeadingText()).toBeResolvedTo('');
   });
 
   it('should get the heading text when heading text is hidden', async () => {
@@ -72,6 +77,13 @@ describe('Radio group harness', () => {
     await expectAsync(radioGroupHarness.getHeadingHidden()).toBeResolvedTo(
       true,
     );
+
+    fixture.componentInstance.headingText = undefined;
+    fixture.detectChanges();
+
+    await expectAsync(radioGroupHarness.getHeadingHidden()).toBeResolvedTo(
+      true,
+    );
   });
 
   it('should return the heading level', async () => {