Download & Extend

Unique CSS IDs for forums, containers & forum table

Project:Drupal core
Version:6.x-dev
Component:forum.module
Category:feature request
Priority:normal
Assigned:dvessel
Status:closed (fixed)

Issue Summary

I propose to change line 891 in function theme_forum_list

<?php
$rows
[] = array(array('data' => $description, 'class' => 'container', 'colspan' => '4'));
?>

to

<?php
$rows
[] = array(array('data' => $description, 'class' => 'container container-' . $forum->tid, 'colspan' => '4'));
?>

This will let themers to theme each different forum container differently.

Note that this is possible overriding default theme function and writing a phpthemplate theme.
But a solution of this type is similar to what is already available for blocks. And it is smart.

Fabio

Comments

#1

Version:4.6.5» x.y.z

Reassigning this to cvs, since 4.6.x doesn't get new features.

That said, it sounds like a good addition once the feature-freeze for 4.7 ends. (Meaning "someone will be with you shortly" :-) )

#2

Version:x.y.z» 6.x-dev

Or maybe not so shortly. Is this still an issue?

#3

Assigned to:Anonymous» fax8
Status:active» needs work

Yes.. the code is not changed.. and I think this feature is still useful.

The attached patch add the code I described in my first post.

This will make possible to associate different css rules to each forum container
on the forum listing page.

Please note that this patch only make it possible to style forum containers,
not forums on the forum listing page. It could be also useful adding a similar feature
for also forums.

A similar approach could be useful also on the topic listing page.

AttachmentSizeStatusTest resultOperations
forum-container-ccs.patch475 bytesIgnored: Check issue status.NoneNone

#4

Title:make each forum container css themeable» make each forums container, forum table and forum css themeable
Status:needs work» needs review

The attached patch make containers, forum and forum listing tables css themable.

AttachmentSizeStatusTest resultOperations
forum-container-ccs_0.patch1.35 KBIgnored: Check issue status.NoneNone

#5

You should not use classes for things which are known to be unique.

#6

Status:needs review» needs work

#7

Title:make each forums container, forum table and forum css themeable» Unique CSS IDs for forums, containers & forum table
Status:needs work» needs review

This patch makes them IDs.

AttachmentSizeStatusTest resultOperations
D6-f-forum_unique_ids.patch1.59 KBIgnored: Check issue status.NoneNone

#8

Assigned to:fax8» Gurpartap Singh

This one for new forum template files. Doesn't add id to table, as term id for it is unknown. Don't want to dig much into template variables.

AttachmentSizeStatusTest resultOperations
D6-f-forum_unique_ids_0.patch1.43 KBIgnored: Check issue status.NoneNone

#9

Status:needs review» reviewed & tested by the community

+1. I think this should go in. Adding a unique ID to the containers increases the power of the themer for very little cost.

#10

Status:reviewed & tested by the community» needs work

Actually, topic id "$tid" is available in both forum-topic-list.tpl.php and forum-list.tpl.php. Why not apply it to both?

#11

Status:needs work» needs review

Looking at this again, the $tid was clashing with the existing $tid variable. $tid is already supplied and it can be applied to the whole table.

Here it is again with an id added to the table and each *row*. Working off cells is less flexible.

forum-topic-list.tpl.php contains an id of "forum-topic-*" for the table.

forum-topic.tpl.php caintains "forum-*" for the whole table and "forum-list-*" for each table row.

Latest HEAD had some changes since yesterday.

AttachmentSizeStatusTest resultOperations
forum_unique_ids_1.patch2.45 KBIgnored: Check issue status.NoneNone

#12

Priority:minor» normal
Assigned to:Gurpartap Singh» dvessel

Is this good or not? Looks RTBC in my opinion. :)

#13

re-reolled..

AttachmentSizeStatusTest resultOperations
forum_unique_ids_1b.patch2.46 KBIgnored: Check issue status.NoneNone

#14

Status:needs review» reviewed & tested by the community

Great feature

#15

+1

This greatly simplifies the theming of a specific forum.

#16

Status:reviewed & tested by the community» needs work

I'm OK with this patch, but I was wondering what $cid stands for. Do we use $cid because $tid is taken?

Also, I'm not sure the PHPdoc is accessible for non-developers:

<?php
- $tid: Taxonomy term ID for the current forum. Parent to all items within
+ *   the $forums array.
?>

To understand this, you need to understand how the forum module is implemented, and how it leverages the taxonomy system. It think we can make this more accessible for designers by explaining this differently. Let's explain this in "forum terms" rather than "taxonomy terms", so to speak.

Otherwise this patch looks like something that we'll want to commit.

#17

Status:needs work» needs review

Okay Dries, your right. I always miss these things. :-)

AttachmentSizeStatusTest resultOperations
forum_unique_ids_1c.patch3.59 KBIgnored: Check issue status.NoneNone

#18

Patch works as advertised.

Small detail: is it by purpose that "id" is capitalized in .tpl.php files and is lowercase elsewhere? If yes, then this is RTBC.

#19

Yeah, not sure why I did that. here it is removed.

AttachmentSizeStatusTest resultOperations
forum_unique_ids_1d.patch3.44 KBIgnored: Check issue status.NoneNone

#20

Status:needs review» reviewed & tested by the community

RTBC

#21

Status:reviewed & tested by the community» needs work

Did you mean forum_id instead if tid? In the template.

#22

Status:needs work» reviewed & tested by the community

Darnit! I needs glasses.

AttachmentSizeStatusTest resultOperations
forum_unique_ids_1e.patch3.45 KBIgnored: Check issue status.NoneNone

#23

Status:reviewed & tested by the community» needs review

Oops. I missed that.

I'm also unsetting $variables['tid'], to prevent confusion about what that could be.

AttachmentSizeStatusTest resultOperations
forum_unique_ids_1e_0.patch3.51 KBIgnored: Check issue status.NoneNone

#24

Status:needs review» reviewed & tested by the community

That works too. :)

#25

Status:reviewed & tested by the community» fixed

Great, I see Dries only asked for this clarification in the variable name and docs, and otherwise he was fine with the patch, so committed.

#26

Status:fixed» closed (fixed)
nobody click here