Dev 1165 advanced search form #81
Conversation
✅ Deploy Preview for hathitrust-firebird-common ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
This was a fun (but tricky) first a11y-testing for me!
I think everything works the way it is supposed to, given the tickets this intends to solve.
This was my first time seriously trying to use a screen reader (Orca on Ubuntu, in Chrome), so any screen reader issues may be 100% noob issues on my behalf:
-
I could not get the contents of the dropdowns in the "Search by field" form (e.g. "only full text", "all of these words") read out loud by the screen reader, so had I not used my eyes I would not have known those search qualifiers were present. The reader only told me the name of the box and that it opens a menu, but not what's in the menu or which option is selected.
-
The screen reader read each language twice as I tabbed through the list of languages, once just the language name and once with its checkedness: ["ainu, checkbox not checked", "ainu", "akan, checkbox not checked", "akan"]
-
Visually, the s in the language and format s jiggle a little too much for my liking when selected/unselected and the border gets applied. (Attaching video) mw_form_jiggle.webm I don't think my feedback should stand in the way of an APPROVE though.
|
Thank you for your review, @mwarin! I'll take a look at the first two screen reader issues. There are more advanced search form issues from deque and I can definitely address these when those are being worked on. (See https://github.com/hathitrust/firebird-common/issues/72 for example "dropdown fields don't have descriptive labels") As for number three, I agree, there is some wonky margin at play with the focus style. Gayathri will be designing all-new focus styles and I didn't want to spend too much time tinkering with this "placeholder" style that will be replaced probably next week. I will merge and keep these in mind for future work! |
Resolves some of #71 , #72
Fixes for critical advanced search form issues:
idattribute of the radio<input>and theforattribute of its corresponding<label>not only have to match, they must be unique. they matched before but were not unique, so I added an extra variable to theid/forcombo and now we're good 👍FilterableSelectionlists (language and format) on the advanced search forms, so now if you tab over them, you'll see a darker-orange box as you move through the list, and if you select an item with the space bar, the background of the them item turns orange and the focus ring turns black. Gayathri is working on a better design for all our focus states, butI wanted to fix this for now since it's a critical issue.Other fixes:
FilterableSelectioncomponent was still using the inaccessible shade of orange (primary-500), so I updated it to the approved slightly-darker-orange (primary-600)role="checkbox"is redundant when used withtype="checkbox", so I removed that hereto test
This is staged on dev-3: Advanced Search page, either in Catalog or Full-text.
boolean operators
To test the AND/OR boolean operator radio buttons, you'll need a screen reader. If you test in on prod, it's doing something like this:
Because all the AND buttons had the same id/for attributes, they were all being read together like "AND AND AND" but if you use a screen reader on dev-3 and tab through this version of the form, it should read the ANDs and ORs separately now.
Also, you may notice that the current version has this legend that says "Boolean operator for field 0 and field 1", and I fixed that, too. It will now say "Boolean operator for field 1 and field 2" and 2/3, 3/4 depending on which fieldset you're in.
filterable selection list
You don't need the screen reader for this one, but it doesn't hurt. Tab through the language and format selection lists and select an item using the space bar. There should be a dark orange focus ring around unselected items, and selected items should have a black focus ring.