Fix: Layout conflict

[Adnan-cds]

What does this change?

Layouts from the localgov_paragraphs_layout submodule (e.g. Two column (simple)) are experiencing conflict with similar core layouts (e.g. Two column). This is because these layouts are using the same CSS selectors as the core ones (e.g. .layout--twocol). Renaming these CSS selectors resolves this.

How to test

1. Enable the publications module or the subsites module where you can use layouts.
2. From Configuration > Content authoring > Layout paragraphs settings > Layout sections (i.e. /admin/config/content/layout_paragraphs/sections), enable these layouts: Two column, Three column 25/50/25, Three column 33/34/33, Two column (simple), and Three column 33/34/33 (simple). The last two of these layouts are provided by the localgov_paragraphs_layout submodule.
3. Add a subsite page or publication page and in that page select both the "Two column" and "Two column (simple)" layouts. Add images (or any other paragraphs) to all available regions of these layouts.
4. Repeat the same for Three column 25/50/25, Three column 33/34/33, and Three column 33/34/33 (simple) layouts on a separate page.
5. The layouts should act as expected.

Resolves #260

[tonypaulbarker]

@markconroy @finnlewis @Adnan-cds

First, thank you for this PR Adnan.

My learnings:

/web/core/modules/layout_discovery/layout_discovery.layouts.yml

is a good place to check the icon_map and see what the spec is for each core layout

Core two columns specifies four regions (!):
- [top]
- [first, second]
- [bottom]

Two column bricks specifies seven:
- [top]
- [first_above, second_above]
- [middle]
- [first_below, second_below]
- [bottom]

and so on.

Any reason not to document the layouts we have this way?

I seem to recall there was mention last week of some prior changes that these changes follow up?

Recommendations

I think it's good to fix the conflict and distinguish the localgov layouts with their own classes but I don't think that layout--threecol-33-34-33--localgov naming is valid BEM
Consider naming classes to also match the template naming layout--localgov-threecol-33-34-33 rather than layout--threecol-33-34-33--localgov

We could (and probably should) go further on the consistent naming front and revise CSS file names (and any library refs to them) to match the changes.

https://github.com/localgovdrupal/localgov_base/pull/833/files should be updated to match these changes

Lots of refs in:
localgov_paragraphs/modules/localgov_subsites_paragraphs/css/localgov-subsites-paragraphs.css
(and their localgov_base overrides) will require updating - perhaps we should retain the spacing applied to core classes or decide to only apply the spacing to the localgov layouts.

Mitigating custom overrides / breaking changes:

When updated and tested the two PRs should be released together with a minor (not patch) version bump since folks can be expected to follow localgov_base for breaking theme changes when updating their projects.
We should test against Microsites since these changes can affect both profiles.
We should check what's in localgov_base 1.9 and consider testing against that too.

[finnlewis]

Discussing in Merge Tuesday, @markconroy and @tonypaulbarker agree on the naming suggestion.

Like layout--localgov-threecol-33-34-33

We should refactor to align on this. And align this too https://github.com/localgovdrupal/localgov_base/pull/833/files

Also @tonypaulbarker mentions we should document this here in a README and perhaps in other places.

[finnlewis]

@Adnan-cds is this OK? Harmonise on layout--localgov-threecol-33-34-33 style?

[Adnan-cds]

Yeah, that's fine. I will try to complete the changes before the next Merge Tuesday.

[markconroy]

@Adnan-cds Any chance you could get these changes made so we could hopefully merge them tomorrow at Merge Tuesday?

[Adnan-cds]

Sorry Mark, totally stuck on something else :( Just can't spare any time to look into LocalGov tasks for another month or so. But I have some downtime after this week. I will try to use that to get this ready for next week's Merge Tuesday if that's okay.

[markconroy]

Thanks Adnan, take care.

[Adnan-cds]

Ready for review :)

[finnlewis]

@tonypaulbarker thinks this looks good!

Would like to

1. test the functionality
2. push to drupal.org and create a merge request there,

[tonypaulbarker]

@Adnan-cds @finnlewis @markconroy

In functional testing, this issue has gone away if using localgov_base with recent changes to localgov_base because of inheritance. When testing without the latest template versions, I find that the layout issue is gone but the gap between columns is missing.

I think that we might have adjustments to make to localgov_base -
https://git.drupalcode.org/project/localgov_base/-/blob/2.x/templates/layouts/two-col/layout--localgov-twocol.html.twig

set classes = [
  'layout',
  'layout--twocol',  // ← should be 'layout--localgov-twocol' so we don't conflict with the core version 
  'lgd-row',
]

Probing where the gap has gone -

In

modules/localgov_paragraphs_layout/layouts/twocol/twocol.css

we have

.layout--localgov-twocol {
--lgd-page-section-gap: 0;

so I think that there was never a gap when localgov_base overrode it but now with the class name update nothing in localgov_base is overriding the gap for this class.

I think the next step is: create drupal.org issues and PRs against both localgov_paragraphs with the changes from this MR and localgov_base with some adjustments and align the patterns.