[drupal-devel] [feature] Add <caption> support to theme_table()

Tue Sep 6 08:52:51 UTC 2005

Issue status update for 
http://drupal.org/node/30423
Post a follow up: 
http://drupal.org/project/comments/add/30423

 Project:      Drupal
 Version:      cvs
 Component:    theme system
 Category:     feature requests
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  Crell
 Updated by:   Robrecht Jacques
-Status:       patch (code needs work)
+Status:       patch (ready to be committed)

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

Robrecht Jacques

Previous comments:
------------------------------------------------------------------------

Mon, 05 Sep 2005 07:12:18 +0000 : Crell

Attachment: http://drupal.org/files/issues/theme_table_0.patch (1001 bytes)

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.

------------------------------------------------------------------------

Mon, 05 Sep 2005 07:19:10 +0000 : Kobus

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

------------------------------------------------------------------------

Mon, 05 Sep 2005 07:20:23 +0000 : Kobus

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

------------------------------------------------------------------------

Mon, 05 Sep 2005 07:45:29 +0000 : Crell

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.

------------------------------------------------------------------------

Mon, 05 Sep 2005 08:20:54 +0000 : Bèr Kessels

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

------------------------------------------------------------------------

Mon, 05 Sep 2005 08:22:50 +0000 : Kobus

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

------------------------------------------------------------------------

Mon, 05 Sep 2005 10:05:41 +0000 : Bèr Kessels

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

------------------------------------------------------------------------

Mon, 05 Sep 2005 11:49:10 +0000 : Robrecht Jacques

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.

------------------------------------------------------------------------

Mon, 05 Sep 2005 11:56:05 +0000 : Kobus

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

------------------------------------------------------------------------

Mon, 05 Sep 2005 20:05:03 +0000 : Crell

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?

------------------------------------------------------------------------

Tue, 06 Sep 2005 02:46:30 +0000 : thehunmonkgroup

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.