Running a page through http://wave.webaim.org/ results in an error.
This markup is the issue
<button class="pswp__button pswp__button--close" title="<?php print t('Close (Esc)'); ?>"></button>
<button class="pswp__button pswp__button--share" title="<?php print t('Share'); ?>"></button>
<button class="pswp__button pswp__button--fs" title="<?php print t('Toggle fullscreen'); ?>"></button>
<button class="pswp__button pswp__button--zoom" title="<?php print t('Zoom in/out'); ?>"></button>
Proposed solution is to remove the title attribute, which is ignored by many screen readers (See article I thought title text improved accessibility. I was wrong.) and add title tags to aria-label (See MDN example here).
Propose changing markup to:
<button aria-label="<?php print t('Close (Esc)'); ?>" class="pswp__button pswp__button--close" title="<?php print t('Close (Esc)'); ?>"></button>
<button aria-label="<?php print t('Share'); ?>" class="pswp__button pswp__button--share" title="<?php print t('Share'); ?>"></button>
<button aria-label="<?php print t('Toggle fullscreen'); ?>" class="pswp__button pswp__button--fs" title="<?php print t('Toggle fullscreen'); ?>"></button>
<button aria-label="<?php print t('Zoom in/out'); ?>" class="pswp__button pswp__button--zoom" title="<?php print t('Zoom in/out'); ?>"></button
And doing the same on the previous/next buttons. I've attached a revised template file to this post.
Comments
Comment #2
mgiffordButtons are pretty simple:
http://webaim.org/techniques/forms/controls#button
ARIA isn't all that well supported in older browsers either. Any reason not just to use
<button class="pswp__button pswp__button--close" ><?php print t('Close (Esc)'); ?></button>or even<button class="pswp__button pswp__button--close" ><span class="element-invisible"><?php print t('Close (Esc)'); ?></span></button>This is basically the approach suggested here:
https://www.nczonline.net/blog/2013/04/01/making-accessible-icon-buttons/
Note that the silktide.com article is focused on links not buttons.
Comment #3
bdevore commentedI think that approach works just as well. I actually used that one on a different project, but wasn't sure how the maintainer would feel about making that change and adding a css file that could potentially collide with the photswipe library. I'm not sure how accessible users feel about older browsers or what their preference is for visually hidden inline text vs aria.
Comment #4
mgiffordAll good considerations. Thanks for reporting it. Want to submit a patch?
Comment #5
bdevore commentedPatch attached. It's my first ever so fingers crossed I did things correctly.
Comment #6
bdevore commentedComment #7
mgiffordI am just installing your patch here https://simplytest.me/
It seems to apply, so now it's just a matter of it doing the right thing. It doesn't work with jQuery Update (>2.2), so may not be able to test it.
Comment #8
anybodyIs there a .visually-hidden class in Drupal 7 core?
Could someone check if we're having the same issue with the 3.x version? Then we should at least fix it there.
Comment #9
anybodyComment #10
anybodyAs of 2022 we'll use aria-label as it doesn't require a element-invisible class, which isn't existing in Drupal 7. That should be a good mix. :)
Could someone please check if this is fixed by Photoswipe v5 library? That would be nice for Drupal 9+!
Comment #13
anybodyHere we go! Fixed in 7.x-2.x - please have a look!