Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert page template elements into movable layout blocks #796

Closed
ghost opened this issue Mar 3, 2015 · 9 comments
Closed

Convert page template elements into movable layout blocks #796

ghost opened this issue Mar 3, 2015 · 9 comments
Milestone

Comments

@ghost
Copy link

ghost commented Mar 3, 2015

What/who originally determined what was made into a block for layouts to organise and what was left to .tpl.php files to organise?

I'm creating a layout and am wondering why things like messages, page titles and tabs are organised/laid-out in .tpl.php files, while things like breadcrumbs and main navigation are organised in layouts as blocks? On top of that, there's the 'header block' that manages the logo, slogan, site name and menu in yet another method.

I think we need to either:

  1. Make everything a block that can be managed through layouts (e.g. logo is a block, account menu is a block, messages are a block, tabs are a block, etc.) <- my preferred option
  2. Do for other things what we did for the 'header block' (e.g. make a 'contextual navigation' block that displays the tabs and actions links)
  3. Move everything back to .tpl.php files (e.g. remove the 'breadcrumb' block, move to layout .tpl.php files instead)

Note that I don't personally like or recommend options 2 or 3, but had to list them as that's what's currently done in certain places in Backdrop.
Things as they are are very inconsistent...

@ghost ghost added the needs - more feedback label Mar 3, 2015
@quicksketch
Copy link
Member

I think the way these things were determined was essentially as follows:

  • The page.tpl.php was directly converted to layout.tpl.php files.
  • We moved the header into a block, which handled the messy logic of how to display if a logo and slogan was or was not present. That's the reason why individual blocks for logo, site title, slogan, and secondary menu wouldn't work, because they are all displayed relative to each other and had to be grouped together.
  • We moved the breadcrumb and main menu to their own blocks. In the first pass they were still in the layout template.
  • Then we ran out of time. 😛

I think you're right that it would be nice to move things like tabs, action links, page title, and messages into blocks, but we'd be giving users a lot of rope to hang themselves with. There's a good chance that users might remove the "Tabs" block if they think it's unnecessary, but then a module depends on them in a way the user didn't expect (take Webform module's "Webform" tab and its sub-tabs as an example).

@ghost
Copy link
Author

ghost commented Mar 3, 2015

Regarding the header block, I understand about the logo, slogan and site name, but I don't see why a menu is grouped in with them... This seems to be just because certain D7 themes had the secondary menu in the header region. I think it'd be just as easy to make this a separate block that users can move wherever they want (same as the primary menu).

Regarding other things as blocks (tabs, etc.), I think these should be treated the same as the 'main page content' block. It allows users to hang themselves by removing it but has a bold message stating that it may be needed. It looks like this is done simply using the block description. I recommend instead adding an option that blocks can set that tells layouts whether they're core, 'needed' blocks or not (similar to how modules can set required = TRUE). Setting this option in a block would display the bold message next to it and, possibly, would display a confirmation message when trying to remove it. I think that would suitably solve the issue.

@quicksketch
Copy link
Member

Oh, I think the other reason why this wasn't completely figured out is because we didn't want to include wrappers on all of those elements. Although this could be solved using the Dynamic block style, we'd probably be introducing a bit of a performance regression running styling and conditional checks on these additional 3-4 items per page. If we pursue this we should benchmark the additional overhead we're creating.

Setting this option in a block would display the bold message next to it and, possibly, would display a confirmation message when trying to remove it. I think that would suitably solve the issue.

Yeah, agreed. This seems like a generally good idea to me.

Unfortunately, I don't know if this is something that could be done in the 1.x cycle. There are already layouts out there (e.g. https://github.com/backdrop-contrib/radix_layouts) that print out these items directly in the layout.

@docwilmot
Copy link
Contributor

docwilmot commented Mar 3, 2015

https://github.com/backdrop-contrib/custom_header_block
https://github.com/backdrop-contrib/site_info_blocks
I thought this was strange as well. Made these in contrib. (second one is missing from contrib, thought I'd uploaded it).

@quicksketch
Copy link
Member

Seven has some really crazy requirements around it's positioning of title/tabs/action links. The title and primary tabs are in the header. While action links are in the content region. We may be better off with 3 separate blocks. Check the seven_preprocess_layout() function to see the craziness. I think we'll probably always tend towards separating blocks, so if we just jumped to having a separate block for each we'd solve the Seven problem at the same time.

@quicksketch
Copy link
Member

We could manage this in the 1.x version, but it would take some extra effort. Here's a plan that might work:

  • Update the layout .info files to have a requirement of backdrop >= 1.1 requirement, to indicate the layout will work with Backdrop 1.1.0 or higher only.
  • Build a mechanism to check the Backdrop version number on layouts (I don't think we do this currently) and provide message to administrators if a layout is currently not supported on the current version of Backdrop.
  • Check the version compatibility of a layout in layout_preprocess_layout() and skip the variables for $messages, $title, $title_prefix, $title_suffix, $tabs, and $action_links if a layout declares itself compatible with Backdrop >= 1.1.
  • Make new blocks for messages, title, tabs, and action links.
  • Add an upgrade path to add these blocks to all layouts.
  • Make these blocks "recommended" in a way that a warning is shown if the block is not placed within a Layout.

Layouts that specify simply backdrop = 1.x in their info files would still get the additional variables in their template and would not be allowed to place the new blocks.

@quicksketch
Copy link
Member

As the only contrib author who has ported a number of layouts to Backdrop, I'd love to hear if @arshad has any feedback on the idea of removing these variables from layouts and turning them into blocks instead.

@mikemccaffrey mikemccaffrey changed the title Layout blocks Convert page template elements into movable layout blocks Dec 17, 2015
@quicksketch quicksketch added this to the 1.6.0 milestone Dec 8, 2016
@jenlampton jenlampton modified the milestones: 1.6.0, 1.7.0 Dec 8, 2016
@jenlampton
Copy link
Member

Pushing to 1.7 since there has been no activity on this issue and code-freeze is in just over 3 weeks.

@docwilmot
Copy link
Contributor

Dupe of #1704 which has a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants