I took some examples from TypePad (http://www.typepad.com/news/screenshots/lists-links.html) and Drupal 4.2 (http://drupal.org/node/view/1912) and created a patch to improve the look of Drupal standard tables.

Changes are marked in attached screenshot.

The patch will follow in next followup.

CommentFileSizeAuthor
#26 011.kika.prettytablefix.png31.12 KBkika
#25 011.kika.misc.drupal.css.patch521 byteskika
#22 highlight_active_column_in_tables_4.patch6.6 KBmoshe weitzman
#20 010.kika.commentsbug.png16.84 KBkika
#19 010.kika.prettytables.png27.02 KBkika
#18 010.kika.pushbutton.xtemplate.css.patch366 byteskika
#17 010.kika.default.xtemplate.css.patch406 byteskika
#16 010.kika.misc.drupal.css.patch647 byteskika
#14 highlight_active_column_in_tables_3.patch4.84 KBmoshe weitzman
#13 highlight_active_column_in_tables_2.patch4.11 KBmoshe weitzman
#11 highlight_active_column_in_tables_1.patch4.11 KBmoshe weitzman
#10 highlight_active_column_in_tables_0.patch4.08 KBmoshe weitzman
#8 highlight_active_column_in_tables.patch4.12 KBmoshe weitzman
#4 005.kika.prettytables.tablesort.png23.04 KBkika
#1 005.kika.drupal.css.prettytables.patch717 byteskika
005.kika.prettytables.png14.85 KBkika
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kika’s picture

kika CreditAttribution: kika commented
FileSize
005.kika.drupal.css.prettytables.patch717 bytes

Here's the patch

Apply against 4-4 version of misc/drupal.css

(I had to mark this issue as CVS one since 4.4 is not the option yet in project.module)

jhriggs’s picture

jhriggs CreditAttribution: jhriggs commented

I like not wrapping the asc/desc control alone. It should be tied to the last word in the header. I don't like not wrapping the header at all, though. When the header is sufficiently long, and the content in the column is sufficiently short, I would much rather have it wrap...especially if I am trying to fit a lot of columns in a table. I am undecided about always forcing tables to be 100% width. I can think of cases where I may not want that. Finally, I am more or less indifferent with regards to the cell borders. They don't bother me, and actually help make distinctions between units of data (cells).

Couldn't this all just be handled in the theme or CSS, though? Why patch core? The only thing I see that may be worth adding to core is not wrapping the asc/desc control by itself. That may be a small usability issue. Everything else regarding the look, though, seems to be subjective.

kika’s picture

kika CreditAttribution: kika commented

> When the header is sufficiently long, and the content in
> the column is sufficiently short, I would much rather have it
> wrap...especially if I am trying to fit a lot of columns in a table.

The solution is to patch tablesort.inc to include a special class to the cell where the asc/desc image currenly is. Even better, add a special class to the entire column - to allow "sorted by" column to stand out better (you can see such a behaviour in WinXP and OSX)

> Couldn't this all just be handled in the theme or CSS, though? Why
> patch core?

since it already being done there, i just tidied it up.

I do not care where the actual styling is taking place, in core or in xtemplate (a barebone example theme with übercool table is looking pretty weird)

kika’s picture

kika CreditAttribution: kika commented
FileSize
005.kika.prettytables.tablesort.png23.04 KB

> Even better, add a special class to the entire
> asc/desc column -
> to allow "sorted by" column to stand out better
> (you can see such a behaviour in WinXP and OSX)

A mockup that expresses this behaviour (emphasis
of asc/desc column) is included

Brian@brianpuccio.net’s picture

Brian@brianpuccio.net CreditAttribution: Brian@brianpuccio.net commented

My (personal, scientifically unproven) opinion is that I like the second one.

Dries’s picture

Dries CreditAttribution: Dries commented

I committed this patch to HEAD.

The 'width: 100%;' makes wide tables look better, but makes narrow tables look weird (eg. admin/user/role). We need to fine-tune (or revert) that, which is why I set the status of this report to 'active'.

The proposed improvements look good. I like version #1 best as it is easier for the eye (less intrusive). It makes the tables look really nice.

Dries’s picture

Dries CreditAttribution: Dries commented

For the pretty tables to work, we'd have to extend the emitted HTML code. Suggestions for how this HTML code should look like so we can do both version #1 and version #2 of the proposed tables?

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Assigned: Unassigned » moshe weitzman
FileSize
highlight_active_column_in_tables.patch4.12 KB

attached patch adds a class attribute called 'cell-highlight' to all cells within the actively sorted column. This class is not new - its the one we currently use in the table header. any themes currently defining this css class will automatically enjoy full column highlighting (e.g. phptemplate). Other themes may want to take advantage as well.

Dries’s picture

Dries CreditAttribution: Dries commented
  1. Why do we do is_array($header) in if (is_array($header) && $i === $ts['index']) { when $header does not appear to be used further down?
  2. Is cell-highlight a good name? As a theme designer I wouldn't realize it is used to denote a cell that belongs to the active column. Maybe <tr class="active"> is more semantic (<tr>, not <td>)?
moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
FileSize
highlight_active_column_in_tables_0.patch4.08 KB

new patch which
- more secure tablesort SQL
- changed class name to 'active'. themes should use these css classes

th .active
td .active

- tidied up the is_array($header) stuff a bit. It is still present though because we want to skip cell highlighting for tables which don't have headers.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
FileSize
highlight_active_column_in_tables_1.patch4.11 KB

new patch. i had missed one instance of class='cell-highlight'

Dries’s picture

Dries CreditAttribution: Dries commented

This patch is a _lot_ better but seems to overwrite existing 'td classes'. Take a look at the watchdog table.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
FileSize
highlight_active_column_in_tables_2.patch4.11 KB

attached patch adds onto existing TD classes instead of overwriting them.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
FileSize
highlight_active_column_in_tables_3.patch4.84 KB

even better this time. tables without sortable headers no longer get the 'active' attribute ...

DRUPAL.CSS
added italics to sorted column. may be further defined in theme css. use the css classes 'th.active' and 'td.active'. also prevented image in active table header from wrapping.

Dries’s picture

Dries CreditAttribution: Dries commented

Committed to HEAD.

Kika, Megagrunt and friends: now all preparations have been made to support 'pretty tables', we expect theme patches to hit the patch queue. Marking this active, not fixed.

kika’s picture

kika CreditAttribution: kika commented
FileSize
010.kika.misc.drupal.css.patch647 bytes

Here come the patches...

kika’s picture

kika CreditAttribution: kika commented
FileSize
010.kika.default.xtemplate.css.patch406 bytes
kika’s picture

kika CreditAttribution: kika commented
FileSize
010.kika.pushbutton.xtemplate.css.patch366 bytes
kika’s picture

kika CreditAttribution: kika commented
FileSize
010.kika.prettytables.png27.02 KB

An added screeshot to check the 010.* patches impact

kika’s picture

kika CreditAttribution: kika commented
FileSize
010.kika.commentsbug.png16.84 KB

While creating the 010.* patches, I discovered the bug on "Home » administer
Comments" page: the 'delete comment' column is marked as "active", not the 'time' column, as it should be. An attached screenshot illustrates this odd behaviour

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

attached patch:
- fixes comment.module bug highlighted by kika in previous comment
- move a little tablesort logic from theme_table() to tablesort_x functions.
- fix bug when multiple sortable tables appear on same page

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
FileSize
highlight_active_column_in_tables_4.patch6.6 KB

this time with a patch actually attached

Dries’s picture

Dries CreditAttribution: Dries commented

Committed to HEAD. Thanks Kristjan and Moshe.

Dries’s picture

Dries CreditAttribution: Dries commented
kika’s picture

kika CreditAttribution: kika commented
FileSize
011.kika.misc.drupal.css.patch521 bytes

A small patch what removes the offending "table {witdh: 100%}", breaking the Chameleon theme and also enables the table bottom-borders on IE6.

kika’s picture

kika CreditAttribution: kika commented
FileSize
011.kika.prettytablefix.png31.12 KB

Adding a screenshot illustrating the 011.* patch behaviour

Dries’s picture

Dries CreditAttribution: Dries commented

Committed to HEAD.

Anonymous’s picture

(not verified) CreditAttribution: commented
KeithDaniels’s picture

KeithDaniels CreditAttribution: KeithDaniels commented

Possible interactions with class="active"?

I am having problems getting a:active{} to work with Drupal in any browser though I can get it to work in other sites. Is it possible that your use of class="active" is interfering with this proper functioning of this CSS code?

Also

When I try to use something like .content .active {color:#ff0000} to control an elements appearance the settings in .content .active a:visited{} override those settings. I know you are supposed to use "a.content" but that syntax does not work when I try to use it.

Is it possible that your use of the class "active" is conflicting with a:active and causing this?

I am going to post this else where since this thread seems to be over...

Keith