Closed (fixed)
Project:
Drupal core
Version:
6.17
Component:
theme system
Priority:
Critical
Category:
Feature request
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
20 May 2009 at 21:40 UTC
Updated:
24 May 2015 at 20:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
dries commentedI've uploaded two screenshots: before and after. I can see these additional regions being useful for complex layouts.
At first, I thought there wasn't a significant difference between 'Footer' and 'Page bottom' but I can actually see a use case for it.
There doesn't seem to be a difference between 'Header' and 'Page top' though, except that the 'Header' spans the entire width which would be necessary for a header.
Comment #2
gábor hojtsyWell, page_top and header would show more different in Minelli and other width constrained themes, where page_top would be full browser width (great for placing stuff like a header menu) and header would only be the header of the theme (great for placing stuff like menus, search box, etc).
Comment #3
dries commentedSmall code-style comment: in Drupal we don't glue words together. I recommend using 'top' or 'bottom'. The other regions don't use 'page' in their names either. I'd also be OK with 'page-top' or 'page-bottom'.
Comment #5
paul.lovvik commentedGood catch! I have changed the region names to 'top' and 'bottom'.
Comment #6
dries commentedOK, this seems to be making sense to me then. Any other thoughts or RTBC?
Comment #7
catchI think we need a screenshot of top/bottom in garland with blocks in them.
Also I don't see the difference between the header and page top regions in Garland at all - since Garland's header is already page top.
Comment #8
paul.lovvik commentedThis is a screenshot of a block in the "top" region, and a block in the "header" region.
While the header is at the top of the page, it is themed such that it has padding around it, and it also has a background color. The header region is already in use by sites using the Garland theme to put content near the top of the page. I would like to add the top region to put something similar to a file menu literally *at* the top of the page without disrupting the theming of any other blocks that might normally be found in the header region and without resorting to creating the menu as something other than a block.
Comment #9
dries commented@catch, if you look at http://drupal.org/files/issues/after_11.jpg you'll see that there is a small but important difference between header and top. If you want to add a header (think admin_menu), using the header region wouldn't work. What this patch allows to be done, is create an admin_menu that, just like your Dock on the Mac, can be moved to different regions on the page. It probably doesn't make a lot of sense for regular blocks, but for specialized blocks like a smart admin menu or header bar, I think it does ...
Comment #10
catchWell I think the header region in Garland is pretty useless as it stands now since it pushes the actual header (logo, menus) down the page. (especially if this is added). However that's probably a different issue.
Being able to do things like put admin_menu down the bottom of the screen or similar seems useful enough, and you can move your toolbars and stuff on linux too you know.
I think people are going to get really, really confused about 'top' vs 'header' and 'bottom' vs. 'footer' though. In word processing, if you add a header and footer, these are for pagers, copyright or similar on every page - above the title content of your site (which would be at the 'top' of the first page) - so this completely inverts that model. We also have content top and content bottom in a lot of themes. So Top / Header / Cotent top / content bottom / footer / bottom. Doesn't work for me.
Marking for usability and themer review.
Comment #11
gábor hojtsy@catch: D7 already eliminates the need for "content top" and "content bottom" by making "content" a wholly working region with the content as a block. So D7 will not have those potential sources of confusion at least. I agree that in some themes, the top/bottom region can be confusing. Unfortunately I don't have a good suggestion to improve on that. I think "page top" and "page bottom" were better in that they reflected the area they area of top and bottom of.
Comment #12
catchPage top and page bottom are a lot more descriptive. I'd forgotten content top and content bottom are dead, that's good, and that also means no overall increase in the number of regions.
The page bottom region could be useful for 'big-ass footer' blocks as well - often footer itself isn't good for that. But let's rename to Page top and page bottom - at least the human readable names.
Comment #13
paul.lovvik commented@catch - The region names you suggest are more specific. I have changed them to page_top and page_bottom.
Comment #14
Noyz commentedFrom a theming perspective, this makes soo much sense. Maybe now the markup for admin-menu can actually go at the top of the page ;)
Comment #15
manuel garcia commentedOK, here goes my opinion on this:
I don't see why we need another region above the header in core, I still have to find a situation where I'd need that. Seems to me if you do need it for your project, you can easily create it on your template.
The region on the bottom does seem like a good idea however, at least due to current web-design trends, although you could also add this easily, i see a lot more situation where this would be handy to have.
I think a region above header would create confusion, it takes 2 minutes to add it on your theme, or to remove the padding from the header if that's what's causing the problem...
Comment #16
gábor hojtsyManuel: the idea is that the header in Drupal 7 would be a block or two blocks, and it would be in the page top region.
Comment #17
catchOn one community site which has a lot of user / content moderation I've got a below-footer region with new revisions, new users stuff like that - so admins can access them on every page but without them interfering with the actual layout of the site. I'd still like to see some themer review on the semantics of page-top vs. header etc., but maybe if I mark RTBC more will show up.
Comment #18
manuel garcia commentedHumm ok, I have now read that link you point to, which seems like something we'd all like :)
However, reading the patch I don't see what you mean, Gábor... the header region would be inside the page top region? Doesn't look like it in the code. Or did you mean something else when you say 'header'?
If we want (and yes we do!) what is suggested in d7ux I can understand the need for a div pickin up all space above the header region. I wonder if this should be a region or not though... I guess other modules would like to add stuf in there also (admin_menu).
I'm thinking that maybe it'd be nice to add descriptions to regions, to help the user choose what would go where, then again, this might be for another feature request hehe =)
Perhaps I'm a bit out of my league here, but well, yoroy asked for input on the issue -- thanks for explaining! =D
Comment #19
manuel garcia commentedoops replying at the same time sorry
Comment #20
tic2000 commentedI'm with Manuel on this so -1
Even those blocks Gabor pointed to can be placed in the header region and the header styled accordingly.
There are 7 regions now. The reason to add another one is: "in many themes the header region has padding that cause the menu to not stretch to the boundaries of the page". What is the guarantee the new region won't have padding? What stops you from using CSS to remove that padding? What if next we want another left region to the left of the left sidebar because the left sidebar has padding?
Even with the current patch, the block content is wrapped in
.block .contentwhich has a .5em top and bottom margin in Garland so the block output won't stretch to the top boundary even with the patch.The statement "Currently there is no way to create content that appears before the header region" is so not true. You can add how many regions you want where ever you want and style them as you want.
So if there is really a need for a top region, I would just go for a patch against the themes that come with Drupal.
Comment #21
gábor hojtsytic2000: again, this is not about enabling contrib modules and site builders to put stuff into various regions on their site. This is about a core need to put up two header menus (shown at http://www.d7ux.org/header as I've linked before). The fact that arbitrary themes and site maintainers can edit their themes to have such a region does not help when we need this region to put up stuff in core out of the box.
Obviously we could also put this same header content to wherever we want on the page markup output and then style it with absolute positioning towards the top of the page. Then we can implement the user customizable close button in our own way and to this effect reimplement many of the things that block module already does, instead of reusing it. That would help us not clutter the blocks interface with extra regions. We can make this a special case, but we had a pretty nice pattern in Drupal 7 to eliminate special cases in favor of reusing general subsystems we already have built in to Drupal.
Comment #22
mason@thecodingdesigner.com commentedI agree with Manuel too about not making this a region. As a themer I almost always roll my own page.tpl and region sets, and I think a strength of the admin_menu module is that it works independently of all that.
Since this is a single-use area I can see it being abused if it's a region. Admins could add other content to this region or move this to another region, and thus break the usability gains.
Comment #23
Rob_Feature commentedAs a themer, I thought I'd throw in my perspective on a couple things.
First, I think I can pretty safely say that any site that's themed will not be using the core templates anyway...any themer (or even contrib theme creator) will virtually always be creating their own page.tpl and regions, so for most sites, this is a non-issue. We just create our own templates with our own regions. This really only affects people who are running core themes, which seems like it would be relatively few.
Second, why make the admin header a block? If the admin header is anything like the current admin module header, it can't really go anywhere except on the page header. If that's the case, it seems like it shoulnd't be a block...but should just be inserted in a similar method to admin_menu module...not in a block. This means that no region and no block is needed (one thing most sites don't need is more default blocks to sort through!)
Those are my initial thoughts....I see this as something that's not really necessary. As a themer, I think core templates should only contain bare bones regions/features (just enough to make the site work). This seems that maybe it's going 'above and beyond' what a core template should be.
My 2 cents...take em or leave em :)
Comment #24
tic2000 commentedSo this is a case of good thing for the wrong reasons.
I'm for less regions if I can help it, and in the case of the header menus I think we can and still use the current system, no special cases.
Comment #25
gábor hojtsyOk, well. Cons for having it as a block as expressed above:
- People will be able to move it around, might screw up page/site (see pros too).
- Page header region deemed superfluous by many people.
- Blocks page made busy by two new blocks (upper header, lower header at this stage).
- More markup around the output.
Pros for having it as a block (maybe need more explanation, so sorry for being longer here):
- People will be able to move it around (see cons too). Yeah. A theme can theme "the header" differently, if it is top of the page, bottom of the page, maybe even the side of the page (page side regions are not planned to be part of the core themes). Think your OS launcher. Windows / Linux / Mac let you move your "task bar" / "dock" around the four sides of your screen. Also, Drupal 7 has the facility to not let people move blocks to/from certain regions, so site builders can limit that within their site specific modules.
- We don't need to come up with yet another way to disable this feature. People will disable the blocks entirely. (The menus they show are planned to show up with data from actual custom menus, so that can be customized too.) If they are not blocks, we'll have a custom site settings page for it. Or yet another theme setting. However you wish.
- We don't need to come up with yet another custom way to have the per-user lower menu disable feature. Blocks can be set to be user customizable and already support this. We can reimplement this in a custom way for the lower header menu obviously, but we don't need to.
Other items for either side of the argument?
Comment #26
gábor hojtsyCross-posted.
Comment #27
webchickI think the main complaint from the theming perspective is "Oh dear lord, now I'm going to have to worry about what happens if someone sticks that header in my left sidebar region and put gobs of stuff in my CSS so things don't completely explode if that happens." Admin menu module is nice in this respect because it just does its thing, regardless of the theme's design. Themers can completely ignore it in their templates and CSS and it will "just work."
OTOH I agree with Gábor that we've made a big push in D7 to remove hard-coded assumptions. And we gain a lot by re-using the block system here, since it gives us nice features like "move it to the far left of the screen" and "only show it for certain roles" and "turn it off selectively per page" and "let individual users disable it if they want to" etc. There are also a lot of comments at http://www.d7ux.org/header/ from people who are very concerned about Drupal hard-coding this admin menu if they want to get rid of it for whatever reason.
Is there a way we could do both? Make the admin header a block, that's auto-attached to the top of the page and not in a region, but let people who want to do something different hook_page_alter() it? Not sure that's even a good idea, just throwing it out there.
Comment #28
tic2000 commentedWhat stops us from making that a block and use the header region? Why there is a need for a new region just for that menu?
Form a themer point of view I don't care. I can ignore that region, remove it, change its content move it in a complete different place in page, style it as I want it. But if we go with this, as I already asked, why not a left region and a right region in core that are not affected by the Garland styles?
There is some work in progress with that menu in Drupal, not just some designs?
The way the header menu should be implemented it's not the purpose of this patch.
Comment #29
webchickThe problem with the header region is it already has styling (padding, colours, etc.) that existing users of this block region are going to count on. You can't just rip those out without taking away a "feature" of the theme.
For example, here's a screenshot of Garland with both the user login block and admin header "block" enabled:
As you can see, we can't re-use this region for this purpose, because the admin menu bar looks horrid. Yet, if we remove the styling from this region, while the admin menu bar might look good, the user login block will look horrid.
Comment #30
mason@thecodingdesigner.com commentedThere's another feature request (http://drupal.org/node/482100) that might help with this from a themer's perspective. I very rarely give my clients access to blocks because it's just too easy for them to break stuff. Webchick is dead on in saying that we're concerned that users will totally screw up a layout by moving this block, but that holds true for nearly all blocks. If we could lock down some regions and blocks I think that would solve the issue for me and make me comfortable with this solution.
Comment #31
tstoecklerEven though I am actually in favor of this approach, I think the motives are flawed. The only reason that the current header region can't fit the proposed d7ux-header is because of the padding/margin that comes with it. That means we're not introducing a region to be able to place things in a certain place on the page (because we already have a region at exactly that place), but simply because of the need for different styling. This really unveils a current flaw in the block system: All blocks in one region have the same styling. This same problem caused the need for a "Help" region right where the content is, just because Help is supposed to look different. And the same goes for the "Highlight" region. All these (Highlight, Help and Content for one, and Header and the proposed Page top for another) are really one "region", as in "place on the page".
Therefore, if we want to fully embrace our block system, we need to be able to set different styles for blocks in one region.
Not that I don't expect to be beheaded for this comment, but Joomla! has this thing for their block-thingies, where you can enter a predefined "argument" (or however they call it), which adjusts the style of this particular block. This allows for amazing flexibility, and btw for much richer themes.
Sorry, for kind of taking this off-topic, but I felt the topic was kind of off-topic, too.
Comment #32
tic2000 commented@webchick
But still style.css in Garland theme could add some styles to make the menu look as it should. Or if we really want a new region add that only in garland.info and in page.tpl.php in Garland folder cause the problem is with the style for that theme.
I would like to see a screen shot of that menu with the patch applied, cause I think as I already said, it will have some top margin anyway.
Comment #33
Jeff Burnz commentedI tend to think users of Garland would benefit from the extra regions, I never hear anyone complaining about a few extra regions in highly stylized end user themes - the more the merrier seems to be the catch cry.
For core stark templates, I think low/semi skilled "style only" themers will benefit. While regions are easy for experienced themers to add, this can be a major undertaking for the low skilled themer, and I don't think hard-core themers are using core templates. Regions are just as easy to remove also.
As for naming conventions - I have always referred to the top most full width region (above the traditional header or masthead) as the "leaderboard". For the page-bottom I tend to think this should be "footer" and the new region placed above the footer region (swap it) and call it something else. In my experience even when you place menus or blocks at the bottom you usually place the footer below these anyway, such as http://nettuts.com
My 2 cents.
@tstoeckler - this is doable with the Block Theme contrib module, and for most themers/themes the block id and classes would suffice.
Comment #34
ksenzee+1 from me. I think there's a good use case for both regions. I personally like the name "leaderboard," but my vote is for "page_top" and "page_bottom." They're quite clear, and it seems a shame to introduce more jargon into Drupal.
Comment #35
dries commentedInteresting debate on styling vs positioning. People obviously need to be able to style individual blocks, but it is also quite interesting to style blocks on a per region basis.
In this particular case, the header is a single block (?) in which case we could go with either solution. However, it could be really interesting to think of the header as multiple blocks that seemingly integrate/connect. It would make the header extensible/flexible in a unique way -- just like the menu bar on the Mac or the icon dock on Windows.
Comment #36
gábor hojtsy@tstoeckler: I'd suggest you install an actual Drupal 7 and try the regions. This is how it looks on the blocks page:
The highlighted content and help are not just differently styles. The help region patch has multitudes of comments on why it made sense as a region. For example, some themes display a JS tool which allows you to hide/show help. That would hide/show all the blocks at once. Otherwise you'd need to mark blocks as "this is a help block" in some way. There is no way in core for that. Also, there is stuff displayed between help and the page, like the notice that the form is still to be saved. There is all the tabs, the page title, messages, etc. displayed inbetween the highlighted content and the help. I believe the highlighted content and the help and other similar regions make lots of sense in relation to semantic markup too. The web is not all about just styling the blocks as they should look but also about making logical devisions on the page to designate certain content by its role on the page. Things like the "highlighted content" container and the "help texts" container convey this semantic wrapping.
Comment #37
tstoecklerSold, for now. :)
Still making titles, and messages and everything a block and having that all in one region would make the most sense. But I see that is not accomplishable currently.
Comment #39
yoroy commentedsubscribe. Themers, if you override system defaults anyway that removes basically validity of your arguments against this addition. I think it's a better default then what we have now.
Also, it would have helped if this issue outlined it's objective from the start: make room for the proposed d7ux admin header.
Comment #40
yoroy commentedpostponed #484820: Initial D7UX admin header on this one.
Comment #41
tic2000 commentedI disagree. We don't override, we change system defaults, which is what this patch does anyway, only that we will have 7 regions without this patch and not 8.
Comment #42
tic2000 commentedAnd to try to prove my point better.
We have now a header region which is not as the name may suggest the same as the site header. A region that is not used in system defaults, but we want to add a new region above that which will be used.
http://www.miidecuvinte.ro/d7/ user: tic2000, pass: d7test
The link above proves we can use the header region. I applied the admin header patch and modified it so it removes the region it created and change page_alter so the header menu is inside the header region. All I had to do after that was to add 2 css rules to make the menu look the same as it did in the new region.
Comment #43
gábor hojtsy@tic2000: I don't doubt that it would be any problem to even put a block in the footer region and style it to appear above the page top via CSS (the current admin_header module adds markup to the bottom of the page and styles itself to be on top for example). I think the page top and the header regions are conceptually different in that "header" is used to place user facing navigation menus, the search box, etc, which all blend in with the design (are fixed width if the design is fixed width, etc) while the page top would be used to display admin related stuff, which is not necessarily in the end user facing design/width/font/color. I think that distinction is worth the different regions, and having so different things in the same region would be confusing at best. As I said above, regions are also about conceptual placement not only about page level positioning and styling.
Comment #44
Jeff Burnz commented@tic2000 - I don't get why you are blocking on this, they're conceptually different and in the grand scheme of things we are trying to make Drupal easier to theme/configure/use and this archives those aims, no disrespect, but whats your point?
Comment #45
catchCould we get a re-roll with no page-bottom region (doesn't seem necessary for the header, and easy to add in a followup patch)?
Comment #46
yoroy commentedYes, even better so +1 on that, focus back on intended use for this new region.
Comment #47
tic2000 commented@jmburnz - You give me too much credit. I'm sure this issue is not blocked just because of me. You should read my comments to see my points.
@Gabor - My previous post and your post prove that people may think differently on the purpose of the header region. And a region is just a region. Its name it's not a law "Hey, this is 'page top'/'admin tools' so YOU are not allowed to put anything else here, or anything else that is not administration related".
I did not follow the issue about the help region, but from what I see, in D6 there is a $help variable in page.tpl.php, but in D7 there is a region for it because you couldn't hide the content with javascript? So instead of wrapping that in a div a region is added in core? But probably was more than that.
I don't thing a region should be added in core for everything a theme could do, or a module could do. That theme or that module can add its own regions if it needs to.
Now that being said. Admin header will be in core so maybe it needs a region, but then "page top" tells to "Jeremy" (as Leisa likes to tell) that this is on top of the page, not that SHOULD be used for administrative blocks and use the header region for other things, also page bottom has no purpose for now in the "grand scheme of things".
Over and out, I'm not "blocking" this issue anymore.
Comment #48
catchtic2000, the idea with the extra help region was so you can add custom blocks to it and have them displayed as help - contact form help, user registration help and other variables were turned into regular blocks as part of the upgrade path). Since there's no way for a custom block to declare itself as a help block, the only way to get unified styling is to add a region. Yes, a Drupal admin can put their events block in the help region and mess up the layout, but that's not a new issue at all, putting recent comments in the Garland header regions is a disaster as well.
The advantage of the page top region is say I'm setting up a basic site, and want to display a jquery ticker of unpublished content from views (very easy to do in contrib with zero code) - so content editors can get updates on unpublished content in a compact way. With this patch, I can just create the view and a block display, stick it in the header region with the admin header, and that's it - I don't need to reimplement tricky CSS or javascript in my theme to get it in the same spot, then deal with 'position: absolute;' and measuring how many pixels the admin header takes up (and on and on). We already remove the need for content-top and content-bottom regions in core by making content into a block, and removed a lot of magic variables from template.php, if the cost of this is a couple of extra regions then that's a good trade-off IMO.
Comment #49
sunMeh. Why do I see not one maintainer or developer of only one administrative UI enhancement contrib module in this issue?
Sorry, but this is like adding Fields to core without working and leveraging the knowledge and experience of any CCK(-related) developers. This way, we are wasting much time.
A new default region only works for custom themes. Drupal core does not have to provide one, because no module can safely rely on it. Repeat: No module can safely rely on the output, nor on the styling of the output.
FWIW, admin_menu dropped its output-in-a-block approx. 1.5 years ago. #179648: Inject admin menu into theme
Comment #50
gábor hojtsy@sun: good to hear your opinion. Thanks for not letting us waste time. Maybe then reflect on the pros and cons from http://drupal.org/node/468534#comment-1664332 please, if you can.
Also note that core can require regions. A contrib module like admin_menu cannot obviously. But Drupal 7 already requires all themes to have a "content" region, and will not let you enable a theme, if it does not have a content region (it is marked incompatible with D7).
Comment #51
tstoecklersun, I'm not sure I get your point. Do you suggest putting the extra region only in Garland and not in the default regions?
Comment #52
sunTo clarify:
- A "top" region may be useful for themers. But can also be added manually (and quickly). Not sure whether Drupal core should provide that. FWIW, I also don't agree with the new "Highlighted content" region, because it's confusing to find out that sticky posts do not even end up in that region.
- For administrative tools/UI enhancers, as well as for the d7ux work, a region does not make sense. A region implies to use blocks, and blocks have styling. A region can be moved inside of page.tpl.php, or even hook_page_alter(), and probably in countless of other ways, too. It's up to the themer whether the region is wrapped by other markup, whether additional styling applies to it, and whether the region is output at all.
- Blocks can be moved elsewhere. And if blocks are moved, a header bar like the suggested one needs completely reversed logic and behaviors when it is "suddenly" displayed at the bottom - or at the left - or at the right. When displayed somewhere else, many parts of the logic need to be flipped. If that would be easy to implement and to account for, trust me, admin_menu would support to be displayed differently since a long time ago.
- I already stated in the other issue that we don't need a separate theme for the proposed admin header. The support for an administration theme in Drupal core is flawed and very unreliable. Administrative tools cannot safely rely on it - nor can any administration theme. See also #480962: Call to undefined function when editing content in Admin's issue queue. It seems like development on these topics starts from scratch where admin_menu started years ago. Logically, these developments will step by step face all issues that admin_menu needed to tackle over time. I have little interest in duplicating all that work.
Comment #53
gábor hojtsy@sun: while we might revisit some of the decisions which were made in admin_menu, core has a different perspective and toolset. With Drupal 5, the contrib i18n module could not rely on other modules knowing or handling language, but with Drupal 6, that became a core concept, so modules started to take it into account. Even for admin_menu module in D6 to work, the theme needs to output $footer, doesn't it? So we need to rely on certain things anyway.
Here the patch proposed relying on something else, because the admin header proposed for Drupal 7 has some block-like characteristics. For example, the lower menu part is to be disabled on a per user basis permanently, but should be possible to be enabled again. Look, custom blocks do that. Also, obviously people want to customize it and turn it off selectively if desired (eg. make it available to certain roles only). We could make all this in custom ways, eg. add our own code to remember display of this on a per-user basis, add our own code to make this work on a per-role level (add our own permission for example), but all that is already done with blocks. We can obviously reimplement it.
I am not sure admin menu faced with the same task, so it might not have reached the conclusion we would reach. Therefore I think the discussion on how we do it is worthwhile.
For example, in the permissions case, I think it might make more sense to actually add that permission, even if it is only about "displaying the header" (and this was already discussed on d7ux where Leisa proposed an "admin type of user" kind of thing, which ended up suggesting permissions). That is because people think permissions do not only "hide" stuff, but they make stuff "unaccessible", which is two different things. However, in the admin header case, it would probably be the same so far, since there is no functionality which would be accessible but hidden if we hide it with the blocks UI.
Anyway, there is a certain set of requirements with the admin header, which overlaps in some areas with admin_menu (where your input so far is great) but not at others. Since we working on getting the admin header into core and not the admin menu, we need to find answers for the questions on the admin header.
What do you think?
Comment #54
sunThis is a theming issue, and I specifically provided input about why theming an admin header bar does not work reliably as a block. This proposed admin header bar will face the very same theming issues like admin_menu.
Yes, admin_menu requires output of $closure. $closure is a required page.tpl.php template variable. I can provide you a list of modules that will break your site when that variable is not output.
Comment #55
gábor hojtsyWell, if $closure can be required, how would $page_top be any more complex to require? Also, themers are not stopped to wrap $closure in special markup the same way they are not stopped to put $page_top within special markup. Core's special place is that if a theme does not work with a core feature which is enabled by default on install, then it is pretty broken. A theme will work with core on install if $closure is not output, so it would be easier to leave $closure out accidentally compared to $page_top. I think you feel constrained by rules and practices of Drupal 6 themes, while we can make up rules and practices now for Drupal 7 themes, so we should not constrain ourselves with previous practices to overcome a lack of a reliable place to put an admin header to.
The reason to put the admin header as a block is really to make it as customizable as possible. Limit to certain pages, roles, make it per user configurable, remove it altogether, style it differently if it is moved to a region on the bottom or side of the screen, etc. As an alternative, we can use hook_page_alter, hook_footer or whatever, reimplement some, but not much of these configuration options ourselves and get to a special, less customizable, throw-away type of menu for people, who don't like it as it is; or at least we make them write code to alter its placement, display, etc.
Comment #56
sun$closure is not a region. Core working without $closure is a bug.
If we follow this route, then the entire block and region system will be a major mess of exceptions. Just look at how many exceptions we have for that "system-main" block already.
Just yesterday in IRC:
I tried to look it up. I failed to find it. Even after reviewing that patch.
Comment #57
gábor hojtsyYeah, $closure is not a region. The reason we would instead use regions would be to make it user customizable with dozens of ways out of the box. $footer_message, $help, $mission, etc. were exceptions on the theme level. We generalized these to regions again for the same reason, to make them user configurable without further coding. I favor reusing existing subsystems with admin/user facing customization (when the task at hand maps to the subsystem as described above) to doing code based exceptions on the theme level, which requires specific knowledge to customize/remove on a feature by feature basis and can only be done in code -- but I am not the decision maker on Drupal 7, and this issue definitely needs more input from others too.
Comment #58
catchedit: I cross-posted with the past three followups, I largely agree with sun's concerns - we don't want contrib themes to completely break the primary (possibly only) visible navigation for site administrators - so it has to be done in a way in which the blame is firmly placed on the theme. That means either re-using $closure (which probably doesn't help accessibility - especially if we only have 7-8 links up there instead of a full menu tree), or a required region (or $opener variable if there's valid reasons to not use a region).
Comment #59
gábor hojtsyIf the only concern that themes might leave $page_top out, then they might as well leave $closure out. Both cases would break D7 sites using certain modules if the D7 admin header is implemented via $page_top and the modules using $closure keep using it. Neither of these have checks yet in Drupal to ensure it works. We can build in checks for either obviously and not let themes get enabled, if they don't implement certain things we think are important. I think the concern with $page_top and themes just as well applies to $closure. Eg. if a themer makes a simplified page template for Apple Dashboard output or whatever, they would just as well leave out $page_top and $closure, but we would not care. Obviously whatever check we build in, we cannot make sure all page templates include $closure or $page_top, since we don't know what kind of arbitrary names themers might use for page templates.
If there is a concern that themers will delete important stuff, we should prevent that by adding in checks for the enforced elements in the theme. With the proposed block based implementation of the admin header, $page_top would be an important thing themers would need to include. $closure is already an important thing for many contrib modules in Drupal 6 (but not at all to Drupal core itself AFAIK). With regions, we can actually look into the .info file and see whether the theme has the given region (and kind of imply that it is actually output). Special variables like $closure are harder to enforce, since we need more special code to actually parse template code. But if we are truly concerned about themers removing stuff, we would need to do that. I don't buy the argument that leaving out $closure already breaks many contrib modules, so we can assume it is there, since breaking the core admin header is a totally new level of breakage, and requiring $closure by core would be a new thing.
Comment #60
catchI think the main thing is we want people to be complaining to theme maintainers that their theme is broken, not in the core issue queue that admin_header.module is broken. If the requirement for $closure / $content / $page_top is documented somewhere, then IMO that's fine - it's a bug report for deviant contrib themes. I also think it's an improvement to be outputting regions with some exceptions, than special variables. It'd be nice to remove some of those exceptions later (like make the $content block pull like other blocks), but we've got a long way to go on that.
Comment #61
paul.lovvik commentedI rerolled the patch, omitting the page_bottom region. Agreed that this bottom region could easily be added later if needed, and in fact I originally added it just to be symmetric.
There is no dispute that regions can be added at any time after installation as required. I want to stress that getting this type of modification into core is important to the goal of getting some type of menu into core that has the purpose and effect of helping newer users make use of Drupal more naturally. While all of these pieces can be assembled by a site developer that knows his way around Drupal, the focus of this work is to aid in the integration of ease of use affordances into core that will make the capabilities and configuration of Drupal more obvious.
[slaps forehead for not including such reasoning in the original issue text]
The key points that make this an important addition to core for me are:
1. The d7ux menu should be added to core if it is desired to help site creators new to drupal.
2. It seems appropriate that the content that comprises this new header would be a block (as opposed to styling magic to achieve positioning).
3. All of the designs for this header so far include a full-width menu similar in many ways to a file menu, and it feels right that the containing region would control the width of the content within.
Given these motivations, a new region seemed an obvious step.
Comment #62
paul.lovvik commentedComment #63
dries commentedGiven that there is no consensus yet, and given that I'm going back and forth on this myself, I don't feel like I want to force a decision yet. So let's do the following: get the header in using a hook_pager_alter() and leverage the existing 'access administration pages' permission to make the header show up. That will work for now and allows us to make progress with the header. There are some gotcha's with that, but we can revisit this patch once we learned a bit more about them, and once we're ready to take advantage of block-level benefits.
Comment #64
sunThe 'access administration pages' permission may not be abused to trigger the display of that admin header. I've mentioned before: The proposed admin header bar needs to be a module. Which can be disabled.
Comment #65
catchsun - in the current patch, it's already a module which can be disabled. What's not been decided is how it gets displayed - I think it should be a 'view admin header' permission similar to how admin_menu module works.
I'm not sure how hook_page_alter() will help us get the header into the top of the page - it only lets us add / alter stuff in regions, not actually add them right?
Comment #67
lilou commentedSetting to 'needs review' - testbot was broken.
Comment #68
cwgordon7 commentedWhile I think that fewer regions is a good idea, I also think that such a need must be balanced with the need to be able to flexibly display content in different regions. Instead of adding another default region, would it make sense to just add a new region to the particular themes that do not have a suitable header region? (e.g. fixed width headers/etc). Modules could then check for the presence of the region before assigning something to that region; if it's not available, modules could just default to the 'header' region. If core uses this strategy for an administration menu at the top of the screen, this will likely ensure that contributed themes either have a suitable header region or use the 'page_top'/etc. region: otherwise, they would look ugly on the default installation.
Comment #70
seutje commentedsubscribing
Comment #71
webchickI'm pretty sure this was a testing bot freakout.
Comment #72
webchickThere hasn't really been any counter-proposals on this issue that I can see since Dries's proposal in #63 on June 12. Mind you, testing bot repeatedly freaking out probably had something to do with that. :P But IMO we should probably proceed with this so we can get the admin header in. Marking RTBC. Will sleep on it, unless Dries beats me to it.
There are a few minor adjustments needed in two places to #61 which can either be a re-roll or fixed prior to commit:
Needs semicolon.
Needs space before ; and ?
Comment #73
dries commentedI made those minor adjustments and committed this patch. Thanks all!
Comment #74
senpai commentedD7 is now generating a
Notice: Undefined variable: page_top in include() (line 18 of /www/drupalhead/themes/garland/page.tpl.php).Comment #75
yched commentedI think those notices went away after my theme registry got rebuild (visit admin/build/modules)
Comment #76
senpai commentedThis was a brand new install I was testing on, but whatever dude.
UPDATE: ok, ok, they went away. <grin>
Comment #77
gábor hojtsy@sun: the related follow up patch is at #511284: Add html_top and move the admin toolbar into html_top. I'm trying to defend some of your points there, but your input might be valuable there.
Comment #79
ramen2 commentedHow have used drupal 6.17 but we can't create any region into header below, i used garland theam please give me any Suggestion?
Comment #80
ramen2 commentedwe have used drupal 6.17 but we can't create any region into header below, i used garland theam please give me any Suggestion?
Comment #81
yoroy commentedramen2: Please do not re-open issues for a support question. Try the drupal forums or open a new support request, but this is not the place.
Comment #82
yoroy commented(resetting all status thingies.)
Comment #83
yoroy commentedCleaning out 'needs usability review' tag
Looks like we now have in place what was discussed in here.