-
Notifications
You must be signed in to change notification settings - Fork 58
FORMS-18634: Desktop, RWD Tablet, RWD Mobile - Form field with error not identified @sunnym @vavarshn #1585
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.
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
base: dev
Are you sure you want to change the base?
Conversation
…not identified @sunnym @vavarshn
@@ -1,3 +1,3 @@ | |||
<template data-sly-template.errorMessage="${@ componentId, bemBlock}"> | |||
<div class="${bemBlock}__errormessage" id="${componentId}__errormessage" aria-live="assertive"></div> | |||
<div class="${bemBlock}__errormessage" id="${componentId}__errormessage" aria-live="assertive" tabindex=0 role="complementary"></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tabindex needs to be set dynamically, depending on error at runtime on field, if there is no error then it should be -1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will revert this file to original and update the logic in FormFieldBase.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code updated.
@@ -176,6 +177,7 @@ class FormFieldBase extends FormField { | |||
let errorElement = typeof this.getErrorDiv === 'function' ? this.getErrorDiv() : null; | |||
if (errorElement) { | |||
errorElement.setAttribute('id', `${this.getId()}__errormessage`); | |||
errorElement.setAttribute('aria-describedby', `${this.getId()}__label`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is already present in the input field. We can not use the aria-describedby attribute on the error container.
@@ -167,6 +167,7 @@ class FormFieldBase extends FormField { | |||
#syncLabel() { | |||
let labelElement = typeof this.getLabel === 'function' ? this.getLabel() : null; | |||
if (labelElement) { | |||
labelElement.setAttribute('id', `${this.getId()}__label`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this id
@@ -550,7 +554,9 @@ class FormFieldBase extends FormField { | |||
if (!state.validationMessage) { | |||
this.errorDiv.innerHTML = LanguageUtils.getTranslatedString(this.formContainer.getModel().lang, "defaultError"); | |||
} | |||
} | |||
this.errorDiv.setAttribute('tabindex', 0); | |||
this.errorDiv.setAttribute('role', 'complementary'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is usually incorrect to use role="complementary" on an error container.
Use role="alert" or aria-live="assertive" for error messages, as this is the best practice(Optional)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check comments
Description
Related Issue
https://jira.corp.adobe.com/browse/FORMS-18634
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: