Split content region into content_above and content_below

Subscribing, I'd love to take a look at this today......... We might be able to move how comments are rendered below the content to this new content_bottom region.

There will be $content->$content_bottom update path, verdad?

Cross referencing #107794: At least rename the 'content' region to 'below content'.

This issue was also found in UBUserTesting2009, so tagging and referencing: http://www.drupalusability.org/node/10

one more tag…

Added the update function.

This seems to apply just fine for me.. I'll give it another shot.

Are you trying ton replace the content region with content_bottom and content_top, or just add content_bottom and content_top? It seems a bit inconsistent here. In some cases you're replacing, in some cases you're adding bottom and top.

I think it would be logical to add the content_top and content_bottom regions. Either that or come up with a better weighting system for objects that are being added in the regions.

I dont quite understand what you are talking about. This removes the content region and adds a content_bottom and content_top. Can you point me to a specific instance where you believe that the content region is not being properly removed? I have re-tested things and it appears to be working as intended. Until then, lets give the testbot another shot at this.. cause it applies on my end.

Ideally, names should describe purpose, not use cases. $content_bottom tells us something about the case where the region is being used as the bottom part of the content, but what if we want to display it next to the page's general content? (The same goes up for the sidebar regions, but that's not what this issue is about).

before / after?

System.module -- Replace

     'regions' => array(
       'left' => 'Left sidebar',
       'right' => 'Right sidebar',
-      'content' => 'Content',
+      'content_top' => 'Content top',
+      'content_bottom' => 'Content bottom',
       'header' => 'Header',
       'footer' => 'Footer',
     ),

Bluemarine theme -- Add

         <?php if ($tabs) { ?><div class="tabs"><?php print $tabs ?></div><?php } ?>
         <?php print $help ?>
         <?php if ($show_messages) { print $messages; } ?>
+        <?php print $content_top; ?>
         <?php print $content; ?>
+        <?php print $content_bottom; ?>
         <?php print $feed_icons; ?>
       </div>
     </div>

Pushbutton theme -- Add

       <!-- start main content -->
+      <?php print $content_top; ?>
       <?php print $content; ?>
+      <?php print $content_bottom; ?>
       <?php print $feed_icons; ?>
       <!-- end main content -->

Garland -- Add

+            <?php print $content_top ?>
             <?php print $content ?>
+            <?php print $content_bottom ?>

Xano at #16 is also correct. We'll have to describe the region rather then state its location. The theme could essentially place the region anywhere, not exclusively at the top of the content.

Also, note that the new drupal_render() gives us even more control over how things are weighted during the rendering process. So, we should keep that in mind before adding a couple regions beside each other.

I haven't really checked out the new drupal_render() and hook_page_alter() yet (which both are relevant as far as I can tell), but kika's before and after make sense.

Let the Bot have a stab at it.

Updated search test to add the block to the content_below region instead of content. Tests should pass now.

Not sure if this is desired, but:

+    $this->regions[] = array('name' => 'content_above', 'id' => 'center');
+    $this->regions[] = array('name' => 'content_below', 'id' => 'center');

...... Two different regions with the same 'center' ID?

In this particular case, I believe this is correct.. both the content_above and content_below regions are being placed within div#center. What you posted in #25 is what is telling simpletest where to look for blocks placed in those regions.. though this is my first real exposure to simpletest, so I could be mistaken.

Above and below what? Those names don't make sense without a central $content region. If we stick to splitting $content in two, the two new regions should be $content_top and $content_bottom. This is more semantically correct.

Shouldn't semantics define the content instead of where this content should be? Top and bottom doesn't really define much, aside from where it should be. Since they are regions, you could essentially stick the content_top below the content_below.....

As a bit of a Drupal theming newbie, the changes in #18 that Rob is questioning make sense to me.

It seems that we currently have a Content region, into which we can add various blocks. The proposal here is to split that into Content Above and Content Below regions, which flank the main content of the page. As it stands, the block settings page does not allow us to choose which region the main content goes into, and so therefore the Above and Below names do make sence, in that the $content is output in between these regions. (As an aside, does anyone else like the idea of having the $content be outputtable into a region? Kind of like a mini-panels i guess.)

I guess hat I am understanding is that in D6, you can add blocks to the central region (I'm not sure off the top on my head if they display above or below the node content - I'm assuming below based on the upgrade path). What this patch seems to do to me is provide administrators with a bit more flexibilty in deciding whether those blocks appear above or below the main content.

If I have missed the boat on the intention and functionality of the patch, let me know.

@#28: True. Both $content_above and $content_top are wrong names from that point of view.

Something else: should this really be in core? This really looks theme-dependent to me.

subscribing

Sorry, but was is $content_top and $content_bottom ? $content is a completely non-descriptive and nonsense variable already; so here we add to this nonsense by adding meta-information (locative information) to "content" (whatever that is).

...now I had a look at the patch and found out that it would have made sense to have a proper description of the goals in this issue.

The separation makes sense, but the naming issue still stands.

I think that the naming of regions should be saved for a different issue. We already have "left sidebar" and "right sidebar" which have locative information. I do not see "content top" and "content bottom" as any worse. Now, with that said, I do agree that locative information should be kept to a minimal, but that is something that should probably be discussed elsewhere, as it is probably going to generate a debate of some sort, as there is no perfect answer.

Will we drop $content's functionality being a region to which blocks can be assigned here? That would be awesome!

I've submitted an issue to get the IDs of regions unified: #422738: Improve consistency in base region IDs. It was marked in part a duplicate of this issue (since I've worked with the region IDs there). Reading through the existing comments, I agree that "content" is an unfortunate and ambiguous name. It is how it was in all Drupal versions I knew, and the way we would be able to get out of it IMHO is to make it a "block" so you can weight it as anything else in a region. Then "top" and "bottom" would not make sense, since the "content" itself would be movable among other items. Same applies for footer, where the "footer message" should simply be one block and not a special Drupal setting. After all Drupal supports putting blocks into regions, and reorganizing them, offering the most control over this to the site admin on top of the theme. Then we can just name the region "main" or something (assuming the region covers a major part of the screen, however this is also highly theme dependent), and be done with our job. People would be able to put blocks above and below "content" simply because it would reside in a region as a "block".

I'll look into how twisted it would be to make the content item a block.

Here is a patch which is tested to work with Garland and Stark (and assumed to work with Minnelli obviously). I've also tested basic update.php screens (since that is using the maintenance theme), and that also worked.

What I do here is basically at any point someone tries to set the page, I store the page data for a system module block to use (the function used to generate that is named "drupal_get_page()" which both does some metadata generation and renderable content array generation).

The system module block can be positioned however you want it to and stuff can be put above and below it. Obviously if you disable this block, you can easily make damage to your experience, and getting it working again would require digging in the DB. *However* this opens the door to disabling this and only using other block regions on some pages allowing for more flexible layouts when you need to arrange your page in an unconventional way.

For testers: either install your Drupal 7 fresh or uncomment the line where it says UNCOMMENT. Then do what the comment says (enable and position the content block) and comment it out again.

I think having one region where you can order stuff is much more useful then having "top" and "bottom" regions, especially when in certain theming it could be shown left and right to the content region (eg. horizontal scrolling layouts), so it would rather be "before" and "after", but moving stuff from region to region just to work around something you cannot move is IMHO worse then just making what you cannot move movable :)

This is how it looks on the blocks admin:

Ps. I already have issues to make the help text a block and plan to follow along the line given enough progress in how this gets implemented. :)

Obviously, renaming region "Content" or block "Main page content" to whatever else is completely possible, it does not change the concept.

That's very, very neat...

Isn't the block module optional now in D7, though? I'm not sure if there were good reasons for making it optional or they just made it optional because they could, but either way, this patch doesn't work well if you turn the block module off :)

Nice. I get that you're more interested in concept/code, but couldn't help myself. IMHO, "body" would be the better name for this region. Teh word content is too overloaded in Drupal, and I think "body" or "page body" is what comes to mind when you think of the main element of a page.

Wonderful idea Gábor. It solves the naming problem, adds more functionality, and gives greater flexibility all around .. all with less code.. beautiful.

The only thing I fear is now the explanation of a block becomes difficult. I really like the idea of making all of these "magic theme variables" blocks so that they are seen and positional from the UI.. but then the true definition of a block becomes tricky. But we can worry about wording of help text later.

I did a quick testing of the patch, and things appear to work as suggested. I am in favor of this approach and in general removing as many of these variables from the theme. I can imagine every page.tpl variable ($messages, $help, $primary_nav, $search_box ... ...) being a block, positionable into any region, which makes page.tpl simple a structure of regions.. that would be beautiful. Anything that is in the direction of that goal, I am all for. And this is a wonderful start. +1 from me.

Hm. While I like the idea, this has some drawbacks:

1) The block can be disabled too easily. Once disabled, your site renders _nothing_. At the very least, there should be safety measures to prevent this.

2) New/additional themes take the default block configuration from the default theme. As with 1), it will be hard to ensure proper output of this block in a Drupal site with multiple themes enabled.

3) Block module is indeed not required anymore. Primarily, to allow for alternative implementations (think Panels). I wouldn't like to see the dependency on Block module re-added. So, ideally, this works with and without Block module being enabled.

4) We need to explain themers and site admins when to use page.tpl.php overrides (like in the past) and when to adjust block visibility settings.

Regarding 1), that won't be easy, as themes (like Zen?) can use completely different region names and no "content" region at all. The most trivial approach would be to ensure that this block is enabled and assigned to a region in a form validation handler.

I think a fallback which just makes sure the content block is always assigned to a region is pretty good. Don't see a particular reason why we couldn't require all themes to have a 'main' or 'content' region for this block to live by default - in place of the $content variable.

This would also allow for caching to be enabled for the main content region using the existing block cache.

I think we should move this patch and discussion to a new issue - it'd be a major change and as sun points out has a lot of both good and potentially tricky implications.

As much as I like the idea of having all current template variables available in the block system,I think we're making things much more complicated than needed. Just asigning the page's content to a new template variable, say $page_body, also fixes all stuff discussed here. The current system of theming is flexible enough IMO.

skilip, simply renaming the variable does not solve the issue at hand. The issue that is trying to be solved is the fact that you cannot place blocks above $content. The content region sits below the page content output. The two ideas to solve this issue are, at the moment, split the content region into "content top" and "content bottom" where $content sits in-between the output of those two regions.. or, convert $content into a block, which therefore allows you to position it and change its weight, which in turn solves the problem (but opens up quite a few more issues as described in #41 and #44.

I'd say, lets keep this issue focused on splitting the content region in two, open a new issue based on Gábor's patch in #39, and postpone this issue until a decision has been made on that.

Well I didn't mean to just rename the $content variable. The problem here lies in that the page content is placed into the $content region, bypassing the normal block / region system. Therfore the $content region is an exeption in the regular region system. So what we want to do is separate the region from the page content. Gábors proposal fixes that issue big time, but raises a whole scala of new issues, wich can easily be avoided.

So if we'd change $content in a regular region (or split them up into content- top and -bottom) and put all page content in a new variable like $page_body, just like $messages and $tabs, we're done. Everybody happy :).

I defenately don't want to push this issue into bikeshed talk! I just don't see why we should rebuild a widely appreciated system.

To fix the problem of disappearing content we could add a #required parameter to blocks. Or we could just hardcode that $content should be in a region.

I think #45 explains the answers to #44 pretty well. The issue it does not address is block module dependency. The last time I've seen, panels supported putting blocks into its areas and also, if you disable block module I assume something else would come in in its place (otherwise you loose your search box, login box, etc. as well). Panels or that something else can just do system_set_content_block() to get the content data and display it however they like. I don't think we need to hardcode this to Drupal to do it always, if block module is not enabled, that would require panels to page alter that out.

Ps. I am fine with moving this off to a new issue and mark this as postponed.

@Gábor: ho do you think about my comments at #46 and #48?

@skilip: this issue was all around doing the same thing you explain in #48 until I entered and derailed it saying that adding in "top" and "bottom" regions just show that we don't take advantage of the ordering feature in our regions system, while we should. So your suggestion was already debated in detail up until #37. We are talking about an alternate approach now.

Ah, I totally missed that. Thanks for the clarification, sorry for not reading thoroughly. Is there another issue opened for this?

Copied over the patch and my introduction as well as concerns you had and solutions to those to #428744: Make the main page content a real block. Marking this postponed on that as suggested by ultimateboy. All, let's move to http://drupal.org/node/428744 and work together there!