quasarframework / quasar

Quasar Framework - Build high-performance VueJS user interfaces in record time
https://quasar.dev
MIT License
25.71k stars 3.49k forks source link

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

Open hennzen opened 2 months ago

hennzen commented 2 months ago

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]:

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 commented 2 months ago

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 https://github.com/nvaccess/nvda/issues/8318) but 2024.3 will, according to the pull request.

yusufkandemir commented 2 months ago

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.

hennzen commented 2 months ago

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 commented 2 months ago

@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.