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

Adjust aria roles and labels for collection search #95

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

nngu6036
Copy link
Contributor

@nngu6036 nngu6036 commented Nov 25, 2019

This PR adds an aria-label to the search input, changes the input type to search and changes the search form to a role of search rather than form.

This is certainly an improvement for users of assistive technology as it will announce it is a search rather than just a generic input (and screen-reader users will have it announced as a page landmark), however, some user testing should ideally be done in the future to find out if the markup is too explicit.

Todo

Add the fallback if no title is provided

@JeffersonBledsoe JeffersonBledsoe changed the title Add aria label to search input Adjust aria roles and labels for collection search Sep 4, 2020
# Conflicts:
#	src/collective/collectionfilter/portlets/collectionsearch.pt
#	src/collective/collectionfilter/tests/robot/keywords.robot
#	src/collective/collectionfilter/tests/robot/test_filterportlets.robot
#	src/collective/collectionfilter/tiles/search.pt
@JeffersonBledsoe
Copy link
Member

JeffersonBledsoe commented Jun 3, 2021

@djay If we add an aria-label/ aria-labelledby to the search input, is the placeholder text even required, it seems superfluous. For example, VoiceOver reads the input in the following format: Input name input placeholder, search text field. This ends up being search Search, search text field for a portlet with a title of Search, which is fairly confusing to me.

I propose having the input labelled by the portlet title rather than hard-coding aria-label="Search". This allows for multiple languages as well as customising the label of the search input. The input would be read as filterTitle, search text input then, which seems clearer to me and still help meet WCAG SC 1.3.1 and SC 4.1.2 out-the-box. It would appear follows.

Barceloneta enabled:
Screenshot 2021-06-03 at 5 23 01 pm

Raw content:
Screenshot 2021-06-03 at 5 24 14 pm

@JeffersonBledsoe JeffersonBledsoe marked this pull request as draft July 1, 2021 15:15
@JeffersonBledsoe JeffersonBledsoe marked this pull request as ready for review September 16, 2021 16:33
@JeffersonBledsoe JeffersonBledsoe marked this pull request as draft September 21, 2021 10:08
Copy link
Member

@djay djay left a comment

Choose a reason for hiding this comment

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

I think its better if there is a way to backdown to something still compliant should the user choose to not have a title.

@JeffersonBledsoe
Copy link
Member

@petschki @djay As part of this PR, I'd like to make it easier for CMS users to meet WCAG without having to think about this. Part of that would mean ensuring that the search input always has a label. While I could add fallback aria-label of something like "Contents" (not just the word search as the screen reader should now announce being focused on a search and to help ensure it doesn't conflict with the global search), this isn't really meaningful to the context the search is being used in. For example, a search across a collection of medical records should have something along the lines of "Medical records" as it's label, so it is announced as "Search, medical records".

How would you feel about making the portlet title mandatory (to ensure we get a meaningful input description), but having a checkbox to visually hide the title to preserve styling options collectionfilter users may have already made? An upgrade step should be able to automatically enable this checkbox for anybody who has left the portlet title empty in the past/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants