First off, this module has solved a huge problem for me, so thank you for creating it.

I'd like to contribute back by adding a small accessibility patch to add the "scope" attribute to table headers as dictated here:
http://webaim.org/techniques/tables/data#headers

Adds:
scope="col" for column headers, and
scope="row" for row headers.

Please let me know if there is a better way of doing this!

CommentFileSizeAuthor
#3 views_matrix-accessible-headers-2645492-3-7.patch3.17 KBmsharkeyiii
scope-for-headers.patch1.24 KBmsharkeyiii
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

msharkeyiii created an issue. See original summary.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
CreditAttribution: mradcliffe at Kosada commented
Status: Active » Needs work

Thank you for the patch. I appreciate adding proper attributes to improve web accessibility!

People do a git checkout of the module, make the changes, and then do a git diff > blah.patch when creating patches on modules or for Drupal core. There is a page if you are interested to read more at https://www.drupal.org/patch. We do this because then the maintainer or anyone who is reviewing the patch can apply the patch to the code base with the command curl https://www.drupal.org/files/issues/scope-for-headers.patch | git apply or by adding the patch url to a drush make file for instance.

I have reviewed the patch, and here are some comments.

  1. +++ views-view-matrix.tpl.php-new.php	2016-01-06 15:02:26.000000000 -0500
@@ -18,7 +18,7 @@
-        <th <?php print drupal_attributes($col_header['attributes']); ?>><?php print $col_header['data']; ?></th>
+        <th scope="col" <?php print drupal_attributes($col_header['attributes']); ?>><?php print $col_header['data']; ?></th>

@@ -27,9 +27,9 @@
-        <t<?php print ($col_index === 'header') ? 'h' : 'd'; ?><?php  print drupal_attributes($content['attributes']); ?>>
+        <t<?php print ($col_index === 'header') ? 'h scope="row"' : 'd'; ?><?php  print drupal_attributes($content['attributes']); ?>>

    It would be better to add these to the attributes element property in the template preprocess function so it's not hard-coded in the template. This is a bit difficult to find because views_matrix has a lot of code in the template preprocess function, but around line 106 and line 180 should do the trick. 

  2. +++ views-view-matrix.tpl.php-new.php	2016-01-06 15:02:26.000000000 -0500
@@ -27,9 +27,9 @@
+        </t<?php print ($col_index === 'header') ? 'h scope="row"' : 'd'; ?>>

    I do not think that attributes are allowed on the closing element.

So to re-iterate:

1. Move the scope attribute to the row and column header render arrays in template preprocess.
2. Revert the changes to the template.

:-)

msharkeyiii’s picture

msharkeyiii CreditAttribution: msharkeyiii commented
FileSize
views_matrix-accessible-headers-2645492-3-7.patch3.17 KB

Thank you for the feedback. Editing the preprocess arrays are much cleaner, yes!

I've made one additional change based on WAI example here regarding the first, empty cell:
http://www.w3.org/WAI/tutorials/tables/two-headers/#table-with-header-ce...

I've updated the template file to use a < td > instead of a < th > if the column header is empty, which is the case for the first cell, and doesn't use scope attribute.

msharkeyiii’s picture

msharkeyiii CreditAttribution: msharkeyiii commented
Status: Needs work » Needs review

  • mradcliffe committed 89278f8 on 7.x-1.x 
    Issue #2645492 by msharkeyiii, mradcliffe: Add scope attribute to...
mradcliffe’s picture

mradcliffe
he/him
Primary language English
CreditAttribution: mradcliffe at Kosada commented
Status: Needs review » Fixed

Thanks for the patch.

Status: Fixed » Closed (fixed)

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