Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett created an issue. See original summary.

tim.plunkett’s picture

tim.plunkett’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 2811179-theme_layout-2.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
8.46 KB
24.65 KB

Status: Needs review » Needs work

The last submitted patch, 5: 2811179-theme_layout-5.patch, failed testing.

tim.plunkett’s picture

My only worry with this issue is that right now it adds very little value.
The motivation here is to keep the block/theme system up to date with the other layout work happening in #2796173: Add experimental Field Layout module to allow entity view/form modes to switch between layouts

With entity displays, there is no mechanism for layout, so adding that single feature is a big step forward.
Themes already define complex layouts, and while adding a new region is a two or three step operation, it is completely doable.

Without advanced features like variants and context, this change is almost worthless.

The question becomes, should work here continue, or should it wait until field_layout and layout_plugin go in, and work on variants and context begin?

tedbow’s picture

My opinion is that it adds very little value without the ability to use different layouts in different pages and under different conditions.

It seems like it would be more valuable to select which block show up under different conditions even it is uses the same layout than to be able to switch global layout but have it always apply on your site.

Most sites will probably be extending a base theme either from core or contrib. So they could easily customize the layout they want.

xjm’s picture

jibran’s picture

jibran’s picture

Status: Postponed » Active

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tim.plunkett’s picture

Status: Active » Needs review
FileSize
10.92 KB
6 KB

We might descope this to just respecting *.layouts.yml in themes, with no UI. TBD.

Status: Needs review » Needs work

The last submitted patch, 13: 2811179-theme-13.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
3.5 KB
11.69 KB

Let's actually use the layout.

Status: Needs review » Needs work

The last submitted patch, 15: 2811179-theme-15.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
9.68 KB
8.3 KB

This removes the UI and ability to switch layouts.

Status: Needs review » Needs work

The last submitted patch, 17: 2811179-theme-17.patch, failed testing.

tim.plunkett’s picture

Title: Allow themes to switch layouts via the UI » Allow themes to provide Layout API integration
Status: Needs work » Needs review
FileSize
7.77 KB

Don't need that schema file anymore either.

Status: Needs review » Needs work

The last submitted patch, 19: 2811179-theme-19.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
7.77 KB

Trying it without the hook_install bit

Status: Needs review » Needs work

The last submitted patch, 21: 2811179-theme-21.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
9.43 KB

Rewriting to be part of layout_discovery.

Status: Needs review » Needs work

The last submitted patch, 23: 2811179-theme-23.patch, failed testing.

tim.plunkett’s picture

The last submitted patch, 25: 2811179-theme-25-combined-with-2860346.patch, failed testing.

andypost’s picture

+++ b/core/modules/layout_discovery/layout_discovery.module
@@ -37,3 +40,64 @@ function template_preprocess_layout(&$variables) {
+function layout_discovery_system_info_alter(array &$info, Extension $file, $type) {
+  if ($file->getType() === 'theme' && _layout_discovery_has_layout($file->getName())) {
+    $layout_definition = \Drupal::service('plugin.manager.core.layout')->getDefinition($file->getName());
+    unset($info['regions'], $info['regions_hidden']);
+    foreach ($layout_definition->getRegions() as $region => $region_info) {
+      $info['regions'][$region] = (string) $region_info['label'];
+      if (isset($region_info['hidden'])) {
+        $info['regions_hidden'][] = $region;

why limit themes with one layout only?
and why that works for themes only?

config install will care about install themes last so all config that depends on layout will wait theme install.
I know that orthogonal to the issue but related to how installer will gather info from themes

tim.plunkett’s picture

Here's the problem with this approach:

  • drupal_flush_all_caches() calls ThemeHandler::refreshInfo(), which resets the list of themes, and starts rebuilding it
  • While rebuilding, ModuleHandler::alter('system_info') is called, invoking layout_discovery_system_info_alter().
  • That calls _layout_discovery_has_layout() which consults the LayoutPluginManager for definitions.
  • LayoutPluginManager::getDiscovery() calls ThemeHandler::getDirectories(), which looks for the list of themes.
  • But the list of themes is being rebuilt!
  • So it goes back to rebuilding the lists of themes, and loops forever.

I'm not sure what to do about this without making changes to system.module...


#27, earlier patches attempted to support multiple layouts. But that would require a UI, which is a much bigger task.
Once the code supports one layout, it can be expanded quite easily.

Blocks are already hardcoding region names, this doesn't change that. See block_theme_initialize().

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

markhalliwell’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs reroll

I'm a little confused. What is this issue attempting to solve? It looks as if this allows a theme to define its regions from a layout and then render the page via the layout template? Is that correct?

tim.plunkett’s picture

This was a bit of exploratory coding to see if we could replace theme regions with a layout-based approach.
Not convinced that it's worth pursuing yet.

markhalliwell’s picture

Oh, awesome! Actually, I think it is. It'd be very nice to be able to use a layout for rendering the entire page.

Especially in base themes like Bootstrap where we have to manually add classes to support the columns. It'd be way easier if we could just use one of our layouts.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

vacho’s picture

Very interesting approach. I re-rolled it.

Manuel Garcia’s picture

Status: Needs work » Needs review
Manuel Garcia’s picture

Re #32:

Not convinced that it's worth pursuing yet.

Curious what you mean by "yet" - what (in your opinion) needs to happen before pursuing this would be worth it?

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

jhedstrom’s picture

Status: Needs review » Needs work

Setting to NW for the issue summary update to clarify what the approach is here.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned
Wim Leers’s picture

Almost 5 years after @jhedstrom in #40, the empty issue summary makes it difficult to understand what the intent is.

@tim.plunkett in #7 and @tedbow in #8 both indicated they see little value.

So … should we close this now? Is #1787942: Allow assigning layouts to pages perhaps the spiritual successor?