Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Wanted to update the HTML structure used in this issue. Summary has been depreciated as it was originally expressed in this issue.
<caption>
<strong>Characteristics with positive and negative sides.</strong>
<details>
<summary>Help</summary>
<p>Characteristics are given in the second column, with the
negative side in the left column and the positive side in the right
column.</p>
</details>
</caption>
---- Original ----
For accessibility, it can be useful to add a summary attributes in html table.
Example from http://www.w3.org/TR/html401/struct/tables.html#adef-summary
<TABLE summary="This table charts the number of cups
of coffee consumed by each senator, the type
of coffee (decaf or regular), and whether
taken with sugar.">
<CAPTION>Cups of coffee consumed by each senator</CAPTION>
<TR> ...A header row...
<TR> ...First row of data...
<TR> ...Second row of data...
...the rest of the table...
</TABLE>
Comment | File | Size | Author |
---|---|---|---|
#101 | drupal-843708-101.patch | 9.24 KB | dawehner |
#101 | interdiff.txt | 1.02 KB | dawehner |
#96 | drupal-843708-96.patch | 9.26 KB | dawehner |
#93 | Before_Title_Entered.png | 37.55 KB | mgifford |
#93 | After_Title_Entered.png | 62.04 KB | mgifford |
Comments
Comment #1
samuelsov CreditAttribution: samuelsov commentedHere is a patch that i think solve this problem. It should be easy to do the same for views 6.x-3.x
Please review and comment.
Thanks !
Comment #2
samuelsov CreditAttribution: samuelsov commentedOups, here is a good version...
Comment #3
dawehnerWe should check plain this value.
Comment #4
mgiffordtagging
Comment #5
Everett Zufelt CreditAttribution: Everett Zufelt commented+1 for the concept, haven't reviewed the patch
Not sure about inline help with views, but if it is available it would be useful to provide an explanation about what a summary attribute is used for.
Comment #6
dawehnerI hope this helps to understand summary:
A quote form http://www.w3.org/TR/html401/struct/tables.html
Comment #7
Everett Zufelt CreditAttribution: Everett Zufelt commented@dereine
I don't think that description is sufficiently clear to let developers know when to use a summary and not. I also doesn't think that it is very clear about what type of content to include in the summary.
Comment #8
merlinofchaos CreditAttribution: merlinofchaos commentedI think Everett means we should provide this explanation in the #description of the summary attribute:
So a shorter #title and a longer #description.
That's a lot of open and close PHP tags on one line. How about this instead:
Comment #9
merlinofchaos CreditAttribution: merlinofchaos commentedComment #10
mgiffordI'd agree with that modification. Thanks for the summary & code.
There is still some confusion on when to use caption & summary and what the difference is. Is it worth while putting in a link to a page to help explain what the summary attribute is supposed to contain in the description?
Comment #11
dawehnerCan you provide a good link/explaination?
Comment #12
mgiffordA good link for more info is http://webaim.org/techniques/tables/data#caption
Captions are essentially titles. Summaries provide brief summaries of complex data. Summaries aren't always required, but can be helpful in providing a snapshot.
Another useful link http://jimthatcher.com/webcourse9.htm
Comment #13
Everett Zufelt CreditAttribution: Everett Zufelt commentedSetting to minor. Would it be nice for developers to be able to add a summary to a table, yes. Is it rare that this would be useful, yes. Are there other parts of Views that need more accessibility love, yes.
Comment #14
mgiffordEverett, please open up bugs you find with Views. It's not core, but it's so very close to core for most of us.
Views also has D6 as being the dev version & then things are brought forward to D7. I don't know when this will change.
I'm setting this back to Normal though. Whether or not people using screen readers bother with summary tables is important, but it's not the only factor.
Not being able to effectively set summary tables generated in Views could be used as a reason why Drupal isn't accessible enough. Especially for the output of the page, it is critical for adoption by many government agencies that it can be added. See Canada's CLF:
http://www.tbs-sct.gc.ca/clf2-nsi2/index-eng.asp
Comment #15
gregglesThis patch didn't apply since #868972: summary attribute for tables went in.
I re-rolled it with an attempt to improve the titles/descriptions for both summaries and captions.
Caption element:
Title: Short description of table
Description: This value will be displayed as the caption for the table. Set this for better accessiblity of all tables.
Summary element:
Title: Table summary:
Description: This value will be displayed as table-summary attribute in the html. Use this to give a summary of complex tables.
I think this gives better guidance for site builders on when to use each one. I put the Caption input box above the Summary box since it seems Caption should be used more often.
I added this to both table and grid plugins/tpl.phps.
Comment #16
gregglesApplies to latest 6.x-2.x, btw.
Comment #17
gregglesSorry for yet another comment, but I realized the focus has changed here.
Comment #18
lisarex CreditAttribution: lisarex commentedJust a quick review of the text/labeling
Ideally we'd be able to provide example summary and caption, maybe a link to one of the established sites, but it's not crucial.
Comment #19
mgiffordThis will be very exciting to have this in Views! @greggles thanks for the patch!
Comment #20
gregglesI think that's somewhat incorrect. The default HTML styles show the caption tag contents to sighted users.
Comment #21
lisarex CreditAttribution: lisarex commented@greggles, oops, you're right.
How about "Include a caption to provide an overview of the table contents."
Comment #22
lisarex CreditAttribution: lisarex commentedRather, "Include a caption for better accessibility of your table."
Comment #23
gregglesRe-rolled for lisa's suggested text.
Comment #24
dawehnerSadly this patch doesn't apply to 6.x-3.x anymore
Some rerole would be definitive helpful here.
Comment #25
mgiffordRe-rolling this patch for Drupal 7.
Comment #26
colanThe previous patch didn't work because the option wasn't being set in the preprocess functions. Try this one instead.
Comment #27
colanComment #28
colanUploading the same file again so that the test bot uses the correct branch this time. (I can't find another way to do this.)
Comment #30
colanComment #31
colanSorry, the previous patches were adding it to the table attributes instead of making it a separate tag.
Comment #32
mgiffordHere's a screenshot & example of the current output:
Now that's just fine for HTML4 code, but we're going to have to adjust this for HTML5 in D8. That being said, this is a good improvement that will fill a useful need.
I'll put in some examples for recent table best practices (all HTML5) and the depreciated summary attribute will not be a problem and the added caption will be a big improvement in the accessibility of Views.
http://www.developerfusion.com/article/136530/making-tables-more-accessi...
https://developer.mozilla.org/en-US/docs/HTML/Element/table
http://www.w3.org/html/wg/wiki/SummaryForTABLE
http://www.pdprogrammeur.com/tables-and-html5-table/
Comment #33
dawehnerJust in case title is not empty and caption is empty, this would probably lead to a notice of an undefined variable, which is indeed a regression because it will happen for all people
Comment #34
mgiffordGood catch. This patch just returns a null result.
Comment #35
dawehnerThere are two places using that code, sorry.
Comment #36
mgiffordThank you for pointing that out. No need to apologize, you're making this patch better. I also realized that I didn't use the proper formatting for the else statements I was adding. Cleaned that up too.
Comment #37
dawehnerCommitted the patch finally to d7, let's first discuss how a proper solution would look like for d8.
Wow this one is hard to decide who gets commit credit, but i choosed mgifford, especially because you made the final push and the port.
Fixed some whitespace errors before the commit.
Comment #38
dawehnerAccording to the first link you would probably need both summary and details for the caption. Do you agree with that...
These two should be hidden for the normal user, does core already provide some css for that?
Comment #39
mgiffordThanks @dawehner - was definitely a community effort & I'll get @colan a beer next time I see him. :)
Definitely own @greggles a beer for tonnes of other stuff, but I'm less likely to run into him around town.
Comment #40
xjmComment #41
mgifford#36: add_captions_to_views_tables_and_grids-843708-36.patch queued for re-testing.
Comment #42
mgiffordtagging.
Comment #44
mgiffordRe-rolling here.
Comment #45
mgiffordTalking to Bojhan he suggested the "patch probably needs a style in Bartik as followup" and I expect he's right.
We definitely need a screenshot or two. Very likely styling is going to be required.
Comment #46
mgiffordupdating patch.
Comment #48
mgiffordAnnoying.. Missed a } in copy/paste.
Comment #50
mgiffordArg, rushing around too many issues today. Missed another } for the class.
Comment #52
mgiffordFound some other issues. It's still not showing up though.
Comment #53
dawehnerDoes it really makes sense to provide captions for grids, because if it does you might want to add those to all of the style plugins?
Wow, a whitespace, didn't had that for a long time
Why should be the header not set?
Comment #54
mgiffordOn captions for grids, all depends on #1903746: Replace the views grid table template with one using divs I think. If it's remains a table, we should be able to add caption & summary elements through Views.
I wish I could say I added that whitespace for nostalgia purposes, but think I'm just tired.
I was getting PHP Notice errors, so added the check. The Caption/Summary aren't being spit out properly, but perhaps when they are this will not be needed.
Comment #55
mgiffordI still don't have the summary element working, but think that was busted in bringing it over to Core.
Not sure how this:
$vars['attributes_array'] = array('summary' => filter_xss_admin($handler->options['summary']));
Is supposed to become available to:
<table <?php print $attributes; ?>>
Comment #56
mgifford#55: add_captions_to_views_tables_and_grids-843708-55.patch queued for re-testing.
Comment #57
mgiffordThis has been marked RTBC several times since it was open in 2010. I'm marking it RTBC Feb 18 as I do think it is an important feature for some types of views tables (which presently includes grids).
Comment #58
mgiffordSorry, Summary's not being displayed. I'm hoping this can be considered a task though rather than a feature.
Comment #59
dawehnerThe only thing I'm not sure about, is it common to show the actual caption.
Are we sure, that it makes sense to have a caption for grids as well? They will be probably converted to a div based approach, but then it seems to be not needed anymore?
Wouldn't !empty($caption) be enough?
Comment #60
mgiffordI was receiving notice errors, which is why I added that in. Would be better to fix it elsewhere though.
The summary didn't work in Views (already in Views) because it was formatted with:
Rather than:
With this change we should be good. @dawehner I nixed the isset() as requested as I can't see how that would show up at this point. Should be a null value or a caption.
Comment #61
dawehnerDo you have an oppinion about grids?
Just wondering whether we really want to have first the summary and then the caption in the UI?
Yeah no reason for isset and !empty as well.
We could simplify that quite a bit by splitting the if for title and if for caption. In general I feel like there should be the title in h3 and the caption, but i'm not sure.
we should maybe open a follow up to add test coverage for that.
Comment #62
mgifford1) Grids - I like the idea of CSS only grids. Would be much more mobile friendly. However, with caption/summary there's no accessibility problem I know of with exposing grids in a table.
2) I'm not sure it matters whether the caption or summary is first. I suppose the caption is more like a heading so might make sense to go first. I've swapped it.
3) Removed isset()
4) If captions are selected but not filled in, then the title can be used in it's place. I think that's a useful default.
5) Maybe, but this seems to be a basic bug that can be fixed here. I'd like to have tests for both caption & settings and maybe put that in a follow-up issue. It just wasn't possible to use a summary element.
Comment #63
mgiffordforgot to attach it.
Comment #65
mgifford#63: add_captions_to_views_tables_and_grids-843708-62.patch queued for re-testing.
Comment #66
mgiffordJust trying a re-roll as this didn't make sense to me.
Comment #67
Oaryx CreditAttribution: Oaryx commentedWorks with tables in views, however not with grids. Summary attribute does not show up when display is set to grid.
Comment #68
mgiffordThis is re-rolled with the grids and should be working. It was broken previously with the summary in the table too. I fixed it there, but didn't check to see if it was also broken with the Grid.
Comment #69
mgifford#68: add_captions_to_views_tables_and_grids-843708-68.patch queued for re-testing.
Comment #70
mgifford#68: add_captions_to_views_tables_and_grids-843708-68.patch queued for re-testing.
Comment #71
mgiffordInterdiff between my two patches. Should include changes for grid as specified.
Comment #72
falcon03 CreditAttribution: falcon03 commentedPatch works great. However I cannot mark it RTBC. This is my first code review, I hope I did it properly. :-)
In the table summary description:
maybe "table attribute" should be "table summary" or "table summary attribute". From a usability point of view I would prefer the first option.
>
+
?>
why do we need this?
I am not sure about this, but maybe we don't need this comment (wherever it is used).
If the table summary has a test somewhere in Views, this patch misses also test coverage.
Comment #73
falcon03 CreditAttribution: falcon03 commentedRestoring proper issue values. Not sure what happened, but I didn't change them, except for status :-)
Comment #74
mgiffordI changed the text to "table summary"
I changed this because I must have hit a PHP notice and decided to fix it. I can remove it from this patch for clarity.
The comment just follows in the example of the caption below it so I think it's fine.
options['caption'])) ? filter_xss_admin($handler->options['caption']) : '';
Good review
Comment #75
mgiffordChanging the status on the bot.
Comment #76
dawehnerI still think this can't happen, and if it does, maybe we need more test coverage.
Please don't remove the newline at the end :)
Comment #77
mgiffordSo if there are no headers,
count($vars['header'])
would return 0 & therefore that if statement would return false. Right? Is there a better way to check that $vars['header'] contains at least one header?Totally fine with more test coverage, but I'm not the one to write it.
Sorry about removing the newline..
Comment #78
mgiffordAdding back in the line at the end of the file.
Comment #79
mgifford#78: add_captions_to_views_tables_and_grids-843708-78.patch queued for re-testing.
Comment #80
dawehnerI still think we should describe better when to use the summary and when to use the caption. If you have no clue about accessible tables
you won't be able to find that out.
Comment #81
mgiffordFrom http://webaim.org/blog/10-easy-accessibility-tips/ - "Most data tables have short descriptive text immediately before the data table that describes that table."
I agree that summary/caption is confusing for folks. That's why I tried to clarify with this text:
Caption:
+ '#description' => t('A title which is semantically associated to your table for increased accessibility.'),
Summary:
+ '#description' => t('The table table summary should be used to increase accessibility by providing a summary of complex tables to describe how they are laid out structurally.'),
Maybe the Caption could be simply:
"A short descriptive title for a table that describes the table and improves accessibility"
And the Summary:
"For complex tables consider providing a summary of how it is laid out structurally to improve accessibility"
The description text is pretty easy to change though. Up until the release.
Comment #82
dawehnerWell I don't see why we should not start with a proper help text from the beginning.
I asked Bojhan for help.
Comment #83
Bojhan CreditAttribution: Bojhan commentedCan this get some screens or something, I have no clue where to look :)
Comment #84
mgiffordI created a new View (test table) that had a page with a table layout:
admin/structure/views/view/table_test
Then went to Format: Settings and this is the popup of the latest patch:
Comment #85
mgiffordMaybe this is clearer.
Caption:
A title which is logically associated to the table to improve accessibility.
ie. The List of Registered Users
Summary:
a description of the structure for complex tables to describe how they are organized.
ie. This table charts the registered users of the site by username, email address, occupation and favorite hockey team.
They are sometimes they overlap, but not always. The summary is optional.
Comment #86
mgiffordOk, Just adding to the confusion on this issue. Apparently SUMMARY is obsolete in HTML5.
HTML5 Candidate Recommendation - Section 4.9.1.1 Techniques for describing tables
Which leaves us in a situation where Core has legacy code already in it (the SUMMARY element), but CAPTION is still supported, but isn't in Core.
So this issue is still valid, but we will probably need to remove SUMMARY.
Comment #87
mgiffordI am a bit sleep deprived (and DrupalCon hasn't even started yet).
But figured it would be useful to post this patch to help move this ahead.
Need to be able to produce output like:
Comment #89
mgiffordI'm going to have to figure out why my local install doesn't work on the train to Portland tomorrow...
Comment #90
mgiffordHere's what this patch looks like with expanded details (default is attached, but not embedded below):
This is what the details look like when added to the captions:
I don't know why the details are outside of the table.
Comment #91
dawehnerI'm sorry I had opened the tab quite long without reloading it.
Personally I don't think we should make this for grids, because grids will get responsive based upon DIVs, so we probably don't need this functionality anymore. Wrote the patch so it actually works as expected and added test coverage.
Comment #92
mgiffordAgreed about moving grids to div's. Definitely a better way to go. I'm just disappointed that we've made so little progress in this area since January -> #1903746: Replace the views grid table template with one using divs
I've attached an interdiff. You've added testing from the looks of it. That's great!
Good call adding $caption_needed too, but on the resulting view I don't see either the caption or the details I added.
You making it to DrupalCon? Would be good to push this through.
Comment #93
mgiffordOh ya, I grabbed screen shots. I think the UI for this is good. We're probably going to need to add alerts to it, but I do think this is the way to go.
When you expand the detail element you see just the title field:
After the title is entered the description appears:
Comment #94
dawehnerOpened an issue to connection states js and announce js: #2000324: Let state.js uses announce.js
Comment #95
mgifford@dawehner - great meeting with you briefly. We're pretty close to marking this RTBC. If we use state.js to announce the change in the UI via Drupal.announce, then I think we're just left with my note in #92 "I don't see either the caption or the details I added."
Can you confirm that with the latest patch that the Caption & Details are now added to the table when it is saved? I didn't see the results when I did my testing.
Comment #96
dawehnerRerolled the patch as it is.
Comment #97
dawehnerManual testing let the summary appear.
Comment #99
dawehner#96: drupal-843708-96.patch queued for re-testing.
Comment #101
dawehnerI totally failed on the template.
Comment #103
mgifford#101: drupal-843708-101.patch queued for re-testing.
Comment #104
mgiffordThis looks good to me. Tested it locally and the front-end/back-end looked as expected. Will be great to have this in Core!
Comment #105
alexpottCommitted 2a17837 and pushed to 8.x. Thanks!
Comment #106.0
mgiffordupdate of description of summary.
Comment #108
mgiffordJust wanting to add that there is now an inconsistency in that CKEditor still uses the old approach to using caption/summary as per:
http://dev.ckeditor.com/ticket/12238