LGD components: consolidate?

[ctorgalson]

Disclaimer: I don't know if this issue belongs with localgov_base, but since it's the main and default frontend for the project I'm starting this here.

Having worked on theming a number of LGD projects, I find there's a bit of friction when customizing essentially identical components from different modules (and the theme itself) that, while well-suited to their original use, aren't compatible with other each other.

Mainly, I'm thinking about grid systems--I believe I counted four grid systems in use from LGD sources on one project--and widgets that implement WAI-ARIA patterns such as localgov_step_by_step's and localgov_paragraphs' accordions.

So:

• does this cause enough friction for others that we should think about tackling it when we find obvious duplication of function?
• if so how (and where) should we start to go about it? (I.e. would we override modules' implementations in the theme, or provide SDCs or what-have-you in a module for consumption by themes and modules?)

[andybroomfield]

I think a lgd components module (maybe in localgov_core) could be a useful way of introducing single directory components if we want to go down that route. What is the Drupal core version where they became avalible?

[ctorgalson]

@andybroomfield it's been in core as 'experimental' since Drupal 10.1, and 'stable' since 10.3.

I'm partial to the idea of a module as well, since that would make it available to LGD (and other) modules and localgov_base (and other themes) as an optional requirement if needed/wanted.

[markconroy]

@ctorgalson The reason there is "stuff" in modules and other "stuff" in the theme is because the modules are supposed to be able to be standalone. To make this happen, we were providing templates + css + js in modules, and then overriding all that in the base theme.

We are now at a stage where we only provide basic markup/CSS in (new) modules and let LocalGov Base do most of the heavy lifting.

I think it's a really good idea for us to consolidate what we can - in the base theme or in a module. I kind of prefer doing it at the theme-level, but that might be just because I know the theme so well. Let's not let that stop us.

[ctorgalson]

The reason there is "stuff" in modules and other "stuff" in the theme is because the modules are supposed to be able to be standalone. To make this happen, we were providing templates + css + js in modules, and then overriding all that in the base theme.

Yep. As I mentioned, well-suited to doing the jobs they were built for.

It's why I waffle a bit about where reusable components should come from. If in the theme--where this kind of work already happens, and where the basic infrastructure already exists--then maybe we don't have to introduce a new dependency, but that also means new LGD features/modules can't just use theme components directly (I don't even know: can a module declare a theme as a dependency?)

On the other hand, if from a new module, then maybe the theme gets a new dependency, as do other modules that want to use something already-built for their own purposes.

[andybroomfield]

If components where in a module, theme would be able to style them? Not familliar with how they work but could be useful for say the icon component to be able to use that in a module and localgov_base styling it as needed.

[ctorgalson]

If components where in a module, theme would be able to style them?

The short answer is "yes".

The better answer is "yes," but I think we have to figure out the best practices (or at least I do).

The main questions I have about doing this are how and how much would we style components provided by a module?

Given that:

• most LGD themes will use localgov_base's css and css variables,
• a module can't assume the theme's css and variables will be available
• a module also can't make too many assumptions about the visual presentation that its consumers will need

...I would say we'd probably want to keep component styles to a minimum.

On the other hand, providing SDCs in the context of localgov_base would mean we could provide components that "fit" pretty well by default (so a component could just directly use --vertical-rhythm-spacing for example).

By default, SDCs automatically detect their own library files. This deprives us of the opportunity to declare a CSS file as component or theme as in regular *.libraries.yml files (which helps with overriding, because it affects the order the files are loaded in).

But SDCs can also take responsibility for declaring their own library files explicitly. I think--but haven't tested--that we can use the libraryOverrides key on the component schema file to take complete control of the library declared by the component. If that's true, then we could (by convention) have components declare their CSS as component to make things simpler.

(Added: as far as I know, we can also use libraries-override, libaries-extend etc at the theme level to override an included component's libraries).

[ctorgalson]

It also seems there are some outstanding problems with the order of dependency loading that affect SDCs.

[JohnAlbin]

I find there's a bit of friction when customizing essentially identical components from different modules

Yes, modules MUST be able to use components in the markup they provide. Otherwise they will be forced to duplicate components that exist elsewhere. Which is no better than what they are already doing in their templates.

In my experience, before SDC, I always built components in the theme layer since I could not hack existing modules and stuff reusable components into them.

But if components reside in themes…

that means new LGD features/modules can't just use theme components directly

Yep. Exactly what @ctorgalson said. That's not going to work.

Hypothetically, we could put components in localgov_base and have the modules' templates use them anyway. But that means sites that used LocalGov could NOT use a theme that was NOT built on top of localgov_base. Otherwise, trying to use localgov modules without the base theme would break the site with a white screen throwing errors like this:

Missing component error: Twig\Error\LoaderError: Template "localgov_base:non-existent" is not defined.

That's unacceptable, IMO.

Drupal core is going to face the same issue. Modules will need to specify a dependency on a specific component library (defined in core/ or modules/). How they solve the "switch to a different component library" is a TODO. Will they create a "component library" that is not a module and not a theme? 🤷

But the only mechanism we currently have for modules to use components is to wire module data through normal template files and, from there, into components.

Which means…

1. LocalGov will need to define a component library in a module. I recommend just using localgov_core to store components.
2. Every template in every LocalGov module needs to be updated to use the components in localgov_core.
   1. If the markup remains the same, we can release new component-based templates in minor releases of LocalGov.
   2. If we are fixing the "why are there two different markup implementations of accordions?" problem, one of the templates will need new markup which is a breaking change. I think we can work-around that by:
      • adding a {%set use_deprecated_markup = true %} {%set use_experimental_markup = false %} flag that is local to that twig template (NOT a global flag)
      • adding an if/then to choose whether to use the old stable markup or the new component-based markup.
      • That would still allow us to release the opt-in change in minor releases of LocalGov.
3. To use components in core and contribute modules, the templates from those places will need to be overridden and placed in the localgov_base theme or a new starterkit theme.

Regarding #3, IMO, we should create a new starterkit theme and leave localgov_base as-is.

If we went with the "add components to the localgov_base base theme" path:

• Updating localgov_base's templates to use new components would be very disruptive for sub themes. Breaking changes (slightly different markup, etc.) would be inevitable.
  • And while we could go with the same use_deprecated_markup use_experimental_markup hack that a handful of modules might have to use, doing this in the base theme would affect all 94 templates in base theme making it very messy.
• The safest way to add components to the base theme would be in a dev branch as we gradually accumulate all the components. And then, after the conversion is complete, release the new component-laden base theme in a new major version of localgov.
• During the lengthy conversion of the base theme to use components:
  • only the components used in localgov_* modules would be used by a site.
  • None of the work being done on core/contrib's templates would be seen unless people actively work to port the unreleased base theme work to their sub theme. That's assuming they saw work that was effectively invisible in a dev branch.

If we went with the "add componets to a new starterkit" path:

• The new localgov_starterkit theme should incrementally get new converted templates that use components added to localgov_core.
• New templates using the new components can be added in minor versions of LocalGov.
• Each time someone creates a new theme from that starterkit, they'll get the progress so far of the conversion process.
• It will be easer for people to contribute new components back to the starterkit since they will have the most recent partial component conversion in their new theme.
• How does a starterkit-based theme get new components? Well, the new components are simply released as part of the component library in localgov_core. The new themes will just be missing the core/contrib templates that use those components that are in the latest starterkit. Drupal's starterkit API has a method to see changes since the last sub theme creation though I've only played with that once and not in production.
• Should the starterkit theme be a sub-theme of localgov_base? Yeah, that probably makes sense.

[JohnAlbin]

It also seems there are some outstanding problems with the order of dependency loading that affect SDCs.

They got that committed to 11.x last week and are working on a back port to 10.x. 🥳

Thinking about the use_deprecated_markup flag I mentioned above. We should probably invert this to match core's idea of a "experimental" feature. I think a starterkit partially implementing SDC is very much an experimental idea. So the flag should be use_experimental_markup set to false by default.

[markconroy]

Here's what I am expecting to happen:

1. Modules provide their own templates + minimal CSS (and perhaps JS) - no different to what they currently do, allowing them to be standalone modules that can be used on any Drupal website, not just LGD sites.
2. Templates in localgov_base are converted to use SDC - exact same markup that they currently have, just with variables names swapped out.
3. Templates in localgov_base then map their variables to whatever their corresponding value is in the SDC version - so there is no change in the outputted markup at all, meaning custom themes relying on specific classes in these templates will still have them
4. Custom themes that have overridden a template (node--localgov-news--full.html.twig for example) will continue to work as they currently do.

When we get that far, we can start thinking about an even better implementation, but for now I think we go with the path of least resistance and try not to over think eventualities, realising that no matter what we do we are going to miss something anyway. And that's okay, we're used to fixing bugs.