For WCAG validation it is important to add semantic information to data tables. They are used throughout Drupal, but is most important for Views.

Some information about the process for creating good tables is in:
http://webaim.org/techniques/tables/data

Taking from the WebAim description:

"The purpose of data tables is to present information in a grid, or matrix, and to have column or rows that show the meaning of the information in the grid. When screen readers read straight through data tables—especially large ones—it's easy for users to get lost.

In order for a data table to be accessible, it must have the proper markup in the HTML. When the proper HTML markup is in place, users of screen readers can navigate through data tables one cell at a time, and they will hear the column and row headers spoken to them."

See http://drupal.org/node/183592#comment-3245200

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs work
+  $output .= theme('table', array('header' => $header, 'rows' => $rows, 'attributes' => array('id' => 'arrange', 'scope' => 'col')));

I don't get how this helps. Should this describe what the table does? t('Rearrange the items')

+  $vars['rearrange'] = l('<span>' . t('Rearrange') . '</span>', "admin/structure/views/nojs/rearrange/$view->name/$display->id/$type", array('attributes' => array('class' => array('views-button-rearrange', 'views-ajax-link'), 'title' => t('Rearrange'), 'id' => 'views-rearrange-' . $type, 'scope' => 'col'), 'html' => true));
+/
+  $vars['add'] = l('<span>' . t('Add') . '</span>', "admin/structure/views/nojs/add-item/$view->name/$display->id/$type", array('attributes' => array('class' => array('views-button-add', 'views-ajax-link'), 'title' => t('Add'), 'id' => 'views-add-' . $type, 'scope' => 'col'), 'html' => true));

This is confusing, too. You add the scope attribute to links, afaik this is not defined here

dawehner’s picture

Perhaps i'm just wrong, could you clarify me?

mgifford’s picture

Let me know if this clarifies anything - http://www.webaim.org/techniques/tables/data.php#scope

This is different from a table summary.

mgifford’s picture

Should I start a new version for D7? This is also a problem with D6, but assume it is only likely going to get fixed in D7 at this point and possible back-ported later (this would be normal).

I was also wondering if I should create a patch from the CVS or the last release. The CVS is currently at D6.

If patches aren't going to be applied for D5 or D6 should the issues be changed to "Won't Fix"?

dawehner’s picture

I think it should be fixed for 6.x, too. As long the codebase keeps similar this makes porting of patches easier

dawehner’s picture

Status: Needs work » Needs review

Can you explain why the full table need's a scope?

mgifford’s picture

There's a side of this argument that is just about compliance to a W3C standard:
http://www.w3.org/TR/WCAG-TECHS/H63.html

That part is pretty clear.

The better reason is that for screen readers it can be used to provide more points to better navigate the stream of data that can be contained in a data table. Tables need to be properly formatted because a lack of row and column headings and specific attributes could prevent users with screen readers from navigating tables effectively.

The problem with this is that data tables haven't been consistently built with scope, caption & summary descriptions defined in them so many blind users don't look for this data.

Adding it is a best practice however and part of getting a website through an accessibility audit. Hopefully with a consistent application of scope, caption & summary descriptions it will be easier for new users of all abilities to learn to use Drupal.

@dereine - I'm totally up for backporting this to 6 if we can find a good solution for 7. I'm just not clear what the best approach is for Views & if I should be using HEAD (which I think is still 6) for providing a patch.

dawehner’s picture

In views the current practise is to patch the drupal6 version , and then if the patch applies merlinofchaos commits in to d7 and if it does not commit, i port the patch.

You should write a patch against DRUPAL-6--2 or DRUPAL-6--3 or DRUPAL-7--3 afaik HEAD is outdated

mgifford’s picture

Ahh, so like this:

$ cvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout -r DRUPAL-6--3 -d views contributions/modules/views

I've got more D6 versions with views kicking around that I can play with. I can try to incorporate some ideas from:
http://www.isolani.co.uk/articles/structuredTables.html

Good to know how things are different here in Views than core. Does make sense considering the size/importance of the code base. Is there any documentation on this?

dawehner’s picture

No i don't think so :) it's a silent rule.

mgifford’s picture

Ok, so this should improve some of the output of the view.

I noticed that there doesn't seem to be a way to insert a summary into the output. I'd like to see something like this with some means for users to enter in a descriptive value about the table.

<table class="<?php print $class; ?>" summary="$summary">

To test this patch one should be able to look at the output of any view table produced. I had some trouble finding the scope="row" & scope="col" in my output though so perhaps this isn't where it should be applied.

The patch should work for some elements. This is a port of what I did in #183592: Small table accessibility improvement but need places to test this

Index: includes/admin.inc
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/views/includes/admin.inc,v
retrieving revision 1.154.2.32
diff -u -p -r1.154.2.32 admin.inc
--- includes/admin.inc	17 Jun 2010 02:45:30 -0000	1.154.2.32
+++ includes/admin.inc	26 Jul 2010 03:11:03 -0000
@@ -1273,9 +1273,9 @@ function template_preprocess_views_ui_ed
       $rearrange_url = "admin/build/views/nojs/rearrange/$view->name/$display->id/$type";
   }
 
-  $vars['rearrange'] = l('<span>' . t('Rearrange') . '</span>', $rearrange_url, array('attributes' => array('class' => 'views-button-rearrange views-ajax-link', 'title' => t('Rearrange')), 'html' => true));
+  $vars['rearrange'] = l('<span>' . t('Rearrange') . '</span>', $rearrange_url, array('attributes' => array('class' => 'views-button-rearrange views-ajax-link', 'title' => t('Rearrange'), 'scope' => 'col'), 'html' => true));
 
-  $vars['add'] = l('<span>' . t('Add') . '</span>', "admin/build/views/nojs/add-item/$view->name/$display->id/$type", array('attributes' => array('class' => 'views-button-add views-ajax-link', 'title' => t('Add')), 'html' => true));
+  $vars['add'] = l('<span>' . t('Add') . '</span>', "admin/build/views/nojs/add-item/$view->name/$display->id/$type", array('attributes' => array('class' => 'views-button-add views-ajax-link', 'title' => t('Add'), 'scope' => 'col'), 'html' => true));
 
   if (!$display->handler->is_default_display()) {
     if (!$display->handler->is_defaulted($types[$type]['plural'])) {
dawehner’s picture

Title: Rearrange table accessibility » Improve accessibility of views

This is strange. You changed a total different file :)

This file is not part of the admin pages at all. No wondern that you cannot find find the output of the table :)

We could say you say what's needed to be changed and where and i create the patch, i think this saves time for everyone :)

One suggestion: We could add a textfield for administrators to be able to add a summary attribute to the views table output.
What do you think about this?

mgifford’s picture

FileSize
100.99 KB

Ya, sorry. My main goal was to look for appropriate places to add scope = "col" to table cells in Views. would be good to also add scope = "row", but I do think this could be more complicated, there are some good examples here mind you - http://webaim.org/techniques/tables/data

I was looking at both the admin output as well as the output of the view. I'm assuming that the theme/views-view-table.tpl.php is used at any time where there is a table view defined and that you should be able to see it in the output there. But ya, I didn't explain what I was doing very well. Just came to the end of what time I had to contribute at that point and then needed to post it somewhere so that it wasn't lost.

I do think that a textfield for administrators to add summary attributes would be excellent! Simply to provide that as an option would be great. Having a caption field along with that would also be a good improvement.

Oh, I'd so love it if you would create the patch. Views is a pretty big module & probably one of the more complicated ones in Drupal. I'd like to see more default implementations like this for public content:

    <thead>
    <tr>
              <th class="views-field views-field-sourceid" scope = "col">
          <a href="/admin/content/tw/view/map?order=sourceid&amp;sort=asc" title="sort by sourceid" class="active">source id</a>        </th>
              <th class="views-field views-field-destid" scope = "col">
          <a href="/admin/content/tw/view/map?order=destid&amp;sort=asc" title="sort by destid" class="active">destination id</a>        </th>

              <th class="views-field views-field-needs-update" scope = "col">
          <a href="/admin/content/tw/view/map?order=needs_update&amp;sort=asc" title="sort by needs_update" class="active">needs_update</a>        </th>
          </tr>
  </thead>

There are going to need to be changes to the Drag/Drop rearrange section in the admin page (which I think now is the only place where tables are used here), but we can't really fix it in six & then migrate that to seven as core's basically broken in D6 for drag/drop capacity. It will be interesting to be able to enable/disable this table in D7....

In anycase, improving the output will benefit the most people so very excited about this.

Taken from the Yourhtmlsource guide on table accessibility.

The caption tag, which will appear on the screen is used to give the title of the table.
The summary attribute gives a broader description of the table's purpose.

Views is capable of some pretty complex stuff. Would be good if there were ways of making that content more accessible to people. Some useful guides include:
http://www.usability.com.au/resources/tables.cfm
http://www.userite.com/ecampus/lesson3/lesson3c.php
http://www.evengrounds.com/developers/accessible-tables
http://webstandards.psu.edu/accessibility/tech/tables

Note that the last two are Drupal sites.

dawehner’s picture

Assigned: Unassigned » dawehner

I do think that a textfield for administrators to add summary attributes would be excellent! Simply to provide that as an option would be great. Having a caption field along with that would also be a good improvement.

Sure this is possible.

Assign to me

merlinofchaos’s picture

I'm totally on board with dereine in #14. That's an easy improvement that would go a long ways.

dawehner’s picture

Here is a issue to add a summary for tables: #868972: summary attribute for tables

mgifford’s picture

this is exciting! Thanks for setting up the new issue about summary tables and already providing a patch!

dawehner’s picture

Here are two new issue. I suggest to use this issue as a collection for other issues.

mgifford’s picture

Hey Dereine,

I'm just trying to see what we can do to move this along. You've opened two new issues. Thanks! I'll look at those.

I think that this one here then will be focused on views/includes/admin.inc and adding the scope = "col" in six as I tried to do it in seven in this patch: http://drupal.org/node/183592#comment-3245200

Is that right? I don't want to loose all momentum on bringing accessibility into views.

hobo’s picture

adding scope="col" to all the headers would be good. currently i'm doing this through the theming layer..
as far as i know tables generated by views have headers on top and data below.. which is perfect for scope.

it might be good to have an option for this.. but the cases where you wouldn't want scope="col" are only if you make views generate a table with headers on the left or more complex header structure.. in this case i think doing the work in the theming layer will make sense (probably because you will be doing some heavy theming to make the tables look so different anyways) so there should be an option 'remove scope from table' because 99.99% of the time this is wanted.

This is needed for anyone working with the wcag standards.. (a lot of government sites require this as well as sites which need to be accessible)

So thanks for putting in the caption!! brings us a step closer!!

merlinofchaos’s picture

Status: Needs review » Needs work

Due to changes in from the semantic views patch, this patch will no longer apply.

I think I understand the point of the scope variable now, and certainly in the table style I think this is a good idea. So +1 to the effort here. We might also consider doing it for the grid style, but I'm not certain if scope is appropriate on the grid which is a table but without headers.

However, instead of using a counter it's pretty easy to calculate the first field using something along the lines of reset($array); $first_key = key($array) and then doing an if to compare.

I also would rather see a scope variable created in the preprocess than actually embedded harshly into the template like this. This fails my sniff test of "too much PHP code in the template."

dawehner’s picture

Status: Needs work » Needs review
FileSize
2.21 KB

What about this?

drupal_attributes can be replaced in d7 with a process function.

mgifford’s picture

Ok, so how do we test this again?

Is the latest patch against the latest dev release of D7? Or is this against the CVS of views in D6?

Just want to help if I can.

merlinofchaos’s picture

This patch appears to be against latest CVS of DRUPAL-6--3

merlinofchaos’s picture

Assigned: dawehner » bojanz

Assigning to bojanz for testing.

Sunflower’s picture

Is there a Drupal 7 solution for this?

Status: Needs review » Needs work

The last submitted patch, 864006-view-table-scope.patch, failed testing.

mgifford’s picture

Title: Improve accessibility of views » Improve table semantics by adding scope element
Version: 6.x-3.x-dev » 7.x-3.x-dev
Status: Needs work » Needs review
FileSize
1.84 KB

Changing the title to be more specific, bumping this to D7 & upgrading the patch.

colan’s picture

Status: Needs review » Needs work

scope="row" is there, but what about scope="col"? Is this taken care of elsewhere?

mgifford’s picture

Status: Needs work » Needs review
FileSize
3.54 KB

Try this.

mgifford’s picture

Status: Needs review » Postponed (maintainer needs more info)

I'm actually going to mark this as postponed. I'm not sure what the future of the scope attribute is after seeing:
http://www.developphp.com/view_lesson.php?v=545

Certainly doesn't appear here:
http://www.w3.org/TR/html5/global-attributes.html#global-attributes

If it's depreciated in HTML5, is this really the right time to be adding it into Views (even though D7 is going to be HTML4 by default).

Yet there are still lots of docs that suggest it's a best practice:
http://developers.whatwg.org/tabular-data.html#attr-th-scope

And no alternate example from WCAG:
http://www.w3.org/TR/WCAG20-TECHS/H63

I think we need another opinion before this can go much further.

Jeffdo’s picture

Status: Postponed (maintainer needs more info) » Needs review

I'm not sure what the future of the scope attribute is after seeing ...

All that's noteworthy about scope in that list of HTML5 deprecated elements/attributes is that we can no longer use the scope attribute on a TD element. Scope's valid on a TH element. Makes sense, semantically.

Certainly doesn't appear here ...

That's a list of global attributes. Scope is just used on TH elements.

Yet there are still lots of docs that suggest it's a best practice ...

It's in the HTML5 spec.

And no alternate example from WCAG ...

H63 is one WCAG 'technical solution' for associating a table cell with its row or column header(s). Another's H43, which describes how to do it using id/headers attributes.

To sum up, scope is solidly in the spec. At the moment, the only problem with scope is dodgy support in some user agents, as mentioned at the top of H63 in 'User Agent and Assistive Technology Support Notes.'

mgifford’s picture

Thanks @Jeffdo I've been engaging with others on Twitter about this. When bringing something into a module as important as Views I'd like to make sure that we're right about this being a best practice.

As you correctly stated, with table headers scope is still supported:
http://dev.w3.org/html5/alt-techniques/#hag21

Looks like either TH/scope or id/header approaches will work:
http://webaim.org/techniques/tables/data/#id
http://www.evengrounds.com/developers/accessible-tables
http://www.jimthatcher.com/webcourse9.htm
http://www.w3.org/TR/WCAG20-TECHS/H43
http://www.evengrounds.com/developers/accessible-tables

For Section 508 folks:
http://www.hhs.gov/web/policies/webstandards/htmltable.html

Both add semantics to the tables that is presently missing. I'm not sure much is being added semantically by simply adding a scope attribute with a direction to the TH tag, but it certainly seems easier.

Thanks for re-opening it.

mgifford’s picture

Ok, let me just simplify this a bunch. Views at this stage doesn't allow users to customize the TH element. In a row, you can't make a TD into a TH because it makes more semantic sense. So if we're going with scope we might as well just add it to the table header in the views-view-table.tpl.php's template like:

-          <th <?php if ($header_classes[$field]) { print 'class="'. $header_classes[$field] . '" '; } ?>>
+          <th <?php if ($header_classes[$field]) { print 'class="'. $header_classes[$field] . '" '; } ?> scope="col">

This would provide some semantic improvements and would output a header row like:

      <tr>
                  <th class="views-field views-field-title"  scope="col">
            Title          </th>
                  <th  scope="col">
            Overview          </th>
                  <th  scope="col">
            Listing          </th>
                  <th  scope="col">
            Options          </th>
              </tr>
    </thead>

Views table handling would need to much more robust (as I understand it), to make use of any of the other complexities that often are needed in producing accessible tables.

colan’s picture

Status: Needs review » Needs work

Now the "row" is missing.

mgifford’s picture

You can't add the row. Scope is only valid on TH's not TD's. TD's are only available as the rows. The only option available to us (that I can see) is the patch I've added. I'm all for Views being able to create more complex views, but don't think that is possible.

colan’s picture

Status: Needs work » Needs review

Gotcha. Let's get some more eyeballs on this.

mgifford’s picture

Title: Improve table semantics by adding scope element » Improve table semantics by adding scope or headers/id attributes
FileSize
1.34 KB

Ok, here's a better solution that will give semantics that AT can use for all cells. This approach is pretty simple, but I can't see a use case where it doesn't work. It's more useful than #34 because it gives details for the individual cells basically following the ID method proposed here rather than scope.

http://webaim.org/techniques/tables/data/#id

Also the related issue for core's defaults is - #1645308: Improve table semantics by adding scope or headers/id attributes to theme_table()

dawehner’s picture

Status: Needs review » Needs work
+++ b/theme/views-view-table.tpl.phpundefined
@@ -26,7 +26,7 @@
+          <th <?php if ($header_classes[$field]) { print 'class="'. $header_classes[$field] . '" '; } ?> id="<?php print $field; ?>">

Wouldn't this potential introduce duplicated ids? I'm really not sure whether we should add unexpected IDs to the html output. What about view-$name-$display_id-table-field as ID? This should be hopefully unique enough, but to ensure that, we can use drupal_html_id in the preprocess function.

Beside from that, isn't it actually also important to add these kind of help to the admin UI?

mgifford’s picture

@dawehner I can see how that would be possible. We'd just have to expose the $name & $display_id variables in
theme/theme.inc to theme/views-view-table.tpl.php though as I'm pretty sure they aren't right now.

There's definitely a related issue from Core that I've also been working on.

To be clear though it will actually bring the admin UI closer to meeting WCAG 2.0 AA guidelines (which is quite important for many institutions), but I'm not certain at this point how much of an actual benefit it will provide to screen reader users.

The disadvantage of the id/header approach in #38 is just adding additional text to the html output. It's much bigger than the scope method suggested in #34 (although the patch there is incomplete).

dcmouyard’s picture

Component: User interface » Code
Assigned: bojanz » Unassigned
Status: Needs work » Reviewed & tested by the community

The patch in #34 is the correct approach and should be committed. This was the approach we took on EPA.gov to make Views-generated tables accessible.

Related issue for Views in Drupal 8.x: #1831162: Tables generated by Views need better semantics.

dawehner’s picture

I trust mgifford for accessiblity reasons so i don't think #34 is rtbc, see http://drupal.org/node/864006#comment-6463332

mgifford’s picture

Really it's either the scope method or header/id. There are advantages & disadvantages to either.

The main problem with the header/id approach is bloat. I can't tell you how much larger it makes the table, but when we're building D8 for a mobile environment, you gotta count the characters.

The method I proposed in 38 was simpler, but it had not ability to match row headers. This is partly because there is no way to define row headers in Drupal at this point.

I think it's fair to say that many blind users don't really need either scope or header/id definitions if it is a simple table. Most Drupal tables are just dead simple to understand and there is very little benefit for most screen reader users to have this for Drupal tables. Least that's what I understand.

The guidelines however specify that one of the two approaches should be used in a data table.

If it's good enough for the EPA.gov at this point, I think it's a model that the Drupal community can follow. I don't know of another agency that has implemented a different approach. It's a light weight approach that addresses the basic use case in Drupal without much bloat.

Thanks for your confidence @dawehner - I think the scope="col" approach may be the best we can do.

dcmouyard’s picture

The header/id approach is only needed for more complicated data tables.

mgifford’s picture

Agreed. And although I'd like to see Views able to create more complex tables, I don't think we're there yet.

mgifford’s picture

#38: 864006-view-table-scope-38.patch queued for re-testing.

dawehner’s picture

So I'm still worried that my comment in #864006-39: Improve table semantics by adding scope or headers/id attributes is still TRUE. You can for sure workaround that using some preprocess logic and call drupal_html_id() which would solve that. Isn't that a possibility?

mgifford’s picture

It was a good point. I didn't see it as being very likely, but very likely isn't the same as producing consistently valid HTML. Running view-$name-$display_id-table-field through drupal_html_id() would be useful to ensure it's unique.

mgifford’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 7.x-3.x-dev » 8.x-dev
Component: Code » views.module
Status: Reviewed & tested by the community » Needs review

I applied this to D8.. Feel free to revert back to D7 Views module.

mgifford’s picture

oops

dawehner’s picture

This looks great, thank you. (We can get this into 7.x-3.x for sure.).

The only thing i'm wondering is whether we should document this new variable in the template and whether adding a test would be helpful.

mgifford’s picture

Ok, here's some template documentation. This can be backported to D7, but lets see if we can bring it into D8.

Tests are always good, but I don't write them.

dawehner’s picture

This looks great beside this one small thing:

+++ b/core/modules/views/views.theme.incundefined
@@ -480,6 +480,7 @@ function template_preprocess_views_view_table(&$vars) {
+$vars['id'] = array();

Sorry ... two spaces are needed here.

I guess it would be great if someone could write an issue summary, so other people understand this change.

mgifford’s picture

Spaces added. I also added a tag to help with the summary.

Essentially it's a change from just using the column to using the full header/id approach to the Views tables. It's a more comprehensive approach to adding semantics to the tables.

dawehner’s picture

The code is looking great, but I have no idea about the accessibility aspect.

Does the core gates tell that you should have manual testing for that?

mgifford’s picture

Realized that I needed to only use drupal_html_id() once.

mgifford’s picture

I will do some manual testing on this, but as far as the core gates go, I'm pretty sure it's fine to go with the best practices.

Status: Needs review » Needs work
Issue tags: -Needs issue summary update, -Accessibility

The last submitted patch, 864006-view-table-scope-56.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review

#56: 864006-view-table-scope-56.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 864006-view-table-scope-56.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review
Issue tags: +Needs issue summary update, +Accessibility

#54: 864006-view-table-scope-54.patch queued for re-testing.

mgifford’s picture

addressing php notice.

dawehner’s picture

+++ b/core/modules/views/templates/views-view-table.tpl.phpundefined
@@ -38,7 +40,7 @@
+          <td <?php print $field_classes[$field][$row_count]; ?> headers="<?php print $headers[$field]; ?>">

+++ b/core/modules/views/views.theme.incundefined
@@ -570,6 +573,9 @@ function template_preprocess_views_view_table(&$vars) {
+      // Improves accessibility of complex tables.
+      $vars['headers'][$field] = (isset($vars['id'][$field])) ? $vars['id'][$field] : '';

It would be cool, if the variable could be named a bit different. There are already two similar variables "header" and "header_attributes". Why not use $vars['field_classes'] directly, because this isalready an Attribute object, so you can add new attributes.

mgifford’s picture

@dawehner - as it's defined right now this patch has nothing to do with CSS classes so not sure to add it to "An array of classes to apply to each field". It's a different attribute, so not sure it would make sense to just add it to "field_classes". I'm open to suggestions though.

I couldn't find a "header_attributes" variable when grepping through d8.

dawehner’s picture

FileSize
3.39 KB

Let's do it properly and add a todo.

Status: Needs review » Needs work

The last submitted patch, drupal-864006-65.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
807 bytes
3.41 KB

Let's do it different.

Status: Needs review » Needs work
Issue tags: -Needs issue summary update, -Accessibility

The last submitted patch, drupal-864006-67.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review

#67: drupal-864006-67.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Accessibility

The last submitted patch, drupal-864006-67.patch, failed testing.

dawehner’s picture

Issue tags: +Needs reroll

Well, even it is a reroll, it is probably more like a full rewrite as now twig is in core.

AjitS’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.91 KB

Re-rolling the patch from #67

mgifford’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
56.41 KB
117.23 KB

I'm going to mark this RTBC. Looks good to me!

Table-headers.png

Table-headers-WAVE.png

YesCT’s picture

Issue tags: +RTBC July 1

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/views/views.theme.incundefined
@@ -542,6 +542,7 @@ function template_preprocess_views_view_table(&$vars) {
+  // @todo Remove field_classes to field_attributes.

@@ -606,6 +607,7 @@ function template_preprocess_views_view_table(&$vars) {
+      // @todo Rename to header_attributes.

Is there a follow up to address these todos?

Also the issue summary could do with an update.

mgifford’s picture

@dawehner these were added in #65 based on feedback in #63. Can you create the follow-up issues for these todos? Also verify that they are still relevant.

@alexpott agreed on the summary. I've bumped it up a bit.

dawehner’s picture

Status: Needs work » Needs review
mgifford’s picture

#72: drupal-864006-72.patch queued for re-testing.

mgifford’s picture

Status: Needs review » Reviewed & tested by the community

I believe that @alexpott's concerns have been addressed now so moving this back to RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll

git ac https://drupal.org/files/drupal-864006-72.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1954  100  1954    0     0   3088      0 --:--:-- --:--:-- --:--:--  4800
error: patch failed: core/modules/views/views.theme.inc:542
error: core/modules/views/views.theme.inc: patch does not apply
mgifford’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.98 KB

re-roll...

bowersox’s picture

Issue tags: +TwinCities

Looks like this just needs a re-roll and review to be RTBC again.

mgifford’s picture

#81: drupal-864006-81.patch queued for re-testing.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Let's get it in again.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 41f6e35 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Needs issue summary update, -Accessibility, -RTBC July 1, -TwinCities

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.