Well, if I made this patch properly :-), this patch adds a fourth parameter to the theme_table() function, $caption. If set, it adds a <caption>$caption</caption> line to the table. If it is not set, this patch has no effect. All styling is left up to CSS to figure out.

It seems to me that $caption would make more sense as the 3rd parameter, before $attributes, but that would break backward compatibility so I just tacked it on at the end. I'll leave the final order of parameters up to someone else to figure out.

CommentFileSizeAuthor
theme_table_0.patch1001 bytesCrell

Comments

Kobus’s picture

Kobus commented

Hi,

I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the or

tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the tag is IBM's Homepage Reader.

Regards,

Kobus

Kobus’s picture

Kobus commented

Hi,

I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the <summary> or <caption> tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the <summary> tag is IBM's Homepage Reader.

Regards,

Kobus

Crell’s picture

Crell commented

Well, summary is an attribute of <table>, not an element in itself. You can already set that with the $attributes parameter. I don't know if the Powers That Be would want another parameter just for that attribute, although that might increase its usage. I just needed the caption because I have two tables on one admin page, and not having some way to tell them apart was not good.

Bèr Kessels’s picture

Bèr Kessels commented

+1 from me too. This is a huge accessability improvement, yet a very small patch.

We should follow this patch up, with a list of theme('table') calls that should use this caption.

Kobus’s picture

Kobus commented

Yes, indeed, you are correct. Pardon my oversight that it is already a attribute. I evaluated homepage reader about 2 months ago, and merely recalled summary. Sorry!

Kobus

Bèr Kessels’s picture

Bèr Kessels commented

I must follow Kobus. Excuse my ignorance in this too. This should indeed be set with attributes, which already is perfectly possible. Unless someone comes with new arguments, I suggest we close this issue as "won't fix".

Robrecht Jacques’s picture

Robrecht Jacques commented
Status: Needs review » Needs work

No Bèr; "summary" is an attribute of "table" and is already supported (although nowhere used) by the "$attributes" argument. "caption" is NO attribute of "table" but an element by itself, eg it can contain HTML code (which attributes can't contain).

So: +1 for this, but the core module theme('table') uses should be checked to see if we can add it somewhere. So: code needs work: go through the uses in core and add a caption where it would be usefull.

Kobus’s picture

Kobus commented

I don't think that <caption> is an attribute, only summary. Therefore, the patch stands as is; my first note about code> was wrong. The patch is therefore necessary, but ONLY for <caption> and not for summary.

Kobus

Crell’s picture

Crell commented

OK, to clarify:

- caption is a sub-element of <table>. Prior to this patch, there is no way to set it at all.
- summary is an attribute of <table>. It can be set via the $attributes parameter now. This patch does not do anything with regards to summary.

I can see about suggesting caption locations in core, but I believe that should be a separate patch from adding the caption support in the first place. I'd hate to see captions not available to module developers when 4.7 freezes in a few days just because we couldn't agree on where there should be captions in core. :-)

Should I make a separate patch and post it in this thread, or as a new thread?

hunmonk’s picture

hunmonk commented

i agree w/ Crell here--use cases in core are a seperate issue from the functionality itself. patch looks good, so let's get this in and make another patch to clean that up.

Robrecht Jacques’s picture

Robrecht Jacques commented
Status: Needs work » Reviewed & tested by the community

OK. Patch is ready for commit then: applies, conforms to coding style, works as advertised, has no side effects.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAd.

Maybe look into http://drupal.org/node/28777 next?

Anonymous’s picture

(not verified) commented
Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)