Skip to content

FORMS-18671: Visible label missing @sunnym @vavarshn #1603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

FORMS-18671: Visible label missing @sunnym @vavarshn #1603

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c4755fc
FORMS-18671: Visible label missing @sunnym @vavarshn
Jun 4, 2025
8856026
Merge branch 'dev' into ens37130/FORMS-18671
Jun 16, 2025
e53cbe2
FORMS-18671: Round 1 feedback
Jun 18, 2025
9fdab17
FORMS-18671: Keeping original class names in fileinput.html
Aug 16, 2025
0f1f005
FORMS-18671: Fixed case where Checbox wasn't getting an Accessible label
Aug 28, 2025
55f3648
FORMS-18671: Added missing screen reader text for File Upload and TnC…
Sep 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .../content/jcr_root/apps/core/fd/components/form/checkboxgroup/v1/checkboxgroup/widget.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
~ limitations under the License.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/-->
<template data-sly-template.widget="${ @ checkboxgroup }">
<div class="cmp-adaptiveform-checkboxgroup__widget ${checkboxgroup.orientation}" id="${widgetId}">
<div class="cmp-adaptiveform-checkboxgroup__widget ${checkboxgroup.orientation}" id="${widgetId}" role="group">
Copy link
Contributor

@amit-mnnit amit-mnnit Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add the role="group" attribute, you must connect the label to the group using the aria-labelledby attribute.

Copy link
Contributor Author

@stefan-dragomir stefan-dragomir Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added logic in FormFieldBase.js for this. Should help both CheckboxGroup and RadioGroup.

<div data-sly-list="${checkboxgroup.enums}" data-sly-unwrap>
<div class="cmp-adaptiveform-checkboxgroup-item ${checkboxgroup.name}">
<label class="cmp-adaptiveform-checkboxgroup__option-label"
Expand Down
30 changes: 16 additions & 14 deletions .../main/content/jcr_root/apps/core/fd/components/form/fileinput/v1/fileinput/fileinput.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,22 @@
<div data-sly-call="${questionMark.questionMark @componentId=file.id, longDescription=file.description, bemBlock='cmp-adaptiveform-fileinput'}" data-sly-unwrap></div>
</div>
</br>
<label data-sly-test="${file.buttonText}" for="${file.id}_widget" tabindex="0" role="button" class="cmp-adaptiveform-fileinput__widgetlabel">${file.buttonText}</label>
<input class="cmp-adaptiveform-fileinput__widget"
data-sly-attribute.multiple="${file.isMultiple ? 'multiple' : ''}"
style="${file.buttonText ? 'display:none' : '' @ context='styleString'}"
title="${file.tooltipVisible ? '' : file.tooltipText}"
name="${file.name}"
disabled="${!file.enabled}"
required="${file.required}"
accept="${file.accept}"
id="${file.id}_widget"
type="file"
aria-describedby="${file.id}__errormessage ${file.id}__longdescription ${file.id}__shortdescription"
data-cmp-data-layer="${file.data.json}"
/>
<div class="cmp-adaptiveform-fileinput__widget-container">
Copy link
Contributor

@amit-mnnit amit-mnnit Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a V3 component. You should update the HTML in the latest version, but V3 component already includes a container

.

Copy link
Contributor Author

@stefan-dragomir stefan-dragomir Jun 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in V3. Also changed the name of the container to align with the #syncAriaLabel() update. V1 remained changed as well.

<label data-sly-test="${file.buttonText}" for="${file.id}_widget" tabindex="0" role="button" class="cmp-adaptiveform-fileinput__widgetlabel">${file.buttonText}</label>
<input class="cmp-adaptiveform-fileinput__widget"
data-sly-attribute.multiple="${file.isMultiple ? 'multiple' : ''}"
style="${file.buttonText ? 'display:none' : '' @ context='styleString'}"
title="${file.tooltipVisible ? '' : file.tooltipText}"
name="${file.name}"
disabled="${!file.enabled}"
required="${file.required}"
accept="${file.accept}"
id="${file.id}_widget"
type="file"
aria-describedby="${file.id}__errormessage ${file.id}__longdescription ${file.id}__shortdescription"
data-cmp-data-layer="${file.data.json}"
/>
</div>
<ul class="cmp-adaptiveform-fileinput__filelist"></ul>
<div data-sly-call="${shortDescription.shortDescription @componentId=file.id, shortDescriptionVisible=file.tooltipVisible, shortDescription=file.tooltip, bemBlock='cmp-adaptiveform-fileinput'}" data-sly-unwrap>
</div>
Expand Down
36 changes: 19 additions & 17 deletions .../core/fd/components/form/termsandconditions/v1/termsandconditions/termsandconditions.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,29 @@
<div data-sly-call="${label.label @componentId=tnc.id, labelValue=tnc.label.value, labelVisible=tnc.label.visible, labelRichText=tnc.label.richText, bemBlock='cmp-adaptiveform-termsandcondition'}" data-sly-unwrap></div>
<div data-sly-call="${questionMark.questionMark @componentId=tnc.id, longDescription=tnc.description, bemBlock='cmp-adaptiveform-termsandcondition'}" data-sly-unwrap></div>
</div>
<div class="cmp-adaptiveform-termsandcondition__content-container ${!wcmmode.edit && tnc.showAsPopup ? 'cmp-adaptiveform-termsandcondition__content-container--modal' : ''}"
data-cmp-visible="${tnc.showAsPopup ? 'false' : 'true'}">
<div class="cmp-adaptiveform-termsandcondition__body">
<div class="cmp-adaptiveform-termsandcondition__header" data-sly-test="${tnc.showAsPopup && !wcmmode.edit}">
<button type="button" class="cmp-adaptiveform-termsandcondition__close-button" >
X
</button>
<h3>
${"Please review the terms and conditions" @ i18n, context='html'}
</h3>
</div>
<div class="cmp-adaptiveform-termsandcondition__content">
<div data-sly-test="${!tnc.showLink}" class="cmp-adaptiveform-termsandcondition__text">
<div data-sly-resource="${'text'}"></div>
<div class="cmp-adaptiveform-termsandcondition__text-intersect"></div>
<div class="cmp-adaptiveform-termsandcondition__widget-container">
Copy link
Contributor

@amit-mnnit amit-mnnit Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it necessary to add a new container element? Is it not possible with the existing container element cmp-adaptiveform-termsandcondition__content-containe?

Copy link
Contributor Author

@stefan-dragomir stefan-dragomir Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed a container to encapsulate both the T&C text description and checkbox (for using with label and role). On Win+NVDA keyboard navigation will skip directly to the Checkbox, ignoring the fact it's part of a component.

<div class="cmp-adaptiveform-termsandcondition__content-container ${!wcmmode.edit && tnc.showAsPopup ? 'cmp-adaptiveform-termsandcondition__content-container--modal' : ''}"
data-cmp-visible="${tnc.showAsPopup ? 'false' : 'true'}">
<div class="cmp-adaptiveform-termsandcondition__body">
<div class="cmp-adaptiveform-termsandcondition__header" data-sly-test="${tnc.showAsPopup && !wcmmode.edit}">
<button type="button" class="cmp-adaptiveform-termsandcondition__close-button" >
X
</button>
<h3>
${"Please review the terms and conditions" @ i18n, context='html'}
</h3>
</div>
<div class="cmp-adaptiveform-termsandcondition__content">
<div data-sly-test="${!tnc.showLink}" class="cmp-adaptiveform-termsandcondition__text">
<div data-sly-resource="${'text'}"></div>
<div class="cmp-adaptiveform-termsandcondition__text-intersect"></div>
</div>
<div class="cmp-adaptiveform-termsandcondition__link" data-sly-test="${tnc.showLink}" data-sly-resource="${'link'}"></div>
</div>
<div class="cmp-adaptiveform-termsandcondition__link" data-sly-test="${tnc.showLink}" data-sly-resource="${'link'}"></div>
</div>
</div>
<div class="cmp-adaptiveform-termsandcondition__approvalcheckbox" data-sly-test="${tnc.showApprovalOption}" data-sly-resource="${'approvalcheckbox'}"></div>
</div>
<div class="cmp-adaptiveform-termsandcondition__approvalcheckbox" data-sly-test="${tnc.showApprovalOption}" data-sly-resource="${'approvalcheckbox'}"></div>
<div data-sly-call="${shortDescription.shortDescription @componentId=tnc.id, shortDescriptionVisible=tnc.tooltipVisible, shortDescription=tnc.tooltip, bemBlock='cmp-adaptiveform-termsandcondition'}" data-sly-unwrap></div>
<div data-sly-call="${longDescription.longDescription @componentId=tnc.id, longDescription=tnc.description, bemBlock='cmp-adaptiveform-termsandcondition'}" data-sly-unwrap></div>
</div>
16 changes: 13 additions & 3 deletions ui.frontend/src/view/FormFieldBase.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,15 +229,25 @@ class FormFieldBase extends FormField {
}

#syncAriaLabel() {
const bemClass = Array.from(this.element.classList).filter(bemClass => !bemClass.includes('--'))[0];
const regionContainer = this.element.querySelector(`.${bemClass}__widget-container`);
let widgetElement = typeof this.getWidget === 'function' ? this.getWidget() : null;
let widgetElements = typeof this.getWidgets === 'function' ? this.getWidgets() : null;
widgetElement = widgetElements || widgetElement;
const model = this.getModel?.();

if (widgetElement && model?.screenReaderText) {
if (model?.screenReaderText){
// Use DOMPurify to sanitize and strip HTML tags
const screenReaderText = window.DOMPurify ? window.DOMPurify.sanitize(model.screenReaderText, { ALLOWED_TAGS: [] }) : model.screenReaderText;
widgetElement.setAttribute('aria-label', screenReaderText);

// Some elements have the Widget hidden by default and other are Panels
// So this container mimics having a single, showing widget to attach the Accessibility label to.
if(regionContainer) {
regionContainer.setAttribute('role', 'region');
Copy link
Contributor

@amit-mnnit amit-mnnit Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

role="region" attribute is not required to meet WCAG. It is the best practice.

Copy link
Contributor Author

@stefan-dragomir stefan-dragomir Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NVDA on Windows will ignore the div element despite having an aria-label. role="region" was the best compromise I could find for this situation.

Custom Label without Region
Custom Label with Region

regionContainer.setAttribute('aria-label', screenReaderText);
} else if (widgetElement) {
widgetElement.setAttribute('aria-label', screenReaderText);
}
}
}

Expand All @@ -249,7 +259,7 @@ class FormFieldBase extends FormField {
this.#syncLabel()
this.#syncWidget()
this.#syncShortDesc()
this. #syncLongDesc()
this.#syncLongDesc()
this.#syncAriaDescribedBy()
this.#syncError()
this.#syncAriaLabel()
Expand Down
Loading