Problem/Motivation

We had to abandon some of the designs (particularly sections that were laid-out side-by-side, such as 'Recipe Ingredients' alongside 'Recipe Instructions'). The reason we had to abandon this was so we could continue to use the 'Manage Display' page settings, rather than hardcoding fields into templates.

Proposed resolution

Layout Builder is nearly stable (hopefully it will be for Drupal 8.7). Once this is stable, we should use it so we can be more faithful to the original designs by @kjay

Remaining tasks

  • Enable Layout Builder module
  • Set the 'Full' view mode for recipes to use layout builder templates for rendering content.
  • Make sure that the site still looks like it currently does, no regresssions.

There is a follow-up issue to have this view mode then look like the original designs.
#3027616: Style Recipes in Full View Mode to Look Like the Original Designs

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#28 Screenshot_2019-03-29 Edit layout for Recipe content items Site-Install(1).png80.7 KBshaal
#28 Screenshot_2019-03-29 Manage display Site-Install.png44.49 KBshaal
#27 3027614-26.patch12.86 KBmarkconroy
#27 interdiff-25-26.txt589 bytesmarkconroy
#25 interdiff-19-25.txt533 bytesmarkconroy
#25 3027614-25.patch12.82 KBmarkconroy
#24 Screenshot 2019-03-28 at 22.01.10.png147.98 KBGábor Hojtsy
#21 no-layout-enabled-in-recipes.png136.71 KBshaal
#19 Screenshot 2019-03-28 at 19.49.30.png432.01 KBmarkconroy
#19 interdiff-11-19.txt4.79 KBmarkconroy
#19 3027614-19.patch12.14 KBmarkconroy
#12 d87x_before3027614_uid1_settings.png129.35 KBtatarbj
#12 d87x_after3027614_uid1_modules.png74.26 KBtatarbj
#12 d87x_after3027614_uid0.png417.35 KBtatarbj
#12 d87x_before3027614_uid1_settings.png129.35 KBtatarbj
#12 d87x_before3027614_uid1_modules.png74.17 KBtatarbj
#12 d87x_before3027614_uid0.png403.04 KBtatarbj
#11 3027614-11.patch7.15 KBBrightBold
#9 umami-recipe-with-layout-builder.png3.95 MBBrightBold
#9 umami-recipe-no-layout-builder.png3.95 MBBrightBold
#7 3027614-7.patch7.22 KBmarkconroy
#6 3027614-6.patch560.9 KBmarkconroy
#5 Recipe-page-with-layout-builder-fullpage.png5.08 MBDenes.Szabo
#4 Recipe-page-before-layout-builder-fullpage.png5.08 MBDenes.Szabo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markconroy created an issue. See original summary.

markconroy’s picture

Issue summary: View changes
Denes.Szabo’s picture

Assigned: Unassigned » Denes.Szabo
Status: Active » Needs work

I just tested it manually that is described under the remaining tasks. Everything seems fine.

I will attach few screenshot of a recipe page that proves the layout builder works as expected.

Next steps should be to enable the layout builder module as dependency of the Umami demo and find out the way to to set recipe's full display mode use layout builder from install configuration. If I get it well the new dependency should be added to the info YML.

Denes.Szabo’s picture

Screenshot of a recipe page without using Layout Builder.

Denes.Szabo’s picture

Screenshot of a recipe page using Layout Builder.

markconroy’s picture

FileSize
560.9 KB

Thanks for checking that @Denes.Szabo.

I've now created a patch for us that:

1) Enables layout builder (and layout discovery) modules
2) Sets Layout Builder for the layout of the 'Full' view mode for recipes

I see no regressions, so if you'd like to review this and mark it RTBC if you are happy with it, that would be great. We can then start to work on #3027616: Style Recipes in Full View Mode to Look Like the Original Designs.

Thanks again.

markconroy’s picture

Status: Needs work » Needs review
FileSize
7.22 KB

Sorry, looks like I uploaded the wrong patch.

This one should be working.

Status: Needs review » Needs work

The last submitted patch, 7: 3027614-7.patch, failed testing. View results

BrightBold’s picture

I don't know why the tests are failing, but I can confirm that the patch applies cleanly, that with the patch applied Layout Builder is enabled and Recipes in Full view mode are set to use it, and that (as @Denes.Szabo confirmed by making the changes manually), recipe full page views are identical before and after the patch.

BEFORE:
Umami recipe page before Layout Builder

AFTER:
Umami recipe page with Layout Builder in use

tim.plunkett’s picture

+++ b/core/profiles/demo_umami/config/install/core.entity_view_display.node.recipe.full.yml
@@ -16,10 +16,184 @@ dependencies:
+_core:
+  default_config_hash: Iwtr5kvAlrb66bPfLrOQhHDhH5a5g8-PICP0_MUQugY

This part must be removed, then tests should pass.

BrightBold’s picture

Status: Needs work » Needs review
FileSize
7.15 KB

Thanks Tim! Here goes...

tatarbj’s picture

Assigned: Denes.Szabo » Unassigned
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
403.04 KB
74.17 KB
129.35 KB
417.35 KB
74.26 KB
129.35 KB

Performed manual tests confirm it works well, here are the steps:

  • Drupal 8.7.x site (without #11 patch)

    8.7.x code is cloned, composer install, site is manually installed from the UI, chosing demo umami as profile.
    Site is successfully installed and the recipe node (/recipes/vegan-chocolate-and-nut-brownies) shows the attached screen d87x_before3027614_uid0.png as Anonymous sees it.
    You can see how admin/modules pages look like to see none of the Layout related modules are enabled on d87x_before3027614_uid1_modules.png; and the settings of recipe content-type's full view is the classic drupal one on d87x_before3027614_uid1_settings.png.

  • Drupal 8.7.x site (with #11 patch)

    8.7.x code is cloned, patch is applied successfully, composer install, site is manually installed from the UI, chosing demo umami as profile.
    Site is successfully installed and the recipe node (/recipes/vegan-chocolate-and-nut-brownies) shows the attached screen on d87x_after3027614_uid0.png as Anonymous sees it.
    You can see how admin/modules pages look like to see none of the Layout related modules are enabled on d87x_after3027614_uid1_modules.png; and the settings of recipe content-type's full view is the classic drupal one on d87x_after3027614_uid1_settings.png.

Note: Few pictures are not shown as files are not correctly installed, but as it's on both installation, we should ignore it as false positive in this scope.

The attached screens show the #11 resolves the issue correctly. I've also updated the remaining tasks as all of them are resolved now.

Bests,
Balazs.

Eli-T’s picture

Eli-T’s picture

Status: Reviewed & tested by the community » Postponed

Postponing as although RTBC, do not want to risk committing without Layout Builder being stable. Awesome to have this ready to go when that happens!

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.

Gábor Hojtsy’s picture

Status: Postponed » Needs review

Sending for retest now.

Gábor Hojtsy’s picture

I see layout builder is being used but the template is not removed. Is that so to keep the template around in case people disable layout builder? Or should we remove the template since it now works with layout builder?

Gábor Hojtsy’s picture

Speaking of this one: https://cgit.drupalcode.org/drupal/tree/core/profiles/demo_umami/themes/... is that still being used with layout builder?

markconroy’s picture

It looks like we can remove the node--recipe--full.html.twig template, as it's only really being used to create a .node__header .page-title class, but this is also available in the node.html.twig file, so it gets picked up from there not. Actually, I deleted that file just to test before applying the patch and the recipe full view mode was still perfect, so looks like we can remove that file whether we commit this patch or not.

Patch and screenshot of before and after attached (I bet you can't guess which is which).

before/after node--recipe--full.html.twig in demo_umami

Status: Needs review » Needs work

The last submitted patch, 19: 3027614-19.patch, failed testing. View results

shaal’s picture

I tested #19
It looks like recipe content type doesn't have Layout enablad.

(But if I enable layout manually for that content type - it looks great!)

Gábor Hojtsy’s picture

That would need to be in the patch then.

markconroy’s picture

Status: Needs work » Needs review

@shaal

Correct, that's default view mode. We are only using Layout Builder for Full content

Gábor Hojtsy’s picture

Issue summary: View changes
FileSize
147.98 KB

True, we should not enable it for default. I tried this out and the styling of the layout builder display / help needs some work. It appears much better with the same margins, etc. in bartik.

markconroy’s picture

That’s the ‘highlighted’ region, it doesn't really have anything to do with Layout Builder per se.

Here's a new patch to add our standard container class around it - perhaps we can make it even nice in a new follow-up.

Status: Needs review » Needs work

The last submitted patch, 25: 3027614-25.patch, failed testing. View results

markconroy’s picture

Status: Needs work » Needs review
FileSize
589 bytes
12.86 KB

New patch, which should stop the failing tests.

shaal’s picture

Version: 8.8.x-dev » 8.7.x-dev
Status: Needs review » Reviewed & tested by the community
FileSize
44.49 KB
80.7 KB

Layout builder is enabaled for Full content view-mode of the recipe content type.

It is possible to edit the page layout, and change the positions of various elements in the page.
#24 was also fixed, and now the help message is displayed with the correct margin.

Great work!

Gábor Hojtsy’s picture

Title: Use Layout Builder for the Recipe Content Type in Full View Mode » Use Layout Builder in Umami for the recipe content type in full view mode

Assigning credits.

  • Gábor Hojtsy committed 84880d9 on 8.8.x
    Issue #3027614 by markconroy, BrightBold, tatarbj, shaal, Denes.Szabo,...

  • Gábor Hojtsy committed 03653af on 8.7.x
    Issue #3027614 by markconroy, BrightBold, tatarbj, shaal, Denes.Szabo,...
Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks all, committed and will hopefully be included in 8.7.0-beta1.

Gábor Hojtsy’s picture

Followups!

@markconroy opened #3044250: Make the highlighted region in Umami more flexible so it adapts to its use with Layout Builder and could be used to demonstrate placement of other blocks yesterday, we also need a followup for styling the layout builder components, particularly the checkbox, that is quite unfortunately placed/styled also.

tim.plunkett’s picture

When working on any custom Layout Builder styling, please consider helping expand the handbook docs for how to theme the Layout Builder UI: https://www.drupal.org/docs/8/core/modules/layout-builder/theming-and-ex...

Additionally, there is a known bug when using Full Content view mode, Umami should be aware of that: #2955065: Customized layout does not display for node with customizations enabled when using full content view mode

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.