Fix styling of Umami for layout builder

There is also #3043228: Add Umami-specific styling for Layout Builder messages which has an in-code @todo.

The patch in #4 applies cleanly. We don't have any particularly good classes to use to target the checkbox element and so the selector is working around this. I don't know if that's going to be fragile but it does work. The form is displayed with a 1em margin between the 'Save Layout' button and the 'Show content preview' checkbox as desired.

There is missing RTL styles for this but they seem to be missing down at our base styles for button. Attached patch addresses this and looks like the screenshot attached when applied.

Patch#8 working fine both in LTR and RTL as per attached screenshots.

Moving this to RTBC.

I removed the layout-builder specific CSS on this patch and created a solution with flexbox and negative margins.

Patch #8 had an issue with mobile display:

The current patch has a fix for mobile (flex-wrap) and doesn't require a special RTL rule (works the same for both directions)

I added a preview image for #12)

@shall
Thanks for the nice work!

I reviewed your patch#12 and yes it resolved the issue for both desktop and for mobile in the small resolution. Good catch on this. ;)

Let's wait for others to share their feedback on this and then we can move this to RTBC.

There seems to be a regression on our main search form caused by this patch for me in Firefox. I'm just heading to the office and will pick this up as soon as I get there if someone else can't get there first.

Having taken another look at this. I don't think that #12 is the right approach and as per the screenshot, introduces new theme-wide implications. #8 fixes this specific problem of this issue, but suffers from having a nasty selector.

#12 is adding left/right margin to fix the problem of the components touching the left / right edge of the viewport but I believe that is a much broader issue for Umami. Probably a follow up issue if it doesn't relate to #3001660: Fix Umami's responsive layout styles.

By the way, the same issue at smaller screen sizes exists for the layout builder interface itself as per @shaal's screenshots above.

I think #8 could be the 'quick fix' for this if we can accept bigger issues to resolve at smaller screen sizes?

The attached patch will certainly need a little improvement but this approach could resolve this issue so long as the tricky class selector for targeting the form is acceptable.

The idea is to include a margin on the left and right for the entire layout builder form when the form is less than 1200px.

Thank you @kjay, #17 is a great improvement.

The only problems I found is that buttons are pushed from the sides too much on smaller screens, and the page title is missing paddings on smaller screens.

I tested it on chrome, IE11, RTL, mobile.

Mobile RTL:

IE11 RTL:

Great stuff. Patch applies cleanly and the extra padding is now removed when viewing the form elements on smaller viewports.

@smaz commented on Slack about some missing RTL rules.

Looking into that I realized we have rules that are not affecting anything
I also found out that we're missing other more generic rules for forms that will affect both contact page's form and layout-builder's form.

Tested it on mobile + RTL, and paid attention to both layout-builder + search appearance.

I have reviewed patch#20 and found no issue in terms of the search appearance and layout builder which was their earlier.

I have verified this test on desktop, mobile and IE11 browsers and patch looks good as per the attached screenshots.

Moving this to RTBC.

Media random fails

Following up on our discussion in today's OOTB call, we are discussing the use of the following selector:

[class$="layout-builder-form"]

This selector is inherited from the patch in #4 and we thought it may be possible to simplify this by using the full class name 'entity-view-display-layout-builder-form' instead.

@bnjmnm, could you confirm the reason for building the selector as '[class$="layout-builder-form"]'? Leaving as RTBC since the patch works and there may be no reason to change anything.

Per-entity overrides (like /node/1/layout) will also have this element, but the selector is entity type dependent. So, node-layout-builder-form or user-layout-builder-form, etc.

@tim.plunkett

In that case, might it be better to provide a class and a modifier in the layout-builder? Like so:

layout-builder-form layout-builder-form--node and layout-builder-form layout-builder-form--user etc?

Do any of the latest comments from #25 through to #27 mean that we need to make changes to this patch? It is unclear.

yarn run lint:css
yarn run v1.13.0
$ stylelint "**/*.css"

profiles/demo_umami/themes/umami/css/components/layout_builder/layout-builder.css
 19:3   ✖  Expected "margin-right" to come before "margin-left"      order/properties-order
 24:19  ✖  Unexpected unit                                           length-zero-no-unit
 25:5   ✖  Expected "margin-right" to come before "margin-left"      order/properties-order
 25:20  ✖  Unexpected unit                                           length-zero-no-unit

Some CSS code style things to fix. If you apply the patch and run yarn run lint:css --fix from the core directory it should fix the patch. I've done that...

We can't stop node-layout-builder-form or user-layout-builder-form from being available, since that is the HTML class equivalent of the form ID.

These two forms (DefaultsEntityForm and OverridesEntityForm) *can* be rewritten to include some #attributes to contain layout-builder-form as a class.

I have tested the patch in #20 and discussed its approach with @shaal in Slack and I believe it is good to go so long as the [class$="layout-builder-form"] selector is ok to use.

The patch provides the following:

• For both LTR and RTL, introduces an inline layout for the form actions that includes even margin between each.
• Adjusts search form and contact form to support these new form action layout styles.
• Collapses form elements down to the smallest screen sizes.
• Introduces a left and right page margin across smaller screen sizes for the layout builder form.
• Adds padding to the page title to prevent the title from being edge to edge on smaller screens when used with layout builder.

+1 for RTBC.

Can we RTBC patch #20 ?
Are there any other concerns?

This implements #29

This is looking good to me, but it would be nice if we could fix the + icon not lining up with the add links:

After a quick chat with Shaal & tim.plunkett, the icon issue should be fixed in Layout Builder & not Umami, which makes it out of scope for this issue.

The rest all looks ok, so RTBC!

Adding related issue.

1. 
   It seems like there's a regression in the search form.

2. +++ b/core/profiles/demo_umami/themes/umami/css/components/blocks/search/search.css
   @@ -156,3 +153,12 @@
   +#search-block-form .form-actions {
   ...
   +#search-block-form .form-actions input.button {
   

   Could we use something else than the id for targeting these elements?

3. +++ b/core/profiles/demo_umami/themes/umami/css/components/blocks/search/search.css
   @@ -156,3 +153,12 @@
   +#search-block-form .form-actions input.button {
   
   +++ b/core/profiles/demo_umami/themes/umami/css/components/forms/buttons.css
   @@ -2,12 +2,22 @@
   +.form-actions > input.button,
   

   Could this be .button instead of input.button?

Thank you @lauriii!

Regarding #37:
37.1
I used the latest 8.7.x branch from this morning and did not find regression in the search form.
I tested it on Ubuntu with Chrome, Firefox, on a MacBook Pro (screen DPI differences caused some issues in the past), and Windows with IE11.

Desktop screenshot (after applying the patch in this comment):

37.2
I changed the rules to target class instead of id.

37.3
Using now .button instead of input.button

In addition, I tested layout-builder confirmation screens (when choosing to discard or revert a layout) and I found a small misalignment within the buttons and some missing side margins on small screens.

Here's how it looks now after applying the latest patch:

Setting to 8.8.x-dev

This looks great, and also tidies up styling of other form elements as well, so that's another bonus.

Thanks for this work @shaal

Tested in Firefox, Chrome, Safari, and IE11

This is a nice improvement for layout builder in Umami 👍

There was a styleling error which I fixed on commit:

core/profiles/demo_umami/themes/umami/css/components/blocks/search/search.css
 37:3  ✖  Expected "align-items" to come before "justify-content"      order/properties-order

Committed ffb1e1e and pushed to 8.8.x. Thanks!

I'm leaving this as RTBC against 8.7.x. I think we can backport this to 8.7.x. This does add an additional class to layout builder form (including on production sites), but given that there are no styles attached to that outside of Umami, there's not too high risk involved to prevent this from being backported.

Cherry-picked to 8.7.x! 🍒