Problem/Motivation

At the moment the button state , if pressed or not, of the list and grid buttons is not available in the aural interface. A screenreader user has no way to distinguish which of the two buttons is actually pressed, see missing_toggle_state.mp4.

Steps to reproduce
  • Go to /admin/modules/browse
  • Activate your screenreader of choice
  • Tab to the list and grid buttons
  • Also hover and press the buttons

Proposed resolution

CommentFileSizeAuthor
missing_toggle_state.mp4418.09 KBrkoller

Issue fork project_browser-3446591

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

rkoller created an issue. See original summary.

kwiseman made their first commit to this issue’s fork.

kwiseman’s picture

kwiseman commented

I added the suggested aria-pressed attribute to the Link and Grid buttons that functions similarly to the two button grouping on https://tink.uk.

However, I wasn't sure how to go about applying the hover and active state styling on them given that the background color of the selected display button (either Link or Grid) in any state is the same as the design system's button active state background color. (In other words, the background of a selected Link or Grid display button is always #ADAEB3). When hovering over a selected display button, should it get 15% darker than #ADAEB3, and 30% darker if the user clicks it again? I looked around in the design system and other admin pages to try to find an example to copy, but couldn't.

I was thinking a solution to this hover/active state styling problem could be changing the color of the selected display button to blue, and then apply the design system's hover and active state styling as usual. Project Browser's Figma Designs shows an example of this on the "Updated MVP April 2022" page, on the frames that start with "April 2022: New Design Draft", but since they're called drafts, I wasn't sure.

sime’s picture

sime
any/any
Location Melbourne
commented
Status: Active » Needs review

I think #3 makes sense, and i think this should be needs review.

rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
commented

hm i am not sure if changing the styling from the regular button grey to the primary button blue for the pressed button would be the right choice. might be potentially confusing :/ technically there doesn't exist an actual toggle state for the drupal design system unfortunately yet. :( the following comment and the comments in the following in this thread https://drupal.slack.com/archives/C1AFW2ZPD/p1704456082112179?thread_ts=... are about introducing a toggle state to claro. the switch between the grid and list view would actually another example to be named in the list of examples in this linked discussion.

I wonder if it would make sense to narrow the scope for this issue and make it only about screenreader accessibility and move the design to a dedicated issue and maybe ping @ckrina in that regard, bringing the new toggle state for the drupal design system back on the map? what do you all think?

and would it be possible that you open a MR for this issue fork. that way it would be easier to apply the patch for me since i rely only on the composer patches based workflow still. would have to manually test the changes in voiceover.

kwiseman changed the visibility of the branch 3446591-improve-styling-and to hidden.

kwiseman opened merge request !488

kwiseman’s picture

kwiseman commented

Personally, I think it makes sense to narrow the scope of this issue to just the screenreader accessibility and move the design part to its own issue, given that design for the toggle state doesn't exist yet.

I created a merge request on a new branch that's caught up with 1.0.x and has the same changes.

Also, I forgot to mention in #3 that I manually tested with VoiceOver on a Mac in Firefox. Manual testing with other screen readers and/or browsers is probably necessary.

rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
commented

thankt @kwiseman, this looks good! i've also tested in safari and edge with voiceover. the only small nitpick with voiceover is that if you have a state change from not pressed to pressed, voiceover is just announcing "selected" without repeating the context like for example "selected list toggle button". but that is a voiceocer thing.

one detail i've noticed, i am testing on drupal 11.x-dev, project browser is still using yarn 1.x, but drupal 11 is requiring yarn 4 for a few weeks now: https://www.drupal.org/node/3428571. would it make sense to create a follow up upgrading the yarn version?

and in regards of the styling, we've both agreed (according to your comment on the thread on slack) to move the styling to new issue. the only question is how to approach it. cuz that would be an issue for core in general, project browser would be the exception having two toggle buttons next to each other which are slightly different contstraints. not sure but probably a single issue summarizing the points from the linked thread including the requirement for project browser as another mandatory variant might be the best choice?

kwiseman’s picture

kwiseman commented
Title: Improve styling and accessibility for the list and grid button » Improve accessibility for the list and grid button
kwiseman’s picture

kwiseman commented
Issue summary: View changes

kwiseman closed merge request !488

kwiseman changed the visibility of the branch 3446591-improve-styling-and to active.

kwiseman changed the visibility of the branch 1.0.x to hidden.

kwiseman opened merge request !522

bernardm28’s picture

bernardm28 commented
Status: Needs review » Reviewed & tested by the community

Seems like this issue MR accomplishes the main goal of this issue.
It could always be improved later on a follow-up.

kwiseman’s picture

kwiseman commented
Version: 1.0.x-dev » 2.0.x-dev

kwiseman changed the visibility of the branch 2.0.x to hidden.

kwiseman opened merge request !523

chrisfromredfin made their first commit to this issue’s fork.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Reviewed & tested by the community » Fixed

Easy win for a11y!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.