From 811610012882486ca499cc1c3327b95cf86dede8 Mon Sep 17 00:00:00 2001 From: Maximilian Koeller Date: Wed, 20 Aug 2025 17:33:48 +0200 Subject: [PATCH] fix(forms): always require a label in `si-form-item` Not having a label is an a11y flaw. This was not enforced before to maintain compatibility with the legacy mode. Since the legacy mode was officially dropped we no longer need this. BREAKING CHANGE: The `si-form-item` now always requires a label. Not having a label is an accessibility flaw. --- .../si-form-item/si-form-item.component.html | 40 +++++----- .../si-form-item.component.spec.ts | 77 ------------------- .../si-form-item/si-form-item.component.ts | 4 +- .../wrapper/si-formly-wrapper.component.html | 2 +- .../wrapper/si-formly-wrapper.component.ts | 4 - .../si-formly/si-dynamic-form-fields.ts | 7 -- 6 files changed, 22 insertions(+), 112 deletions(-) delete mode 100644 projects/element-ng/form/si-form-item/si-form-item.component.spec.ts diff --git a/projects/element-ng/form/si-form-item/si-form-item.component.html b/projects/element-ng/form/si-form-item/si-form-item.component.html index c60de3c22..4e144656c 100644 --- a/projects/element-ng/form/si-form-item/si-form-item.component.html +++ b/projects/element-ng/form/si-form-item/si-form-item.component.html @@ -17,27 +17,25 @@ } - @if (label()) { - @let labeledBy = formItemLabelledBy; - @if (labeledBy) { - {{ label()! | translate }} - } @else { - - } + @let labeledBy = formItemLabelledBy; + @if (labeledBy) { + {{ label()! | translate }} + } @else { + } diff --git a/projects/element-ng/form/si-form-item/si-form-item.component.spec.ts b/projects/element-ng/form/si-form-item/si-form-item.component.spec.ts deleted file mode 100644 index c1931e512..000000000 --- a/projects/element-ng/form/si-form-item/si-form-item.component.spec.ts +++ /dev/null @@ -1,77 +0,0 @@ -/** - * Copyright (c) Siemens 2016 - 2025 - * SPDX-License-Identifier: MIT - */ -import { ChangeDetectionStrategy, Component, signal, viewChild } from '@angular/core'; -import { ComponentFixture, fakeAsync, TestBed, waitForAsync } from '@angular/core/testing'; - -import { SiFormItemComponent } from './si-form-item.component'; - -@Component({ - imports: [SiFormItemComponent], - template: ` - - - - `, - changeDetection: ChangeDetectionStrategy.OnPush -}) -export class TestHostComponent { - readonly formItem = viewChild.required(SiFormItemComponent); - readonly label = signal(null); -} - -describe('SiFormItemComponent', () => { - let component: TestHostComponent; - let fixture: ComponentFixture; - - const getLabel = (): HTMLElement | null => { - return fixture.nativeElement.querySelector('label'); - }; - - beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [TestHostComponent] - }).compileComponents(); - })); - - beforeEach(() => { - fixture = TestBed.createComponent(TestHostComponent); - component = fixture.componentInstance; - fixture.detectChanges(); - }); - - it('should create', fakeAsync(() => { - expect(component).toBeDefined(); - expect(getLabel()).toBeDefined(); - })); - - it('should display the bound label value', () => { - component.label.set('Testlabel'); - fixture.detectChanges(); - expect(getLabel()?.textContent).toContain(component.formItem().label()); - }); - - it('should not display label colon with no label set', () => { - fixture.detectChanges(); - expect(getLabel()).toBeFalsy(); - }); - - it('should not display label colon with empty label set', () => { - component.label.set(''); - fixture.detectChanges(); - expect(getLabel()).toBeFalsy(); - }); - - it('should not display label colon with undefined label set', () => { - component.label.set(undefined); - fixture.detectChanges(); - expect(getLabel()).toBeFalsy(); - }); - - it('should not display label colon with null label set', () => { - component.label.set(null); - fixture.detectChanges(); - expect(getLabel()).toBeFalsy(); - }); -}); diff --git a/projects/element-ng/form/si-form-item/si-form-item.component.ts b/projects/element-ng/form/si-form-item/si-form-item.component.ts index d42b974ad..692da9287 100644 --- a/projects/element-ng/form/si-form-item/si-form-item.component.ts +++ b/projects/element-ng/form/si-form-item/si-form-item.component.ts @@ -28,7 +28,7 @@ import { ValidationErrors, ValidatorFn } from '@angular/forms'; -import { SiTranslatePipe } from '@siemens/element-translate-ng/translate'; +import { SiTranslatePipe, TranslatableString } from '@siemens/element-translate-ng/translate'; import { SiFormFieldsetComponent } from '../form-fieldset/si-form-fieldset.component'; import { SiFormContainerComponent } from '../si-form-container/si-form-container.component'; @@ -65,7 +65,7 @@ export class SiFormItemComponent * The label to be displayed in the form item. * It will be translated if a translation key is available. */ - readonly label = input(); + readonly label = input.required(); /** * A custom width value to be applied to the label. diff --git a/projects/element-ng/formly/wrapper/si-formly-wrapper.component.html b/projects/element-ng/formly/wrapper/si-formly-wrapper.component.html index ebbe2460b..c95f0adcd 100644 --- a/projects/element-ng/formly/wrapper/si-formly-wrapper.component.html +++ b/projects/element-ng/formly/wrapper/si-formly-wrapper.component.html @@ -1,6 +1,6 @@ diff --git a/projects/element-ng/formly/wrapper/si-formly-wrapper.component.ts b/projects/element-ng/formly/wrapper/si-formly-wrapper.component.ts index d66ecaa2a..b1fc37c05 100644 --- a/projects/element-ng/formly/wrapper/si-formly-wrapper.component.ts +++ b/projects/element-ng/formly/wrapper/si-formly-wrapper.component.ts @@ -14,10 +14,6 @@ import { SiFormlyFormFieldProviderDirective } from './si-formly-form-field-provi templateUrl: './si-formly-wrapper.component.html' }) export class SiFormlyWrapperComponent extends FieldWrapper { - protected get label(): string | undefined { - return this.props.label && this.props.hideLabel !== true ? this.props.label : undefined; - } - protected get labelWidth(): number | undefined { return this.props.labelWidth; } diff --git a/src/app/examples/si-formly/si-dynamic-form-fields.ts b/src/app/examples/si-formly/si-dynamic-form-fields.ts index d3578f84d..541d8c6bd 100644 --- a/src/app/examples/si-formly/si-dynamic-form-fields.ts +++ b/src/app/examples/si-formly/si-dynamic-form-fields.ts @@ -446,7 +446,6 @@ export class SampleComponent { type: 'button', className: 'd-block mb-4', props: { - hideLabel: true, // Hide wrapper label label: 'Button function handler', clickListener: (a: string, b: string) => { alert( @@ -461,7 +460,6 @@ export class SampleComponent { type: 'button', className: 'd-block mb-4', props: { - hideLabel: true, // Hide wrapper label label: 'Button expression handler', clickListener: 'formState.btnClicked', clickArgs: ['1st', 23] @@ -472,7 +470,6 @@ export class SampleComponent { type: 'button', className: 'd-block mb-4', props: { - hideLabel: true, // Hide wrapper label label: 'Primary', clickListener: 'formState.btnClicked', clickArgs: ['primary', 23], @@ -484,7 +481,6 @@ export class SampleComponent { type: 'button', className: 'd-block mb-4', props: { - hideLabel: true, // Hide wrapper label label: 'Secondary (Default)', clickListener: 'formState.btnClicked', clickArgs: ['secondary', 23], @@ -496,7 +492,6 @@ export class SampleComponent { type: 'button', className: 'd-block mb-4', props: { - hideLabel: true, // Hide wrapper label label: 'Tertiary', clickListener: 'formState.btnClicked', clickArgs: ['tertiary', 23], @@ -508,7 +503,6 @@ export class SampleComponent { type: 'button', className: 'd-block mb-4', props: { - hideLabel: true, // Hide wrapper label label: 'Warning', clickListener: 'formState.btnClicked', clickArgs: ['warning', 23], @@ -520,7 +514,6 @@ export class SampleComponent { type: 'button', className: 'd-block mb-4', props: { - hideLabel: true, // Hide wrapper label label: 'Danger', clickListener: 'formState.btnClicked', clickArgs: ['danger', 23],