search

modxcms / a11y

MODX Accessibility for the Manager
13 stars 10 forks source link

Content Area Title needs to be H2 #55

Closed dubrod closed 8 years ago

dubrod commented 8 years ago

Per the interface requirements image.

Collapsible regions such as "Content" need to have a title with <h2>

dubrod commented 8 years ago

this section also requires aria-expanded="true/false" but the markup is not a true parent child relationship, its based on the toggle icon setting new Positioning. 

<div id="modx-resource-content" class="x-panel x-form-label-left" style="width: 1224px;">
  <div class="x-panel-header x-unselectable" id="ext-gen121">
  <div class="x-tool x-tool-toggle" id="ext-gen124">&nbsp;</div>
  <span class="x-panel-header-text" id="ext-gen125">Content</span>
  </div>
<div class="x-panel-bwrap" id="ext-gen122" style="position: static; top: 0px; left: 0px;">

do we need to alter the markup to a true parent child relationship or will adding the aria-expanded attribute to the x-panel-header suffice since it is the element above it?

@kensgists @paulbohman

paulbohman commented 8 years ago

It doesn't need to be a true parent-child relationship. You can add aria-controls="ext-gen122" to the expand/collapse controller. You may also want to use aria-label or aria-labelledby on the panel to give it a name, especially if the panel is focusable when using the tab key. If it's not focusable, the aria-label/aria-labelledby value may not be read by screen readers, because support isn't yet very good for those attributes on non-focusable elements, but I would probably still add a name.

theboxer commented 8 years ago

@dubrod so how the HTML should look like?

dubrod commented 8 years ago

if its possible to render the generated id of the content div (x-panel-bwrap) in the x-panel-header above it. It would look like this.

<div class="x-tool x-tool-toggle" id="ext-gen124" aria-controls="ext-gen122">&nbsp;</div>

Next change would be adding aria-label:

<div class="x-panel-bwrap" aria-label="Content" id="ext-gen122" style="position: static; top: 0px; left: 0px;">

Next would be setting the correct aria-expanded=true/false when x-tool-toggle is clicked:

<div class="x-tool x-tool-toggle" id="ext-gen124" aria-controls="ext-gen122" aria-expanded="true">&nbsp;</div>
theboxer commented 8 years ago

Changes applied, please review

dubrod commented 8 years ago

Looks perfect. Assigning to the stakeholders for testing.

dubrod commented 8 years ago

I also found in another request file to add aria-label="Hide Content Field" to this toggle.

I did that.

dubrod commented 8 years ago

I also found in another request to add SR only text.

I did that.