Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Although it is possible to define blocks as appearing in the header area (along with content, footer, left sidebar and right sidebar), when this is done, they appear nowhere and, looking at the code, the region does not actually exist.
This prevents use of this area for emergency messages appearing above content, which is a common use of the header area.
Comment | File | Size | Author |
---|---|---|---|
#53 | Picture 14.png | 43.12 KB | webchick |
#51 | header_1.patch | 2.42 KB | webchick |
#47 | header_0.patch | 2.44 KB | webchick |
#46 | Picture 13.png | 37.23 KB | webchick |
#44 | Picture 12_0.png | 49.47 KB | webchick |
Comments
Comment #1
fgmI've provided a quick fix (ok, call it a hack) : since the appropriate region for the "header" region is the same as the mission, this patch just implements the "header" region as another instance of the mission div.
I *know* it's not as should be, but until a better garland specialist gives the proper fix, this allows users with garland deployed to have a "header" region in the proper place... and I needed it !
Comment #2
moshe weitzman CreditAttribution: moshe weitzman commentedour default theme can't ship without supporting our default regions.
Comment #3
ontwerpwerk CreditAttribution: ontwerpwerk commentedThe 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.
Comment #4
fgmFWIW, you can see how it looks with that fix on http://www.php-gtk.eu
Comment #5
Steven CreditAttribution: Steven commentedThe 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".
Comment #6
cog.rusty CreditAttribution: cog.rusty commentedIt's not so bad. I'd keep the header blocks region and ditch the mission. If they can't tell what my mission is...
Comment #7
chx CreditAttribution: chx commentedComment #8
cog.rusty CreditAttribution: cog.rusty commentedHmm... abolish from phptemplate.engine because Garland is not designed for this?
Isn't there a place for a less drastic abolishment?
Comment #9
Dries CreditAttribution: Dries commentedI too, have yet to see a site take advantage of this. I'd be OK with removing it.
Comment #10
cog.rusty CreditAttribution: cog.rusty commentedYes, 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.
Comment #11
moshe weitzman CreditAttribution: moshe weitzman commentedi 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
Comment #12
cog.rusty CreditAttribution: cog.rusty commentedOne 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.
Comment #13
webchickDefinitely -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.
Comment #14
chx CreditAttribution: chx commentedThe 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.
Comment #15
chx CreditAttribution: chx commentedI upped the wrong file.
Comment #16
Dries CreditAttribution: Dries commentedI 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.)
Comment #17
webchickThis 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:
Comment #18
chx CreditAttribution: chx commentedI do not do anything like that. _phptemplate_variables was there, I reused it actually.
Comment #19
Egon Bianchet CreditAttribution: Egon Bianchet commentedMy 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.
Comment #20
fgm+1 on Moshe's solution, which is minimal and preserves the highest level of compatibility. Attached is an enhanced version with two added styles for #header-region. This is the one currently online on the site I already mentioned.
It more detail, I think Webchick really nailed it. Most of the others in this thread seem to ignore the very simple use case I pointed out when entering this issue: the need for non-specialists to have a trivial-to-use feature allowing a site admin to have some content display on his site for an urgent situation (and remove it with just one click when the emergency subsides), without having to resort to patching a theme.
Of course, any of us can create a theme with that feature, but I suspect not many, myself included, would probably be able to create something as nice-looking as garland).
So, for most sites except the larger ones built by a drupal professional and using stock/contrib themes, using a patched theme just for a basic feature like this means in manys case they will not take advantage of new versions and patches because it would require reimplementing their changes. So I think the other solutions offered, although technically sound, are reducing drupal's overall value for simple sites.
Hence the +1.
Comment #21
ontwerpwerk CreditAttribution: ontwerpwerk commentedThe 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.
Comment #22
fgmOntwerperk, do you mean you find the example I suggest (www.php-gtk.eu) not appropriate ?
Comment #23
simeI 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.
Comment #24
ontwerpwerk CreditAttribution: ontwerpwerk commentedfgm: it works, but place a login block there and you'll see
Comment #25
Stefan Nagtegaal CreditAttribution: Stefan Nagtegaal commentedTo 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
Comment #26
webchick@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.
Comment #27
webchickRe-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).
Comment #28
ontwerpwerk CreditAttribution: ontwerpwerk commentedI agree with webchick above. Not all blocks will look good, but for backwards compatibility and banners this is a good solution.
Comment #29
fgmWebchick: I hadn't thought about the "image" use (banners) you describe so your version is more usable for that case. I still think the "header" should go below the mission, though. It won't affect non-home pages, and mission has some semantic reason to be the topmost content.
IMHO, order should be:
So here is the rerolled version with that order applied.
Comment #30
webchickfgm: That would work, except the other core themes always have $header before $mission, so for consistency we should keep it that way.
Comment #31
ontwerpwerk CreditAttribution: ontwerpwerk commentedfgm: 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...)
Comment #32
chx CreditAttribution: chx commented#27 is fine with me.
Comment #33
fgmFor consistency's sake with pushbutton and bluemarine, #27 is good for me too.
Comment #34
Steven CreditAttribution: Steven commentedPlacing blocks where the mission is, is atrocious. It breaks the visual proximity of the breadcrumb, page titles and local tasks.
Comment #35
moshe weitzman CreditAttribution: moshe weitzman commented@Steven. Most everyone agrees - thats why #27 is the leading candidate. I think you reviewed #29.
Comment #36
Steven CreditAttribution: Steven commentedI was talking about the screenshot that webchick submitted.
Comment #37
Steven CreditAttribution: Steven commentedI 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.
Comment #38
drummWe 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.
Comment #39
Steven CreditAttribution: Steven commentedUsing 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.
Comment #40
drummI'm okay with this patch. Any other reviews?
Comment #41
chx CreditAttribution: chx commentedWhat could be better than supporting header regions in Garland -- implemented by the very author of Garland himself?
Comment #42
cog.rusty CreditAttribution: cog.rusty commentedTrue, and we can still hack "if $mission" for our atrocious banners.
Comment #43
Dries CreditAttribution: Dries commentedHaven't tested it, but the code^^^CSS looks sane.
Comment #44
webchickFor 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. :)
Comment #45
Dries CreditAttribution: Dries commentedLooks better than the previous screenshot, but I agree with the padding issue. It's cramped at the top.
Comment #46
webchickk, I changed:
to:
in:
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.
Comment #47
webchickPatch.
Comment #48
Steven CreditAttribution: Steven commentedWebchick:
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.
Comment #49
webchickOk, set back to RTBC for #37.
Comment #50
webchickPatch in #37 no longer applies, I think cos of the CSS url() cleanup patch.
Comment #51
webchickRe-roll.
Comment #52
Steven CreditAttribution: Steven commentedAdded the 0.5em top margin to paragraphs and image tags. That should help space things out.
Committed to HEAD.
Comment #53
webchickLooks great now, thanks!!
Comment #54
lyricnz CreditAttribution: lyricnz commentedWhat'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:
Comment #55
Steven CreditAttribution: Steven commentedThe space at the top is intentional, and was there before this patch as well.
Comment #56
(not verified) CreditAttribution: commented