Add <caption> support to theme_table()

Posted by Crell on September 5, 2005 at 7:12am

Issue Summary

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.

Attachment	Size	Status	Test result	Operations
theme_table_0.patch	1001 bytes	Ignored: Check issue status.	None	None

Comments

#1

Posted by Kobus on September 5, 2005 at 7:19am

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

#2

Posted by Kobus on September 5, 2005 at 7:20am

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

#3

Posted by Crell on September 5, 2005 at 7:45am

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.

#4

Posted by Bèr Kessels on September 5, 2005 at 8:20am

+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.

#5

Posted by Kobus on September 5, 2005 at 8:22am

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

#6

Posted by Bèr Kessels on September 5, 2005 at 10:05am

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".

#7

Posted by Robrecht Jacques on September 5, 2005 at 11:49am

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.

#8

Posted by Kobus on September 5, 2005 at 11:56am

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

#9

Posted by Crell on September 5, 2005 at 8:05pm

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?

#10

Posted by hunmonk on September 6, 2005 at 2:46am

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.