Add page listing the available layouts on the site

Please provide patch to apply to current code base, it will be easy to test/review

PS https://www.drupal.org/patch/submit

Here's that exact same code in the form of a patch, in the hope of getting the ball rolling =)

+++ b/modules/layout_plugin_ui/layout_plugin_ui.module
@@ -0,0 +1,19 @@
+function layout_plugin_ui_theme() {
+  return [
+    'layout_plugin_ui' => [
+      'variables' => ['layouts' => NULL],
...
+function layout_plugin_ui_preprocess_layout_plugin_ui(&$variables) {
+  $variables['base_path'] = base_path();

+++ b/modules/layout_plugin_ui/templates/layout-plugin-ui.html.twig
@@ -0,0 +1,18 @@
+      <div class="image">
+        <img src="{{ base_path ~ layout.icon }}" alt="{{ region.label }}" />
...
+      <div class="regions">
+        <ul>
+          {% for region in layout.regions %}
+            <li>{{ region.label }}</li>

I think that basically could be just a build array, no reason for special theme function

How about just making this a 'report' page?

Hm. layout_plugin is in core now, but this will be introducing something that isn't in core. Does this make sense?

If not we might want to move it to e.g. the Devel module as a central place for developer-related things.

Moving to the Devel module.

For starters, any new submodule in Devel needs a maintainer who commits to managing the issue queue for at least a year. Please volunteer here.

Hi guys, thanks for contributing!
If we want really add this functionality to devel I see no reason for creating a new module in the devel suite.. we could simply create a controller and register its route using _module_dependencies. I'm missing something?
also, since #2296423: Implement layout plugin type in core why we should integrate the layout plugin contrib module and not the layout discovery module shipped with core?

Quick drive by review, patch applies fine, enabled it and checked the permission page which has a problem:

+++ b/layout_ui/layout_ui.info.yml
@@ -0,0 +1,8 @@
+description: 'Provides a list of all registred layouts.'

+++ b/layout_ui/layout_ui.links.menu.yml
@@ -0,0 +1,5 @@
+  description: 'List of all registred layouts.'

+++ b/layout_ui/layout_ui.permissions.yml
@@ -0,0 +1,3 @@
+  description: 'Access registred layouts list overview.'

Typo here registred -> registered

Also I think the layout page itself looks empty of information on several columns, perhaps there are other bugs I didn't dig into...

The attached gets the regions' labels in place.

Core layouts have no description apparently, or at least they come as NULL, and the same goes for the icons paths.

I'm thinking it'd be also be useful to add a new column showing what provides each layout ($layout->getProvider()).

+1 to #20

Tackling the typos on the permission, and adding the provider column.

someone can answer to #14? :)

If we want really add this functionality to devel I see no reason for creating a new module in the devel suite.. we could simply create a controller and register its route using _module_dependencies. I'm missing something?

Imo that should be just a controller and /admin/reports/... route

NW for early render()

1. +++ b/layout_ui/layout_ui.info.yml
   @@ -0,0 +1,8 @@
   +dependencies:
   +  - layout_discovery
   

   should be `drupal:layout_discovery`

2. +++ b/layout_ui/layout_ui.links.menu.yml
   @@ -0,0 +1,5 @@
   +  route_name: layout_ui.admin
   ...
   +  parent: system.admin_config_development
   
   +++ b/layout_ui/layout_ui.permissions.yml
   @@ -0,0 +1,3 @@
   +access layouts overview:
   

   it looks exactly like "report"

3. +++ b/layout_ui/src/Controller/LayoutUiController.php
   @@ -0,0 +1,74 @@
   +        render($image),
   

   early render??

Thanks guys for the reviews, I think #14 makes sense btw. Working on this today.

OK here is a different path, getting this into devel itself.

• Route set to be part of admin/reports, with _module_dependencies: 'layout_discovery'
• Cleaned up a bit some of the comments.
• Renamed the method providing the page to follow a bit whats done in others in devel.
• Changed the icon to be an image render array instead of manually creating the image tag #markup and rendering it. Since core layouts currently provide no icons, I tested this using a hard-coded image, and it works.
• Changed the text when there are no layouts from 'No layout' to 'No layouts available'.

Pls ignore patch devel-2786541-26.patch :)

About the layout icons... see #2660124: Dynamically build layout icons based on well formed config

Nice, imo rtbc but I'd like to see todo in code pointing to related

Good call @andypost - adding the @todo and fixing the comment on LayoutInfoController::layoutInfoPage().

And propably we need a basic test coverage for this new functionality! ;)

@willzyx you're right of course :)

Here's #32 plus the test checking that the page is accessible, that the title is there and the column names as well.
Not sure we should be checking the contents of the table itself, since layout_discovery should be testing its layotus anyway...

Thanx! Imo good to go

Thanks @romainj you're absolutely right, having a look at this...

OK, we cant use _module_dependencies on the menu yml file, but we can make use of hook_menu_links_discovered_alter() for this purpose. Attached patch does just that. Also it'd be great if core took care of this for us :)

As a side note, I have added tests for the other pages in the hope that those would prevent us nearly breaking all things in the future like with this patch... have a look #2876073: Expand test coverage for page controllers :)

Ends up devel already has tests that cover those pages, so not sure why the patches before #38 were coming back green... anyway, we should be in the clear now.

@Manuel Garcia great work. A quick review:

1. +++ b/devel.module
   @@ -105,6 +105,21 @@ function devel_toolbar() {
   +    $links['devel.layout_info_page'] = [
   +      'title' => new \Drupal\Core\StringTranslation\TranslatableMarkup('Layouts Info'),
   +      'parent' => 'system.admin_reports',
   +      'route_name' => 'devel.layout_info',
   +      'description' => new \Drupal\Core\StringTranslation\TranslatableMarkup('Overview of layouts available to the site.'),
   +    ];
   

   Can we import TranslatableMarkup class instead of using FQNS?

2. +++ b/devel.routing.yml
   @@ -242,3 +242,15 @@ devel.event_info:
   +devel.layout_info:
   ...
   +  defaults:
   +    _controller: '\Drupal\devel\Controller\LayoutInfoController::layoutInfoPage'
   +    _title: 'Layouts'
   +  options:
   +    _admin_route: TRUE
   +  requirements:
   +    _permission: 'access devel information'
   +    _module_dependencies: 'layout_discovery'
   

   Now that the functionality is integrated in the devel module can the menu link be placed in devel menu? also is there a strong motivation to expose it in report section?

3. +++ b/src/Controller/LayoutInfoController.php
   @@ -0,0 +1,85 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function __construct(LayoutPluginManagerInterface $pluginManagerLayout) {
   +    $this->layoutPluginManager = $pluginManagerLayout;
   +  }
   

   Missing constructor docblock. @inheritdoc is not applicable here :p

4. +++ b/src/Tests/LayoutInfoControllerTest.php
   @@ -0,0 +1,46 @@
   +<?php
   +
   +namespace Drupal\devel\Tests;
   +
   +use Drupal\simpletest\WebTestBase;
   +
   +/**
   + * Tests Layout Info controller.
   + *
   + * @group devel
   + */
   +class LayoutInfoControllerTest extends WebTestBase {
   +
   +  /**
   +   * Modules to enable.
   +   *
   +   * @var array
   +   */
   +  public static $modules = array('devel', 'layout_discovery');
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +  protected function setUp() {
   +    parent::setUp();
   +
   +    $web_user = $this->drupalCreateUser(array(
   +      'access devel information',
   +    ));
   +    $this->drupalLogin($web_user);
   +  }
   +
   +  function testLayoutInfoPage() {
   +    // Test Devel load and render routes for the Layout Info page.
   +    $this->drupalGet('admin/reports/layouts');
   +    $this->assertResponse(200);
   +    $this->assertText('Layouts', 'Layouts title is present');
   +    $this->assertText('Icon', 'Icon column header is present');
   +    $this->assertText('Label', 'Label column header is present');
   +    $this->assertText('Description', 'Description column header is present');
   +    $this->assertText('Category', 'Category column header is present');
   +    $this->assertText('Regions', 'Regions column header is present');
   +    $this->assertText('Provider', 'Provider column header is present');
   +  }
   +
   +}
    
   

   can we move the test to BTB?
   I think that It might be worth to expalnd a little the test whit some basic checks like:
   - is the page accesible for unprivileged user?
   - is the menu link hidden when layout_discovery is not enabled and visible otherwise?
   - what appen when layout_discovery is not enabled? devel works properly?

   You can find some inspiration in the other devel functional tests here ;)

Thanks @willzyx for the review! A little progress on that:

• #41.1 Yes. Done.
• #41.2 You're right, it would be strange to have just this page on the reports section, unless we moved the rest of the info pages there as well. Moved it into the devel menu on this patch (only shows on vertical toolbar unless you enable it).
• #41.3 Oops! Done.

Still to do: #41.4

Doing #41.4 and removing the old test.

I did expand on what we had before, so on top of what we had we now also check that:

• each layout is printed
• each value in its proper column
• the page is only accessible to users with the right permission
• the menu link appears where it should, with the right url and page title

Scenarios not tested yet:

• is the menu link hidden when layout_discovery is not enabled and visible otherwise?
• what appen when layout_discovery is not enabled? devel works properly?

I have also added a small paragraph with help text for the page.

I thought about adding the search like on the other info pages, but not sure its necessary on this case.. will there be loads of layouts on sites? We could hold this back until people request it perhaps.

Adding tests for when the layout_discovery module is not installed.

• Checking that the menu link is not there.
• Checking that the page returns 404
• Checking that few other devel paths should return 200.

Committed and pushed to 8.x. Thanks @romainj @Manuel Garcia and all the others!

Brilliant, thanks!!

Seems that DevelLayoutInfoTest cause test failures in D 8.1 and 8.2 due to missing dependency layout_discovery :(
https://www.drupal.org/pift-ci-job/671024
https://www.drupal.org/pift-ci-job/671025

I'm not aware of a clean solution for solving the tests failure.. Something like the attached patch could works
Any help/suggestion is really appreciated :)

Oh wow, did not think of that!

Patch looks like it makes sense, though I haven't seen anything else in the wild trying to do this...

ok I haven't found clean solution for solving the tests failure :( .. for now the solution in #52 could works. Added a todo for this
the attached patch also contains some minor fix to tests