Skip to content
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

a11y: handle ARIA error state in form field components #17306

Open
hennzen opened this issue Jun 21, 2024 · 4 comments
Open

a11y: handle ARIA error state in form field components #17306

hennzen opened this issue Jun 21, 2024 · 4 comments
Labels
area/a11y Accessibility area/components bug/2-confirmed We have reproduce the problem and confirmed that this is a bug. kind/bug 🐞 Qv2 🔝 Quasar v2 issues

Comments

@hennzen
Copy link

hennzen commented Jun 21, 2024

What happened?

Our app needs to comply with WCAG guideline level AA. When auditing our app, our accessibility agency reported a severe a11y issue, where screenreaders do not announce an input field as being invalid when focusing.

Take this screenshot from the reproduction URL below with the NVDA speech viewer window open. Tabbing through the elements [random button, input, reset button, input]:

  • on first focus (input field is valid, initial state) NVDA reads "Required Field edit blank"
  • tabbing one step further to reset button and then Shift-Tab'ing one step back to input
  • on second focus (input field now invalid, required and empty) NVDA still reads "Required Field edit blank", no hint it is in invalid state

focusing an invalid qinput

What did you expect to happen?

When focusing on an already invalid field, its invalidity should be announced immediately, so that a screenreader includes the keyword "invalid", ideally including any validation messages, e.g. "Address is a required".

According to MDN aria-invalid="true" on the native <input> should only be added upon validation. An accompanying aria-errormessage="some_id", pointing to the message container (at the bottom), e.g. q-field__messages should further help to identify the error.

I have not yet come to a conclusion, whether aria-describedBy="some_id" or aria-errormessage="some_id" would be the better choice here.

Reproduction URL

https://codepen.io/hennzen/pen/LYodzgg

How to reproduce?

  1. Install NVDA or any other screenreader of your choice
  2. Open NVDA's speech viewer (or listen to audio)
  3. Go to the reproduction URL
  4. Tab through the elements (without typing anything in the input field) and watch screenreader's output when focusing the input field for the first time (valid) and second time (invalid)
    ...alternatively use the mouse to focus, blur and then focus again on the input field

Flavour

Vite Plugin (@quasar/vite-plugin)

Areas

Components (quasar), Composables (quasar), Accessibility [a11y] (quasar)

Platforms/Browsers

Firefox, Chrome, Safari, Microsoft Edge

Quasar info output

No response

Relevant log output

No response

Additional context

Created a patch (v2.16.4) to solve this in a works-for-me-quality and will attach it in the comments.

@hennzen hennzen added kind/bug 🐞 Qv2 🔝 Quasar v2 issues labels Jun 21, 2024
@github-actions github-actions bot added area/a11y Accessibility area/components area/composables bug/1-repro-available A reproduction is available and needs to be confirmed. flavour/vite-plugin Bugs related to Vite usage with Quasar labels Jun 21, 2024
@hennzen
Copy link
Author

hennzen commented Jun 21, 2024

Used npx patch-package quasar to create patches/quasar+2.16.4.patch, patching QInput, QSelect and use-field.js

Unfortunately I did not find a cleaner solution to access hasError within the component's getControl() and ended up changing its signature to pass hasError as second parameter.

I think there is a lot missing, but maybe this helps someone as a starting point.

diff --git a/node_modules/quasar/src/components/input/QInput.js b/node_modules/quasar/src/components/input/QInput.js
index 8e87a62..bbefc8e 100644
--- a/node_modules/quasar/src/components/input/QInput.js
+++ b/node_modules/quasar/src/components/input/QInput.js
@@ -417,7 +417,7 @@ export default createComponent({
         || fieldValueIsFilled(props.displayValue)
       ),
 
-      getControl: () => {
+      getControl: (unused, hasError) => {
         return h(isTextarea.value === true ? 'textarea' : 'input', {
           ref: inputRef,
           class: [
@@ -431,7 +431,9 @@ export default createComponent({
             props.type !== 'file'
               ? { value: getCurValue() }
               : formDomProps.value
-          )
+          ),
+          'aria-invalid': hasError || void 0,
+          'aria-errormessage': hasError ? state.targetUid.value + '_messages' : void 0
         })
       },
 
diff --git a/node_modules/quasar/src/components/select/QSelect.js b/node_modules/quasar/src/components/select/QSelect.js
index b7fab21..d7dece0 100644
--- a/node_modules/quasar/src/components/select/QSelect.js
+++ b/node_modules/quasar/src/components/select/QSelect.js
@@ -1001,7 +1001,7 @@ export default createComponent({
       return hMergeSlot(slots[ 'after-options' ], options)
     }
 
-    function getInput (fromDialog, isTarget) {
+    function getInput (fromDialog, isTarget, hasError) {
       const attrs = isTarget === true ? { ...comboboxAttrs.value, ...state.splitAttrs.attributes.value } : void 0
 
       const data = {
@@ -1019,7 +1019,9 @@ export default createComponent({
         'data-autofocus': fromDialog === true || props.autofocus === true || void 0,
         disabled: props.disable === true,
         readonly: props.readonly === true,
-        ...inputControlEvents.value
+        ...inputControlEvents.value,
+        'aria-invalid': hasError || void 0,
+        'aria-errormessage': hasError ? state.targetUid.value + '_messages' : void 0
       }
 
       if (fromDialog !== true && hasDialog === true) {
@@ -1521,17 +1523,16 @@ export default createComponent({
         }
       },
 
-      getControl: fromDialog => {
+      getControl: (fromDialog, hasError) => {
         const child = getSelection()
         const isTarget = fromDialog === true || dialog.value !== true || hasDialog !== true
 
         if (props.useInput === true) {
-          child.push(getInput(fromDialog, isTarget))
+          child.push(getInput(fromDialog, isTarget, hasError))
         }
         // there can be only one (when dialog is opened the control in dialog should be target)
         else if (state.editable.value === true) {
           const attrs = isTarget === true ? comboboxAttrs.value : void 0
-
           child.push(
             h('input', {
               ref: isTarget === true ? targetRef : void 0,
@@ -1562,7 +1563,6 @@ export default createComponent({
 
         if (nameProp.value !== void 0 && props.disable !== true && innerOptionsValue.value.length !== 0) {
           const opts = innerOptionsValue.value.map(value => h('option', { value, selected: true }))
-
           child.push(
             h('select', {
               class: 'hidden',
diff --git a/node_modules/quasar/src/composables/private.use-field/use-field.js b/node_modules/quasar/src/composables/private.use-field/use-field.js
index 4fde652..79ca189 100644
--- a/node_modules/quasar/src/composables/private.use-field/use-field.js
+++ b/node_modules/quasar/src/composables/private.use-field/use-field.js
@@ -442,7 +442,7 @@ export default function (state) {
     }
 
     if (state.getControl !== void 0) {
-      node.push(state.getControl())
+      node.push(state.getControl(void 0, hasError.value))
     }
     // internal usage only:
     else if (slots.rawControl !== void 0) {
@@ -507,7 +507,8 @@ export default function (state) {
 
     const main = h('div', {
       key,
-      class: 'q-field__messages col'
+      class: 'q-field__messages col',
+      id: state.targetUid.value + '_messages'
     }, msg)
 
     return h('div', {

After this, NVDA speech viewer announces an invalid field like so

focusing an invalid qinput_patched

My current NVDA version 2024.1 does not yet read out the linked aria-errormessage (issue here nvaccess/nvda#8318) but 2024.3 will, according to the pull request.

@yusufkandemir yusufkandemir added bug/2-confirmed We have reproduce the problem and confirmed that this is a bug. and removed flavour/vite-plugin Bugs related to Vite usage with Quasar bug/1-repro-available A reproduction is available and needs to be confirmed. area/composables labels Jun 21, 2024
@yusufkandemir
Copy link
Member

Duplicate of #16865, but this one is far more detailed.

See Exposing Field Errors for different methods and their behavior.

Due to the recent changes you've mentioned, it's getting better. See https://cerovac.com/a11y/2024/06/support-for-aria-errormessage-is-getting-better-but-still-not-there-yet/

We might want to (at least) initially implement this in a way to allow the developers to use the way they want by passing attributes and using slots. That way, we allow making the inputs accessible, while allowing the developer to use the most suitable way depending on their userbase.

@yusufkandemir yusufkandemir changed the title QInput is missing aria-invalid and (aria-errormessage OR aria-describedBy) attributes (relevant for screenreaders) a11y: handle ARIA error state in form field components Jun 24, 2024
@hennzen
Copy link
Author

hennzen commented Jun 27, 2024

As a followup info to my patch above: in order to have this patch applied, I ended up changing QSelect's import statement within its custom wrapper component. So I changed

import { QSelect, QIcon } from 'quasar'

to

import { QIcon } from 'quasar'
import { QSelect } from '../../node_modules/quasar/src/components/select'

Unfortunately, this causes QSelect to malfunction in another part of the application, where QSelect is nested within a QMenu: QMenu closes upon click on any of the options of the nested QSelect - making the the QSelect -of course- unusable.

Note: The changed import alone causes this. Even without applied patch!

So in consequence I cannot use that patch currently.

I have no clue yet what is going on. My expectation would be that impoting via direct path ../../node_modules/quasar/src... behaves the same as the "normal" import, which directs to a modules' dist folder, if I am not mistaken.

Feels as if I am missing things in the build-stack when cherry-picking a single component for local rebuild...

To reproduce check out https://github.com/hennzen/quasar-vite-plugin-testcase

@yusufkandemir
Copy link
Member

@hennzen without knowing where/why/how you import things, it's hard to tell. But, normally, import { QSelect } from 'quasar' gets transformed into import QSelect from 'quasar/src/components/select/QSelect.js'. If you are in a context, e.g. a library, it will not be transformed, so 'quasar' would point to node_modules/quasar/quasar.client.js(or similar). I hope this info will be beneficial for you to figure out the problem/solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/a11y Accessibility area/components bug/2-confirmed We have reproduce the problem and confirmed that this is a bug. kind/bug 🐞 Qv2 🔝 Quasar v2 issues
Projects
None yet
Development

No branches or pull requests

2 participants