Add layout template demonstration

1. The design renames layouts to *templates* and uses layouts for populated templates...

Yes. My strong feeling here is that we need to step back on teminology. I think this nomenclature discussion that has gone around in circles is neatly solved for both the themer and the site builder by this change. Themers will expect a template to map to a twig template, which will be the case; site builders will expect a layout to be the place where they "lay out" the blocks on their pages.

2. The design puts the template listing to the Appearance area...

Changed in IA based on Lisa's study.

3. The listing page does not have any of the fancy checkboxes since there are no operations to do...

Agreed. Updating the design.

4-8 I can't answer

9. I added some v0.1 CSS...

I will send you over all the attributes from the design.

Overall: yes, I'd want to see this happen in D8.

I'm unsure about which level of feedback is most constructive at the moment but here goes:

1. I'm not clear yet on the distinction between templates and layouts. Are templates the 'object' and layouts 'instances' of an object? Why are these seperate things? Do they need equal emphasis in how discoverable they are in the admin IA?

The listing

2. Gabor's number 4: 'Source' column, necessary to know this for whom? I would think it's not so important to have it shown on the overview page.
3. I would prefer to see thumnails of the templates' structure instead (similar to the ones you see in panels when choosing a layout)

The detail page

4. I'm assuming this is purely scaffolding right now, since it doesn't look I can do anything here at this stage :)

---

Gabor: Which of your questions need feedback/answers most in order to proceed constructively?

1. Glad to hear I'm not alone in this. Sad to hear I'm not alone in this :-). My naming scheme based on your graphic would now be:

Layout (amount and stacking of regions) -> Master page (or Template) -> Page
So, Layout > Master page > Page, or even
Template > Master page > Page

I imagine that permissions would let you protect editors from having to mess with the first 'layout' layer, leaving them with a Master page from which to create individual pages. Look at powerpoint and indesign for example, they both have this concept of Master pages.

Hmmm.

Of all the possible words to use in this context, I feel 'Layout' is the weakest when used as a noun. A certain layout is more the result from all the work you put into a creating a page then one of the ingredients you added in doing so.

Template > Master page > Page

I'm not fond of the 'landing page' either, it has quite different meanings depending who's talking about them. I understand this comes from the choice to rebrand 'creating content' as 'creating pages', which I'm also very sceptical about.

So… my main points would be:

- Try to avoid having to use 'Layout' as a noun in this triad
- Consider introducing 'Master pages'
- 'Landing pages' is iffy and seems a work-around label because 'page' is given away too early in the creatin stack (replacing 'content')

You create content, it lives on a page, which is based on a master page which is derived from a template.

Hi Roy,

Thanks for the feedback. Adding the master page is exactly what we are planning for phase 2 of this once we have built out the ability to create one off landing pages. I think there is still some misunderstanding about how this all works though so I have attached a diagram to illustrate how I understand it.

All the nomenclature is user facing and reflects what is in the design right now.

Diagram

Implementation is clear, except access. Also it's good to have 'configure' link in module's info file.
Help text could be adjusted latter

+++ b/core/modules/layout/layout.module
@@ -6,6 +6,48 @@
+    'page callback' => 'layout_page_view',
+    'page arguments' => array(4),
+    'access callback' => 'layout_user_access',
...
+  return (user_access('administer layouts') && layout_manager()->getDefinition($key));
...
+    'administer layouts' => array(
+      'title' => t('Administer templates'),

Suppose this permission should be "view layout" or "layout overview". This screen is a view only

So it looks ready. Further clean-ups and re-factoring could be done in follow-ups.
There's no data changes introduced so not sure we need tests for this

+++ b/core/modules/layout/layout.admin.cssundefined
@@ -0,0 +1,26 @@
+  background: rgb(224, 224, 224);

maybe background-color?

+++ b/core/modules/layout/layout.admin.cssundefined
@@ -0,0 +1,26 @@
+  background-image: linear-gradient(bottom, rgb(70,70,71) 40%, rgb(91,91,94) 70%, rgb(125,124,125) 88%);
+  background-image: -o-linear-gradient(bottom, rgb(70,70,71) 40%, rgb(91,91,94) 70%, rgb(125,124,125) 88%);
+  background-image: -moz-linear-gradient(bottom, rgb(70,70,71) 40%, rgb(91,91,94) 70%, rgb(125,124,125) 88%);
+  background-image: -webkit-linear-gradient(bottom, rgb(70,70,71) 40%, rgb(91,91,94) 70%, rgb(125,124,125) 88%);
+  background-image: -ms-linear-gradient(bottom, rgb(70,70,71) 40%, rgb(91,91,94) 70%, rgb(125,124,125) 88%);
...
+  background-image: -webkit-gradient(

needs follow-up to discuss styling

+++ b/core/modules/layout/layout.admin.incundefined
@@ -0,0 +1,66 @@
+  $build = array();
+  $build['table'] = array(
+    '#theme' => 'table',
+    '#header' => $header,
+    '#rows' => $rows,
+  );
+  return $build;

useless variable init, no #empty key

+++ b/core/modules/layout/layout.admin.incundefined
@@ -0,0 +1,66 @@
+  t('module');
+  t('theme');

this could be used as array to skip wrong providers in loop above

Just some general feedback:

1. I like Gabor's #5 demo patch.

2. I too dislike use of label Landing Pages in *any* usage here for reasons cited plus I've always just found the term leads to more confusion than clarity.

3. I support Roy's point about matching current usage/terminology in page composition programs like InDesign for consistency but am still thinking there are too many (redundant) levels here, ie we're overcomplicating matters (here I'm referring to tkoleary's 4-stage diagram in #13 above). If dismissing term "Layouts" here is an option, my strongest preference is for Roy's "Template > Master page > Page" setup in #11.

4. Permission should read "View Layouts" (or even better if possible, "View Master Pages" - re #17).

5. I do, very much, wish to see this included in Drupal 8.

6. About the layout/arrangement "icons", Panopoly (http://drupal.org/project/panopoly) has a nice implementation of this idea, I support using these (http://drupal.org/files/images/panoply-icon-smaller-still.png).

7. In general, I support Panopoly's UI setup and the advances in UI that have been made there for inclusion in Drupal 8. I left Denver with rather the impression that we were planning to use it as the base for UI in Drupal 8. Has that plan been scrapped? Please pardon if I've misunderstood the directive.

Thanks for the update, Gábor - much appreciated!
Cheers,
Kelly

Docs cleanups that can also be done in a followup (or a reroll; I added this to the office hours task list).

1. +++ b/core/modules/layout/layout.admin.cssundefined
   @@ -0,0 +1,26 @@
   +.layout-region-demonstration {
   +  color: white;
   +  margin: 3px;
   +  padding: 10px;
   +  text-transform: uppercase;
   +  font-size: 0.8em;
   +
   +  background-image: linear-gradient(bottom, rgb(70,70,71) 40%, rgb(91,91,94) 70%, rgb(125,124,125) 88%);
   +  background-image: -o-linear-gradient(bottom, rgb(70,70,71) 40%, rgb(91,91,94) 70%, rgb(125,124,125) 88%);
   +  background-image: -moz-linear-gradient(bottom, rgb(70,70,71) 40%, rgb(91,91,94) 70%, rgb(125,124,125) 88%);
   +  background-image: -webkit-linear-gradient(bottom, rgb(70,70,71) 40%, rgb(91,91,94) 70%, rgb(125,124,125) 88%);
   +  background-image: -ms-linear-gradient(bottom, rgb(70,70,71) 40%, rgb(91,91,94) 70%, rgb(125,124,125) 88%);
   +
   +  background-image: -webkit-gradient(
   +    linear,
   +    left bottom,
   +    left top,
   +    color-stop(0.4, rgb(70,70,71)),
   +    color-stop(0.7, rgb(91,91,94)),
   +    color-stop(0.88, rgb(125,124,125))
   +  );
   

   I think these properties are in the wrong order. Reference: http://drupal.org/node/302199.

2. +++ b/core/modules/layout/layout.admin.incundefined
   @@ -0,0 +1,66 @@
   + * Page callback: Presents list of layouts.
   

   Should have an article ("Presents a list of layouts").

3. +++ b/core/modules/layout/layout.admin.incundefined
   @@ -0,0 +1,66 @@
   + * Page callback: Demonstrate a layout template.
   
   +++ b/core/modules/layout/lib/Drupal/layout/Tests/LayoutDerivativesTest.phpundefined
   @@ -76,6 +76,19 @@ function testDerivatives() {
   +   * Render the layout with sample region content.
   

   Demonstrates, Renders. :) Reference: http://drupal.org/node/1354#functions

4. +++ b/core/modules/layout/layout.moduleundefined
   @@ -6,6 +6,48 @@
   + * Menu access callback. Check for layout existance.
   + */
   +function layout_user_access($key) {
   

   "Existence" is misspelled and the summary format is nonstandard. Should be:
   Access callback: Checks the existence of a layout.
   Also, the $key parameter needs to be documented. Reference: http://drupal.org/node/1354#menu-callback

5. +++ b/core/modules/layout/lib/Drupal/layout/Plugin/LayoutInterface.phpundefined
   @@ -18,15 +18,24 @@
   +   * @param boolean $admin
   +   *   TRUE if the rendered layout is displayed in an administrative context,
   +   *   FALSE otherwise.
   +   * @param array $regions
   +   *   An array of region render arrays keyed by region machine names.
   ...
   +  public function renderLayout($admin = FALSE, $regions = array());
   

   It looks like these parameters are optional, so they should have (optional) at the beginning of the description. Also, it should be bool (not boolean). Reference: http://drupal.org/node/1354#param-return-data-type

docs clean up from #25, rerolled against #21.

adding an interdiff.txt for #26

Excellent, thanks @bdone! This looks great. I gave it another read and caught some undocumented parameters, plus a couple of minor corrections:

1. +++ b/core/modules/layout/layout.admin.incundefined
   @@ -0,0 +1,66 @@
   +/**
   + * Page callback: Demonstrates a layout template.
   + *
   + * @see layout_menu()
   + */
   +function layout_page_view($key) {
   

   It looks like $key and the return value are also undocumented here, so can we add documentation for it too?

2. +++ b/core/modules/layout/layout.moduleundefined
   @@ -6,6 +6,55 @@
   + * @return
   + *   TRUE if the current user can access page layout menu items; FALSE
   + *   otherwise.
   

   This should be @return bool to document the data type.

3. +++ b/core/modules/layout/lib/Drupal/layout/Plugin/layout/layout/StaticLayout.phpundefined
   @@ -87,12 +87,10 @@ public function renderLayout($admin = FALSE) {
   -    // Render all regions needed for this layout.
   +    // Renders all regions needed for this layout.
   
   +++ b/core/modules/layout/lib/Drupal/layout/Tests/LayoutDerivativesTest.phpundefined
   @@ -76,6 +76,19 @@ function testDerivatives() {
      /**
   +   * Render the layout with sample region content.
   +   */
   +  function renderLayoutDemonstration($layout) {
   

   Oops, I should have included more context here. It's the docblock that needs to be "Renders" (as in "This function renders"). For inline comments, it's still "render" as in "do this now".

   I also notice now that the method is missing its @param and @return documentation.

parameter documentation and minor fixes, per @xjm's comments in #28.

Thanks @bdone! RTBC again.

Awesome, thanks!

We're up over thresholds, but I plan to commit this the second we're under again, since it's been hanging around for quite some time. :)

We are kinda under, the are like 5 tasks which are changelog :D

We're still over, but it's close enough for government work atm.

Committed and pushed to 8.x. Thanks! And thanks to bdone for the docs improvements, too! :)

Tagging with "revisit before release"... if it turns out the other blocks and layouts work doesn't land (it's been stalled for quite awhile, which makes this unfortunately likely... :\) we'll need to rip this out.

Add meta issue link.

#2053879: Remove layout.module from core.