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."
Comment | File | Size | Author |
---|---|---|---|
#81 | drupal-864006-81.patch | 1.98 KB | mgifford |
#73 | Table-headers.png | 117.23 KB | mgifford |
#73 | Table-headers-WAVE.png | 56.41 KB | mgifford |
#72 | drupal-864006-72.patch | 1.91 KB | AjitS |
#67 | drupal-864006-67.patch | 3.41 KB | dawehner |
Comments
Comment #1
dawehnerI don't get how this helps. Should this describe what the table does? t('Rearrange the items')
This is confusing, too. You add the scope attribute to links, afaik this is not defined here
Comment #2
dawehnerPerhaps i'm just wrong, could you clarify me?
Comment #3
mgiffordLet me know if this clarifies anything - http://www.webaim.org/techniques/tables/data.php#scope
This is different from a table summary.
Comment #4
mgiffordShould 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"?
Comment #5
dawehnerI think it should be fixed for 6.x, too. As long the codebase keeps similar this makes porting of patches easier
Comment #6
dawehnerCan you explain why the full table need's a scope?
Comment #7
mgiffordThere'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.
Comment #8
dawehnerIn 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
Comment #9
mgiffordAhh, so like this:
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?
Comment #10
dawehnerNo i don't think so :) it's a silent rule.
Comment #11
mgiffordOk, 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.
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
Comment #12
dawehnerThis 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?
Comment #13
mgiffordYa, 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:
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.
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.
Comment #14
dawehnerSure this is possible.
Assign to me
Comment #15
merlinofchaos CreditAttribution: merlinofchaos commentedI'm totally on board with dereine in #14. That's an easy improvement that would go a long ways.
Comment #16
dawehnerHere is a issue to add a summary for tables: #868972: summary attribute for tables
Comment #17
mgiffordthis is exciting! Thanks for setting up the new issue about summary tables and already providing a patch!
Comment #18
dawehnerHere are two new issue. I suggest to use this issue as a collection for other issues.
Comment #19
mgiffordHey 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.
Comment #20
hobo CreditAttribution: hobo commentedadding 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!!
Comment #21
merlinofchaos CreditAttribution: merlinofchaos commentedDue 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."
Comment #22
dawehnerWhat about this?
drupal_attributes can be replaced in d7 with a process function.
Comment #23
mgiffordOk, 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.
Comment #24
merlinofchaos CreditAttribution: merlinofchaos commentedThis patch appears to be against latest CVS of DRUPAL-6--3
Comment #25
merlinofchaos CreditAttribution: merlinofchaos commentedAssigning to bojanz for testing.
Comment #26
Sunflower CreditAttribution: Sunflower commentedIs there a Drupal 7 solution for this?
Comment #28
mgiffordChanging the title to be more specific, bumping this to D7 & upgrading the patch.
Comment #29
colanscope="row"
is there, but what aboutscope="col"
? Is this taken care of elsewhere?Comment #30
mgiffordTry this.
Comment #31
mgiffordI'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.
Comment #32
Jeffdo CreditAttribution: Jeffdo commentedAll 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.
That's a list of global attributes. Scope is just used on TH elements.
It's in the HTML5 spec.
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.'
Comment #33
mgiffordThanks @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.
Comment #34
mgiffordOk, 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:
This would provide some semantic improvements and would output a header row like:
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.
Comment #35
colanNow the "row" is missing.
Comment #36
mgiffordYou 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.
Comment #37
colanGotcha. Let's get some more eyeballs on this.
Comment #38
mgiffordOk, 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()
Comment #39
dawehnerWouldn'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?
Comment #40
mgifford@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).
Comment #41
dcmouyard CreditAttribution: dcmouyard commentedThe 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.
Comment #42
dawehnerI trust mgifford for accessiblity reasons so i don't think #34 is rtbc, see http://drupal.org/node/864006#comment-6463332
Comment #43
mgiffordReally 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.Comment #44
dcmouyard CreditAttribution: dcmouyard commentedThe header/id approach is only needed for more complicated data tables.
Comment #45
mgiffordAgreed. And although I'd like to see Views able to create more complex tables, I don't think we're there yet.
Comment #46
mgifford#38: 864006-view-table-scope-38.patch queued for re-testing.
Comment #47
dawehnerSo 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?
Comment #48
mgiffordIt 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.
Comment #49
mgiffordI applied this to D8.. Feel free to revert back to D7 Views module.
Comment #50
mgiffordoops
Comment #51
dawehnerThis 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.
Comment #52
mgiffordOk, 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.
Comment #53
dawehnerThis looks great beside this one small thing:
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.
Comment #54
mgiffordSpaces 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.
Comment #55
dawehnerThe 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?
Comment #56
mgiffordRealized that I needed to only use drupal_html_id() once.
Comment #57
mgiffordI 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.
Comment #59
mgifford#56: 864006-view-table-scope-56.patch queued for re-testing.
Comment #61
mgifford#54: 864006-view-table-scope-54.patch queued for re-testing.
Comment #62
mgiffordaddressing php notice.
Comment #63
dawehnerIt 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.
Comment #64
mgifford@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.
Comment #65
dawehnerLet's do it properly and add a todo.
Comment #67
dawehnerLet's do it different.
Comment #69
mgifford#67: drupal-864006-67.patch queued for re-testing.
Comment #71
dawehnerWell, even it is a reroll, it is probably more like a full rewrite as now twig is in core.
Comment #72
AjitSRe-rolling the patch from #67
Comment #73
mgiffordI'm going to mark this RTBC. Looks good to me!
Comment #74
YesCT CreditAttribution: YesCT commentedThis issue was RTBC and passing tests on July 1, the beginning of API freeze.
Comment #75
alexpottIs there a follow up to address these todos?
Also the issue summary could do with an update.
Comment #76
mgifford@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.
Comment #77
dawehnerSee #2036369: Cleanup views.theme.inc
Comment #78
mgifford#72: drupal-864006-72.patch queued for re-testing.
Comment #79
mgiffordI believe that @alexpott's concerns have been addressed now so moving this back to RTBC.
Comment #80
alexpottNeeds a reroll
Comment #81
mgiffordre-roll...
Comment #82
bowersox CreditAttribution: bowersox commentedLooks like this just needs a re-roll and review to be RTBC again.
Comment #83
mgifford#81: drupal-864006-81.patch queued for re-testing.
Comment #84
dawehnerLet's get it in again.
Comment #85
alexpottCommitted 41f6e35 and pushed to 8.x. Thanks!
Comment #86.0
(not verified) CreditAttribution: commentedUpdated issue summary.