[drupal-devel] [feature] improve theme_box to make it better themeable and consistent with blocks

Sun Feb 6 11:37:22 UTC 2005

 Project:      Drupal
 Version:      cvs
 Component:    theme system
 Category:     feature requests
 Priority:     normal
 Assigned to:  Bèr Kessels
 Reported by:  Bèr Kessels
 Updated by:   adrinux
 Status:       patch

Personally I find the 'every element with a class and id' style of
drupal slightly annoying. It seems to me there is great emphasis on
keeping the php code clean and effecient, but when it comes to xhtml a
'kitchen sink' include everything that a theme designer might possibly
need and somewhat bloated approach prevails.
To me it all adds clutter that makes designing drupal themes harder not
easier, especially for newcomers faced with a huge array of classes and
ID's and not knowing quite what to use, or where they come from.
Documenting them helps but not a lot.
That said I'm for anything that lets me over-ride xhtml and add my own
in phptemplate :)
I'm not conviced that enforced ID's on all boxes is a good idea...

adrinux

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

January 8, 2005 - 21:37 : Bèr Kessels

Attachment: http://drupal.org/files/issues/theme_box.patch (1.24 KB)

This patch changes the theme_box function. Currently we lak the ability
to style blocks different, since we cannot add classes or IDS.
I fixed the theme_box to follow the system we use for sideblocks. The
id and classes should be provided by arguments. 
I have only one problem: backwards compatibility. To output valid XHTML
all calls of this function must add a unique ID, I documented how this
must be done, thats not the problem. The problem, is that we do not do
this atm. 
So untill all calls are fixed, I do:

<?php
theme_box($title, $content, $class = NULL, $id = NULL)
?>

and not

<?php
theme_box($title, $content, $class, $id)
?>

which would be correct, since only the later will force  developers to
use and ID, resulting in always validating XHMTML.
I will try to generate a grep list with places where theme box is
called and where it should be fixed. 
Bèr

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

January 8, 2005 - 22:25 : tangent

XHTML does not require a div to have an ID attribute. Why would an ID be
required in this case?

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

January 8, 2005 - 22:40 : Bèr Kessels

XHTML does not require a div to have an ID attribute. Why would an ID be
required in this case?
Because i do not wat to add an if($id) { } or even an $id ? 'id= etc.
That would be loads of ifs that are not needed: its much better to
enforce everyone to add an ID. Also, if we make ids optional, a lot of
lazy developers will not add it, resulting in a lot of brokenish
themes. I perfer to have always an ID, and put tit in the coding
guidelines, then to have only sometimes an id.
And the last reason is, that with the current useability discussions
going on, there are people starting to look at javascripts in their
themes. We should be ahead of that, and simply give all elements an ID.
So that themers can add their javascript to do fancy stuff to parts of
the pages. (like hide and show them using cards and or accordeons)

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

January 8, 2005 - 22:46 : tangent

I see where you're going but I don't think that we should necessarily
require *all* blocks to have an id. If a module developer doesn't want
his block to have an id and prefers to have a set of blocks with the
same or different classes I see nothing wrong with that. Requiring an
ID seems a bit heavy handed.
This is really another issue but how will user definable modules be
handled? I assume the module would just use an incremented ID?

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

January 9, 2005 - 01:10 : chrisada

"if we make ids optional, a lot of lazy developers will not add it
"
As one who theme Drupal quite a lot, I am absolutely with Bèr on this
point.
The deveoper might not need the id to be there, but there will be
peoples who want them. Right now there is just too many things in
Drupal that is impossible to style due to lack of CSS id. (classes we
have plenty)

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

January 12, 2005 - 14:01 : Bèr Kessels

I cannot see the issues with "requiring" an ID. Really: is that so hard
to do? A module might have one, or two, sometimes three blocks. Why
cant you add two letters to your code for a unique ID?
Its all about consistency. I already know a huge lot of places in the
code where the *block theme function iteself* should have been used,
but is not. This is annoying, not for developers, they don't care. But
for both users, who see inconsistent interfaces, and for themers who go
like: Hey: I am sure I had all titles of all pages set to bleu, why is
that one red? --aah. f*c, its an h3 that is not in a block, grrr. 
Same will go for blocks ids. I do not know why, but the general
html-code and interfaces of contributions are plain bad, in drupal.
Core is nice, but a lot of contributions are hacks-upon-hacks, to keep
up with the changing drupal-core versions. We should  enforce
developers to at least keep consistancy.

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

January 25, 2005 - 21:03 : Bèr Kessels

Attachment: http://drupal.org/files/issues/theme_box_0.patch (1.22 KB)

This one has all parameters required.

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

January 25, 2005 - 21:53 : drumm

The html outputting could be cleaned up a little, using single quotes
where possible and not putting variables inside strings.
Also, I think this sohuld be renamed to theme_page_section() and it
should not necessarily actually draw a box. Whether something has a
literal box or not should be up to the themer on a case-by-case basis.
I see this themeable function as omething to break pages up into areas
and not add boxes.
Otherwise, I like where this is going and would like to see the code
changed to call this properly. For now I won't give a whole +1, but it
deserves at least +0.5.

-- 
View: http://drupal.org/node/15332
Edit: http://drupal.org/project/comments/add/15332