♿ [#2379] Search feedback notification for screenreaders

[jiromaykin]

issue: https://taiga.maykinmedia.nl/project/open-inwoner/task/2379
the issue from the audit is not quite correct, when it says "...without adding focus...".
We only have dynamic content in our HTMX components (like the Spinner in Mijn Aanvragen) but we do not have dynamic content for our Notifications. Nor for our form Errors. These only becoem visible on PageLoad.
So we need to add focus on them or else most screen readers will ignore them.

role=alert and aria-live usually only work for dynamically loaded content, so hardly work for Divs that are loaded on new pageload.

Adding the negative tabindex= -1 will make the text focusable for javascript, without interfering with the natural Tab order.

Forcing focus feels 'hacky' but this is because there is no 'on page load' solution that will work the same for all screen readers.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.13%. Comparing base (abc196e) to head (2f6998e).
Report is 8 commits behind head on develop.

Files	Patch %	Lines
src/open_inwoner/cms/cases/tests/test_htmx.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #1280       +/-   ##
============================================
+ Coverage    44.23%   95.13%   +50.90%     
============================================
  Files          973      983       +10     
  Lines        35625    35839      +214     
============================================
+ Hits         15757    34096    +18339     
+ Misses       19868     1743    -18125     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jiromaykin]

@pi-sigma This one may look simple but it's quite the juggling of what not-not-not to do. So 'd be happy to explain what I am trying to do here in live conversation.

TLDR:

• add focus for javascript only
• keep the focused element OUT of Tab control
• removing visible focus style from the notification-content (my own decision, not sure if this is good/bad)
• terribly changing of the context by forcing Focus but this seems the only way for elements that are not 'dynamic' and can not make use of 'role=status' or 'role=alert' so 'aria-live' would not work.

[jiromaykin]

@pi-sigma I forgot: You can test this actual issue by searching and going to the Search page (activate Elasticsearch) and submit feedback ("Heeft u gevonden wat u zocht?"/"Feedback versturen") which will generate a Notifications section that contains 1 Notification.

Still my PR needed more tweaking because I based it on this answer, amongst others, and this means I will need to add role="region" at least (or other roles):
https://stackoverflow.com/a/41508795/21179736

but I am still not sure what to do if there are multiple Notifications, since you can only add focus on 1 element and I am not sure if I could just add focus to the surrounding NotificationS.html template, instead of adding focus to just the child 'list item' in notification.html. Styling is possibly applied in a 'cascading' order.

EDIT: I decided to add focus to the first notification in case there are multiple.

Plus: I did add overkill when also adding the focus to the Error.html template... Which is outside of the scope of this issue actually.
But those Error notifications are a bit problematic for screen-readers because in some Forms we now set 'autofocus' on some (?) input element, and not on any of the errors (guess that should be a new issue).

EDIT: I've also set focus on the first error (example: http://localhost:8000/accounts/register/)

[pi-sigma]

notificationContents behaves like a stack, so this will srcoll into the last notification (because it was the last one added). If you want to start with the first notification, you need to loop in reverse (so -1 instead of 0).

I don't know what's the intended (or best) behavior, though. If you want to focus on the last notification, you can ignore this.

[jiromaykin]

@pi-sigma Actually no. If you would manually add multiple templates for notifications by hand (after doing Git pull and rebuild NPM), you can see the reverse is not needed.

The code correctly targets the first notification content by using this.notificationContents[0].
There is no need to loop in reverse or use a different index. The code already achieves the intended behavior of scrolling to and focusing on the first notification, because I am "re-indexing the Nodelist" here.

If you want to visually see which 1 element truly has focus after PageReload, you can adjust/remove the _Notification.SCSS in this PR where I removed the browsers default focus-visible style.

You can test this manually this way: paste in multiple types of notifications in the wrong order, after the last line of the Notification.html template, so you can see the re-ordering and focus is happening correctly and generate these notifications by using the "Feedback versturen" button after doing a Search query:

<div class="notification notification--success ">
   <div class="notification__icon">
      <span aria-hidden="true" class="material-icons-outlined ">check_circle</span>
   </div>
   <div class="notification__content" role="status" tabindex="-1">
      <p class="utrecht-paragraph utrecht-paragraph--oip utrecht-paragraph--oip-compact">Success success</p>
   </div>
   <button class="button notification__close button--textless button--icon button--transparent" type="" name="" value="" title="Sluit deze melding" aria-label="Sluit deze melding">
   <span aria-hidden="true" class="material-icons ">close</span>
   </button>
</div>
<div class="notification notification--info ">
   <div class="notification__icon">
      <span aria-hidden="true" class="material-icons-outlined ">check_circle</span>
   </div>
   <div class="notification__content" role="status" tabindex="-1">
      <p class="utrecht-paragraph utrecht-paragraph--oip utrecht-paragraph--oip-compact">Info blue info</p>
   </div>
   <button class="button notification__close button--textless button--icon button--transparent" type="" name="" value="" title="Sluit deze melding" aria-label="Sluit deze melding">
   <span aria-hidden="true" class="material-icons ">close</span>
   </button>
</div>
<div class="notification notification--error ">
   <div class="notification__icon">
      <span aria-hidden="true" class="material-icons-outlined ">check_circle</span>
   </div>
   <div class="notification__content" role="status" tabindex="-1">
      <p class="utrecht-paragraph utrecht-paragraph--oip utrecht-paragraph--oip-compact">Error error</p>
   </div>
   <button class="button notification__close button--textless button--icon button--transparent" type="" name="" value="" title="Sluit deze melding" aria-label="Sluit deze melding">
   <span aria-hidden="true" class="material-icons ">close</span>
   </button>
</div>
<div class="notification notification--warning ">
   <div class="notification__icon">
      <span aria-hidden="true" class="material-icons-outlined ">check_circle</span>
   </div>
   <div class="notification__content" role="status" tabindex="-1">
      <p class="utrecht-paragraph utrecht-paragraph--oip utrecht-paragraph--oip-compact">Warning warning</p>
   </div>
   <button class="button notification__close button--textless button--icon button--transparent" type="" name="" value="" title="Sluit deze melding" aria-label="Sluit deze melding">
   <span aria-hidden="true" class="material-icons ">close</span>
   </button>
</div>