Aria Accessibility compliance. fixes #765

[jameswilson]

This is a formal approach to fixing #765 with proper use of Aria attributes and is an alternative to PR #821 (Update: #821 has been incorporated into this PR).

Changes include:

• Add aria-hidden attributes on the dialog box and navigational buttons.

• Update the aria-hidden value to true or false at the appropriate times.

• Add aria-label attributes to navigational buttons using the colorbox default settings at page load time and then updating the value based on user-settings when a colorbox is loaded.

• Add aria-labelledby and aria-describedby attributes to the div containing role='dialog'.

[jameswilson]

As I just mentioned over on 765, the original code here did not pass HTML_CodeSniffer, but passes the http://wave.webaim.org

@jackmoore If you're interested in having this as the definitive solution, I've just fixed this to incorporate the code from PR #821 into this one, so you can close and ignore 821, and just use this one.

[jerseycheese]

@jameswilson This is great, but I'm noticing a typo:

.attr('aria-lable', settings.get('slideshowStop'))

.attr('aria-lable', settings.get('slideshowStart'))

Should be aria-label right?

[jameswilson]

You're totally right @jerseycheese. Sorry about that -- just sent a fix for that in a2b39f2.

Anything else? I suppose I could squash the three commits into one before merge if @jackmoore wants to have a cleaner git history.

[jameswilson]

Also, @jackmoore: do need me to run the js through a minifier in the PR?

[Enchiridion]

I was just looking into updating colorbox for better accessibility myself and found you've already done most the work! I think it looks great, but I think a couple of changes should be made so it's more flexible:

• Allow setting different accessibility text for the close/next/prev/etc buttons. On my site, I set all those options to empty strings and use css + symbol font in their place. However this causes all the aria-labels to be blank. How about adding some new string options like this?

{
  close       : '',
  closeAlt    : 'Close dialog',
  next        : '',
  nextAlt     : 'Next image',
  previous    : '',
  previousAlt : 'Previous image',
  // and so on for the rest of the strings
}

• Allow setting separate accessibility text for the title. I often use colorbox for non-image purposes, so the built-in title option is rarely used, which is currently how the dialog a11y label is set. It was be great if I could specify a separate title for use in aria-label or an id for use in aria-labelledby, as I often add my own title inside the modal. Maybe add a setting like this?

{
  title          : false,                  // Disable the built-in title
  titleAlt       : 'My dialog title here', // Set custom title directly
  // or point to custom title
  titleElementId : 'element-id-here,
}

• Between lines 494 - 516, a number of aria strings are being hard-coded, ie 'aria-label': 'previous' instead of fetching the string from the settings.

[jameswilson]

@Enchiridion Thanks for the feedback!

I set all those options to empty strings and use css + symbol font in their place. However this causes all the aria-labels to be blank.

This is a case where it would really be useful to have maintainer feedback 😞 @jackmoore I'm tempted not to add extra text label fields. The problem with the two fields for each button is that someone will unwittingly add the same text in both places, which might actually hurt accessibility, no? It means you'd have to add extra code to check for the various different conditions like if both are non-empty which one do you use which one do you hide with aria-hidden? If one is empty and the other is not, do you just not add the empty aria-label? etc. A couple workarounds come to mind (untested): 1) hide the button text with traditional "visually-hidden" styles in css (eg, negative text indent, etc) and place your symbols inside absolutely-positioned pseudo-elements. 2) provide custom html in the button text that includes everything you might need previous: '<span class="icon-left"></span><span class="visually-hidden">Previous</span>'. Thoughts?

Allow setting separate accessibility text for the title.

The original purpose of this PR was just be able to pass WAVE. I'd be happy if you cloned from my fork, and added a sub-feature-branch off of the 756-accessibility-compliance branch, submit a PR that I could merge into this one. Not sure if this is the right procedure or not, so let me know.

Between lines 494 - 516, a number of aria strings are being hard-coded, ie 'aria-label': 'previous' instead of fetching the string from the settings.

The reason for this is because that code runs before a colorbox instance has been initiated and even so all you have available are the defaults anyway, so to make the code shorter I just used duplicate strings from the defaults. I'm curious if you have a better idea here.

[regularlabs]

Hope this gets pulled soon. But project looks dead :( No new releases since 2016/05/10

[jameswilson]

Possibly, this is superseded by #832 from @bramd but a bit of a description that outlines the changes there would be helpful.

[lkmorlan]

Is there a reason aria-hidden is used instead of display: none? If something is visually hidden and has aria-hidden it might as well just be display: none.