Problem/Motivation

#2988970: Layout Builder should make it easier to modify the default layout for an entity type when viewing an entity adds a new message to Layout Builder that is incompatible with Umami's markup (because it differs from Classy)
It contains a workaround since Layout Builder cannot directly target Umami without also affecting every other theme.

Proposed resolution

Fix the styling and remove the workaround

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#12 layout-builder-3043228-12-message--defaults.png72.94 KBshaal
#12 layout-builder-3043228-12-message--overrides.png89.74 KBshaal
#12 recipe-default-view-mode-layout-builder.png11.99 KBshaal
#12 interdiff_11-12.txt957 bytesshaal
#12 ootb-layout-builder-messages-styling-3043228-12.patch2.68 KBshaal
#11 interdiff_6-11.txt523 bytesshaal
#11 ootb-layout-builder-messages-styling-3043228-11.patch2.63 KBshaal
#11 layout-builder-3043228-11-IE11.png119.86 KBshaal
#6 Screen Shot 2019-04-04 at 06.34.06.png68.49 KBkjay
#6 Screen Shot 2019-04-04 at 06.24.43.png87.37 KBkjay
#6 interdiff-3043228-3-6.txt982 byteskjay
#6 drupal_umami-layout-builder-messsage-3043228-6.patch2.5 KBkjay
#3 override.png55.32 KBbnjmnm
#3 default.png22.68 KBbnjmnm
#3 3043228-3.patch2.56 KBbnjmnm

Comments

tim.plunkett created an issue. See original summary.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Component: layout_builder.module » Umami demo
Status: Postponed » Active
Issue tags: +Out of the Box Initiative
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Active » Needs review
StatusFileSize
new 3043228-3.patch2.56 KB
new default.png22.68 KB
new override.png55.32 KB

Patch + screenshots

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented 
+++ b/core/profiles/demo_umami/themes/umami/css/components/layout_builder/layout-builder.css
@@ -0,0 +1,11 @@
+  background-position: 18px;

If this is the right patch, we need an RTL version for this I think(?)

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
+++ b/core/profiles/demo_umami/themes/umami/css/components/layout_builder/layout-builder.css
@@ -0,0 +1,11 @@
+/* Remove the default "check" background image. */
+.layout-builder__message .messages .messages__content {
+  background: none;
+}
...
+  background-position: 18px;
+  background-size: 16px;

I would expect this to unset the `background` set for .messages by Layout Builder and then to add the `background-image` directly to .layout-builder__message--defaults .messages__content within Umami.

Then there's no need for the background-position change

kjay’s picture

kjay commented
StatusFileSize
new drupal_umami-layout-builder-messsage-3043228-6.patch2.5 KB
new interdiff-3043228-3-6.txt982 bytes
new Screen Shot 2019-04-04 at 06.24.43.png87.37 KB
new Screen Shot 2019-04-04 at 06.34.06.png68.49 KB

As per the review by @tim.plunkett, here's a patch that updates the patch in #3 to unset the default background image for status messages and applies the same globe icon from core using the inner content wrapper, as we do across Umami.

Also added the standard opening comment we have been using for each stylesheet.

Looks like the following when applied. Once #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 gets in, the message will include margin bottom and not be positioned up against the layout builder area.

Fix for globe icon positioning on layout builder

Fix for globe icon positioning on layout builder

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

@kjay: are the buttons smashing into each other in RTL a pre-existing problem or introduced here?

shaal’s picture

shaal
he/him
Primary language English
Location Boca Raton, FL
commented

@gaborhojtsy I think it is because of missing css rules in Umami, which is resolved in a separate issue -
https://www.drupal.org/project/drupal/issues/3044366#comment-13053807

kjay’s picture

kjay commented

@gabor, Yes, as @shaal says and has shared the link to the issue that fixes the button layout.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work

There are two icons, one for defaults and one for overrides. This patch makes both use the one for defaults

shaal’s picture

shaal
he/him
Primary language English
Location Boca Raton, FL
commented
Version: 8.8.x-dev » 8.7.x-dev
Status: Needs work » Needs review
StatusFileSize
new layout-builder-3043228-11-IE11.png119.86 KB
new ootb-layout-builder-messages-styling-3043228-11.patch2.63 KB
new interdiff_6-11.txt523 bytes

I added the missing icon mentioned in #10

I tested the patch on IE11 + RTL (works great)

shaal’s picture

shaal
he/him
Primary language English
Location Boca Raton, FL
commented
Issue summary: View changes
StatusFileSize
new ootb-layout-builder-messages-styling-3043228-12.patch2.68 KB
new interdiff_11-12.txt957 bytes
new recipe-default-view-mode-layout-builder.png11.99 KB
new layout-builder-3043228-12-message--overrides.png89.74 KB
new layout-builder-3043228-12-message--defaults.png72.94 KB

Oops. I missed the overrides' icon override :)

In order to see the Layout Builder's override-icon:
Go to:
Structure > Content Types > Recipe > Manage Display > Default (view-mode)
Enable:
Use Layout Builder, Allow each content item to have its layout customized

This patch is fixing it (with screenshots as a proof)

Icon of message-defaults:

Icon of message-overrides:

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community

Thanks! This works as expected, and the patch is correct

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Version: 8.7.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Needs work

Does not apply on 8.8.x unfortunately.

  • Gábor Hojtsy committed 060c85c on 8.8.x 
    Issue #3043228 by shaal, kjay, bnjmnm, tim.plunkett, Gábor Hojtsy: Add...

  • Gábor Hojtsy committed db21b29 on 8.7.x 
    Issue #3043228 by shaal, kjay, bnjmnm, tim.plunkett, Gábor Hojtsy: Add...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Version: 8.8.x-dev » 8.7.x-dev
Status: Needs work » Fixed

I opted to fix that locally, it was a line offset problem in the libraries.yml only. Thanks all.

Status: Fixed » Closed (fixed)

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