Header region missing

our default theme can't ship without supporting our default regions.

The quick fix is actually a reasonable idea for the place of the header, but the blocks in the header will be totally mangled.
I think there should be a lot more prepared styles to handle the layout for blocks in the header.

The fact that templates can't define their own regions (or features) for that matter is a flaw in the theme system. There is simply no room in the design for header blocks, nor any appropriate styling for them.

Note: the ability to use "themename_regions()" is unacceptable as it breaks the principle that templates should be freely copyable from one directory to another.

Won'tfix as far as I'm concerned. The suggested patch is broken as it duplicates the element with id "mission".

It's not so bad. I'd keep the header blocks region and ditch the mission. If they can't tell what my mission is...

[18:02] <chx> noone has any good idea about that header region problem?
[18:03] <UnConeD_> chx: abolish the header region :P
[18:03] <UnConeD_> i've never seen a theme that actaully uses it in a usable fashion
[18:03] <chx> UnConeD_: fine with me
[18:03] <m3avrck_> thumbs up from me
[18:04] * RobRoy has used the header region
[18:05] <RobRoy> but it took lots of CSS to make it look right, I guess by default we could scrap it
[18:05] <RobRoy> since it's rarely used and since it requires lots of work to make it look nice, only people who know how to add a new region in template.php will do so

Hmm... abolish from phptemplate.engine because Garland is not designed for this?
Isn't there a place for a less drastic abolishment?

I too, have yet to see a site take advantage of this. I'd be OK with removing it.

Yes, but you know what the problem is? It is true that hardly any theme has been using it but on the other hand we don't have many killer themes currently. The question is who is going to develop those missing killer themes. RobRoy's last argument was a bit hasty in that he assumes that php programmers will do it.

i don't like where this is going. removing this region is just causing hassles for uprgaders with little benefit. "i haven't seen many sites use this" isn't a very compelling argument. none of us sees the backend of many drupal sites. consultants probably see the most, and i have seen several use the header region. perhaps big/costly sites are more likely to take the time to use regions, instead of just hacking page.tpl.php as desired.

adds a header region above content and uses region-header as the name of the id, since header is already used

One more patch, just to be thorough.
Garland and Minelli use the regions which they... use.

A benefit is that Garland is the only core theme using a template.php and a demonstration of this feature wouldn't harm.

Definitely -1 for removing the header region from core altogether, especially this late in the release cycle. That is not a fix.

+1 for either moshe's patch in #11 (preferred; needs the indenting fixed though) or CogRusty's patch in #12.

The fact that templates can't define their own regions (or features) for that matter is a flaw in the theme system.

The Drupal Way is then to fix this flaw. We already have the necessary mechanism in place we just need to use it: _phptemplate_variables to the rescue.

With this we still have a default phptemplate_regions for themes that do not have a template.php . And, by using _phptemplate_variable for Garland's template.php , Minelli and other derivatives is fixed too. The code change is minimal, I just factored out the variables call to its own function with Doxygen of course.

I upped the wrong file.

I don't really like the way this works. It introduces yet another mechanism to overwrite defaults. Better to standardize on one system, rather than to invent a dozen parallel, yet slightly different systems. That is, use (regular) hooks. Either the theme implements a regions hook, or the default regions hook is used. This theme stuff is getting crufty.

(Moshe: I've seen a fair amount of Drupal sites in my life despite the fact that I'm not a consultant. The theory that only consultants gets exposed to certain functionality is ... weird. The people that commented on this issue spent time with more than one Drupal site and have more experience theming Drupal sites than you and me together.)

The people that commented on this issue spent time with more than one Drupal site and have more experience theming Drupal sites than you and me together.

This is true; however, the change that most of said people supported (removing the header region from core altogether) doesn't affect advanced themers; advanced themers will probably override this anyway, and have several regions in their themes: header-nav-left, header-nav-right, or whatever.

This change would affect Mr. Joe Blow who just runs a pretty straight-forward 4.7 site with one of either the core or contributed themes with possibly some minor colour changes or whatever. He throws a block with some banner code in the "header" region. It might not look that great, but it's functional and does what he needs it to do.

But now, when he upgrades to 5.0, the banner is gone. He goes to admin >> blocks and the "header" region is gone. He's now officially stuck without diving into template.php in order to put it back. This is not a solution.

(It's worth pointing out that most of us have probably never seen or worked sites like this, because the people who build sites like this don't have the money to hire us to look at it. ;) They're little hobby sites or non-profit sites, etc.)

I don't know for sure if chx's method is "the" solution either, but it has the following advantages:

• Doesn't strip out the "header" region from all of core just because it's difficult to theme in Garland.
• Doesn't break any existing themes implementing a theme_name_regions() function.
• Provides a mechanism that can apply to any PHPTemplate theme regardless of what it's called by way of the _phptemplate_variables function. So newbie themers can just copy/paste the Garland theme into "Garland2" and start hacking it with no loss of functionality and without having to go into template.php and rename "garland_regions" to "garland2_regions".

It introduces yet another mechanism to overwrite defaults

I do not do anything like that. _phptemplate_variables was there, I reused it actually.

My 2 cents:
The header and footer regions have always been rather broken in all themes I remember, and I suspect that header region is often used to place a block before the content, since there is no default "before content".

So if header is so difficult to theme, I propose to remove that region from the theme and put a "before content" region instead (or "top of the page", etc), like moshe's patch at #11.

The reason nobody uses the header for blocks is that blocks there look out of place: imagine a login block in the header, it won't fit, same goes for almost any other block. The only blocks that will look good there are custom blocks which span the whole width of the region and not even have a title.

So the only reason to add a header block there is backwards compatibility - which is a good reason. But please don't do it without making the blocks looking at least reasonable in that region.

So the only reason to add a header block there is backwards compatibility - which is a good reason. But please don't do it without making the blocks looking at least reasonable in that region.

I disagree. I would like backwards compatibility even if the blocks won't look very nice. At least then the site admin can simply change the location of the block in the block settings. Less confusion, less risk of important block data being overlooked.

+1 to #11 for its light touch.

fgm: it works, but place a login block there and you'll see

To be honoust, I think we should get rid of the whole default regions and let every theme itself define the regions it wants to use..
Themes are designed for with some purposes and functionality in mind, and so it becaomes quite hard to define a default set of regions to rely on..

Stefan

@ontwerpwerk: then don't put a login block there. :P I usually use it for ad banners and it looks fine. See screenshot. An "announcement" message is another suitable use case, as fgm points out.

@stefan: -1. That's forcing a PHP requirement on people to create themes. At the very least, they would need to change the name of an existing theme_name_regions() function in order to see any regions at all.

Re-roll of Moshe's patch in #11 with indentation fixed. Marking RTBC because:

- Removing the header region from all of core is not a fix.
- CogRusty's patch would make Garland less hackable.
- chx's patch changes APIs.
- fgm's patch adds some borders and stuff which I don't think fit every use case.
- It looks as good as any other theme's header region (see screenshot in previous post).

I agree with webchick above. Not all blocks will look good, but for backwards compatibility and banners this is a good solution.

fgm: That would work, except the other core themes always have $header before $mission, so for consistency we should keep it that way.

fgm: you are right in your situation, but this theme's use will be primarily for new users and users who were fine with bluemarine before. The people who want different behaviour are encouraged to make their own theme and modifications.

That's why I believe the patch by webchick qualifies as a 'good enough' solution and should be committed. (and lets not discuss the color of this particular bikeshed any longer...)

#27 is fine with me.

Placing blocks where the mission is, is atrocious. It breaks the visual proximity of the breadcrumb, page titles and local tasks.

@Steven. Most everyone agrees - thats why #27 is the leading candidate. I think you reviewed #29.

I was talking about the screenshot that webchick submitted.

I don't see a problem with removing the header region by default. People who used it before can still add the region to their custom theme in 5.0. Just because this technique is not appropriate for a core theme, doesn't mean it cannot be used in contrib or on your own site.

But, to actually stop the bickering, there is an alternative which I've attached. It adds the header region to the top of the page and includes some trivial CSS to make blocks look at least not completely horrible when placed there. But I'd still prefer just removing the region.

Arbitrary changes to Garland like proposed earlier are not acceptable. If we want maintain a high quality standard in Drupal, all patches that alter the way it looks and acts must include the appropriate CSS to keep it pretty. We have been too lax about this in the past (e.g. when Form API screwed up the spacing between checkboxes/radios in groups) and this must change.

For 6.0, we should add .info files to themes, where info like regions, features and even color.module info can be defined. The set of regions that is now defined in phptemplate.engine should be moved to theme.inc and become the global default for all themes. By not relying on PHP code, we can avoid function names clashing when querying the info of a theme other than the current one.

We can implement garland_regions() in its template.php.

Putting the header above the site title may or may not be the best place. Depends on what people would like to use it for.

Using garland_regions() in template.php means the theme cannot be copied freely without hacking the code. This has been mentioned several times in this thread already.

I'm okay with this patch. Any other reviews?

What could be better than supporting header regions in Garland -- implemented by the very author of Garland himself?

True, and we can still hack "if $mission" for our atrocious banners.

Haven't tested it, but the code^^^CSS looks sane.

For the visually inclined, the banner now looks like this. Kind of seems like maybe the padding on the top and bottom should be consistent, but I'm not going to fight it much, as at least it's there now, and I could always tweak the CSS myself. :)

Looks better than the previous screenshot, but I agree with the padding issue. It's cramped at the top.

k, I changed:

  margin-top: 0;

to:

  margin-top: 0.5em;

in:

#header-region * {

And now it looks like this, which to my untrained eye looks right. Patch coming up in a second. Will have to mark back to "needs review" as it needs Steven's approval.

Patch.

Webchick:
Setting a top margin on every single element inside the block, just to fix the banner, seems stupid. The banner's own styling should already provide an equal spacing at the top and bottom.

Other blocks, such as User login, or Who's Online look fine in the header region without your addition.

Ok, set back to RTBC for #37.

Patch in #37 no longer applies, I think cos of the CSS url() cleanup patch.

Re-roll.

Added the 0.5em top margin to paragraphs and image tags. That should help space things out.

Committed to HEAD.

Looks great now, thanks!!

What's the consensus about hiding region DIV tags when there's nothing in them? The current behaviour leaves a (ugly) empty space at the top of the page if there are no blocks in the header. How about:

<!-- Layout -->
<?php if ($header != "") { ?>
  <div id="header-region" class="clear-block"><?php print $header; ?></div>
<?php } ?>

The space at the top is intentional, and was there before this patch as well.