Feature/2.x/785 padding horizontal causes unwanted indenting

[ctorgalson]

DRAFT

Closes #785

What does this change?

This is a proof-of-concept PR demonstrating a way we can eliminate the hard-to-predict and hard-to-manage layout inconsistencies brought about by the use of .padding-horizontal.

---

It's finished enough for demonstration, but not ready for use.

---

The main addition to the theme is a new SDC, localgov_base:section. The new SDC always renders the same markup, and can be configured to output one or more of ten layout classes that address most common layout issues.

For demonstration purposes, the branch makes significant changes to several blocks, the main page.html.twig template, the region.html.twig template, the header.html.twig template, and the full-content SDC, altering all of them to use the new SDC.

Even in the early stage the branch is in, most layout issues are gone. One notable exception to this is the localgov_subsite_* content types. The Page Builder paragraphs haven't been touched, though it wouldn't be a lot of further effort to get them to behave the same way.

I assume, but haven't checked in any detail, that the Scarfolk Council theme would need some/all of these changes as well.

How to test

• checkout this branch of localgov_base in an LGD site
• set localgov_base as the default theme
• try to discover places (at any breakpoint) where there are inconsistencies in the amount of space between the left and right edges of the viewport and the content
• compare to the same URLs on an unmodified version of the site like https://demo.localgovdrupal.org/

How can we measure success?

Reduction or elimination of width inconsistencies in content.

Have we considered potential risks?

The main risk is the reason why this change should only accompany a major version change to the theme: existing child themes will have corrected some the issues this change corrects, but in different, incompatible ways.

[markconroy]

Thanks @ctorgalson, this looks like a great piece of work.

I've left a few small changes that we should look at and then do some more testing.

Would be great to get this and a few more things into a new major release (e.g. remove the components directory that is now using SDC instead)

[markconroy]

margin auto here is causing all the section__inner items to be center aligned, this is affecting the main content block on service pages

[ctorgalson]

This one should be fixed on the current branch.

[markconroy]

I'm not sure exactly why, but this max-width is making our section__inn 1180px by default (e.g. breadcrumbs) but in the current theme (e.g. demo.localgovdrupal.org) they are equating to 1148px.

It's probably because the padding-inline is now on the outer container instead of the inner container.

[ctorgalson]

This is true for every region/thing that's not restricted width (see screenshot). I based it on the header which is 1180px width in the current version of the theme.

In any case, we'd just need to set a single variable to change it to 1148 or any other value.

[markconroy]

I wonder should we create a custom variable for this, something like --section-block-spacing and set that to equate to --vertical-rhythm-spacing in case some designs wants extra/less padding here, but still want the vertical rhythm amount for paragraphs and things.

[ctorgalson]

Do we need an extra variable? This is already achievable with a single line of css in the theme's variables.css or elsewhere:

/**
 * Using my new favourite unit for this, `lh`.
 *
 * @see https://caniuse.com/mdn-css_types_length_lh
 */
.lgd-section {
  --vertical-rhythm-spacing: 1lh;
}

[markconroy]

I think we can leave out role="navigation" since it's implied by the nav element.

[ctorgalson]

Fixed in current branch.

[markconroy]

We'll need a blank line here at the end of file so the tests don't fail

[ctorgalson]

Fixed in current branch.

[ctorgalson]

@markconroy, @tonypaulbarker what do we think of the overall approach?

My thoughts:

Pros

• completely capable of solving the specific issue (including Page Builder components, though I haven't done that work in this PR)
• css-wise, quite simple and easily-documented
• reasonably intuitive (though see "naming" under "Cons")
• handles both page-level and content-level layouts more easily than I'd anticipated
• plays nice with grid formatting (i.e. since it's trivial to make .lgd-section/.lgd-section__inner elements without padding)

Cons

• naming is pretty wordy, and not necessarily descriptive enough
• element class-bloat
• a little markup bloat (i.e. since every place it's used it inserts two <div> elements)
• NIH syndrome 😬? (I don't really like e.g. Tailwind, but I'm sure it or something of that sort could handle this job, and we're proposing breaking changes anyway...)
• I'm still not completely convinced that utility classes are the best way to solve the general problem(see below)

/**
 * Solving the basic issue with (almost) no utility classes 
 *
 * Given a pre-established set of layouts, we can create the utility classes (like e.g. `region--full-width`), but then
 * add selectors to that class's css rule.
 *
 * By doing this, we can essentially eliminate most uses of utility classes. And since localgov_base acts more like
 * a starterkit theme than a base theme, we can always have a local copy of this stylesheet.
 */

/**
 * Full width regions
 *
 * Use .region--full-width utility class, OR add selectors to this list.
 */
.region--full-width,
.region--header,
.region--footer,
.region--content {
  padding-inline: var(--spacing);
}

/**
 * Constrained content regions
 *
 * Use .region--constrained utility class, OR add selectors to this list.
 */
.region--constrained,
.region--content {
  margin-inline: auto;
  max-width: var(--width-container);
   
  /* Variations by node bundle */
  .node--type--localgov-services-page & {
    margin-inline: 0 calc((100vw - var(--width-container)) / 2 + (var(--width-container) - var(--width-restricted-width-section))));
    max-width: var(--width-restricted-width-section);
  }
}

[ctorgalson]

@markconroy, @tonypaulbarker any thoughts?

[markconroy]

@ctorgalson are you coming to tech group drop in today? We could chat about it there.

[ctorgalson]

@markconroy I had another call. I'll come on the 9th for sure.

[ctorgalson]

Sorry @markconroy, last-minute call here.