Massachusetts
Account created on 15 November 2007, almost 18 years ago
  • Senior Software Engineer at Acquia 
#

Merge Requests

More

Recent comments

🇺🇸United States phenaproxima Massachusetts

phenaproxima created an issue.

🇺🇸United States phenaproxima Massachusetts

Capturing some discussion with @pameeela in Slack - ideally a site template would not need to rely on a bespoke theme at all; it would be able to just spin up with an automatic fork of Mercury (so that Mercury is free to evolve as it needs), and then do all further customization in Canvas, with JS components being used if custom design elements that stand outside Mercury's design system are needed.

Canvas isn't quite there yet, though. Site templates still need custom themes, based on Mercury but not extending it (i.e., themes forked from Mercury as a starter kit, rather than using it as a base theme), that can implement their customized styling or tweaks.

Ideally, they can ship these themes in such a way that they immediately become a part of the site itself, rather than exist as specific projects published on drupal.org. That will lead directly to a wasteland of dead and unmaintained themes -- a problem we would like to avoid.

My proposal is that we use Site Template Helper to support the following workflow:

  • A developer creating a site template uses core's generate-theme command to create a custom theme that is a renamed Mercury clone.
  • They build their site template on that theme, customizing it as needed.
  • When it comes time to distribute the site template, they make it depend on Site Template Helper, and they zip up the theme into a file called theme.zip, that sits in the site template.

When a site builder spins up a site using that template:

  • The site template is downloaded and installed by Composer, just like any other recipe.
  • Site Template Helper, being a Composer plugin, springs into action and extracts the theme.zip file to themes/custom.
  • The template can then be applied like a normal recipe. The theme is completely disconnected from Composer, and it is added to the project's Git repository as a totally custom, bespoke theme that will not be affected by any upstream changes in Mercury.

Although this isn't necessarily the ideal long-term solution, it solves the problem of depending on Mercury (or any other upstream theme), and it solves the problem of spamming drupal.org with highly custom, one-off themes, meant for but disconnected from specific site templates. It unambiguously puts the site's presentation layer under the complete control of the site template user, and the theme is truly the only thing that is controlling the site's look and feel. It's also a fairly easy workflow for site template developers -- "generate a theme and stuff it in a zip file" is simple to grok.

🇺🇸United States phenaproxima Massachusetts

Created https://www.drupal.org/project/site_template_helper for this, at @alexpott's suggestion. It's really only meant for site templates. We shouldn't use it yet because we haven't yet figured out what the best practice is.

🇺🇸United States phenaproxima Massachusetts

Merged into 2.x. Not backporting because:

  • ai_agents has no stable 1.2.x release
  • The existing constraints in Drupal CMS 1.2.x will not block updating either module, so this version bump is really just housekeeping, not a necessity
🇺🇸United States phenaproxima Massachusetts

Auto-merged into 2.x. I'll have to backport it to 1.2.x.

🇺🇸United States phenaproxima Massachusetts

I think that, in Drupal CMS, we could add a drush cr as a post-update-cmd Composer script.

🇺🇸United States phenaproxima Massachusetts

OK, well, I implemented one way to do it: with simple config in a shim module.

Frankly, though, I don't like this approach. It makes the shim module a permanent runtime dependency.

It'd be better if we could use something like Easy Responsive Images, or maybe we could use ECA to dynamically generate these breakpoints.

🇺🇸United States phenaproxima Massachusetts

phenaproxima made their first commit to this issue’s fork.

🇺🇸United States phenaproxima Massachusetts

Was able to add the webform-category--footer class to the webform without changing Mercury. It looks good!

🇺🇸United States phenaproxima Massachusetts

Fixed directly in 2.x.

🇺🇸United States phenaproxima Massachusetts

This was due to an upstream change in Canvas. I adjusted the site template to match, and it's rendering properly now. No change is needed in Mercury.

🇺🇸United States phenaproxima Massachusetts
🇺🇸United States phenaproxima Massachusetts

phenaproxima created an issue.

🇺🇸United States phenaproxima Massachusetts

Thanks Pam! Merged into 2.x.

🇺🇸United States phenaproxima Massachusetts

We might not need a preprocess at all; let me see if I can add the requisite classes to the webform’s configuration directly.

🇺🇸United States phenaproxima Massachusetts

phenaproxima created an issue.

🇺🇸United States phenaproxima Massachusetts

This breaks preexisting functionality - the "Duplicate" operation does not appear in the links for each item in the view.

🇺🇸United States phenaproxima Massachusetts

phenaproxima made their first commit to this issue’s fork.

🇺🇸United States phenaproxima Massachusetts

Merged into 2.x.

🇺🇸United States phenaproxima Massachusetts

I think that'd be fine. It's okay to attach it in a Twig file, just not for a specific webform (identified by ID). Putting it in webform.html.twig would be okay.

🇺🇸United States phenaproxima Massachusetts

Merged into 2.x.

🇺🇸United States phenaproxima Massachusetts

As part of this, we should remove CSS classes that target specific webforms by their config ID, such as webform--newsletter-signup. Webforms have the ability to be assigned arbitrary CSS classes, so instead let's just replace that selector with something like mercury-footer--webform, and then rely on site templates to assign that class to any webforms that they might want to place in the footer.

🇺🇸United States phenaproxima Massachusetts

phenaproxima made their first commit to this issue’s fork.

🇺🇸United States phenaproxima Massachusetts

phenaproxima made their first commit to this issue’s fork.

🇺🇸United States phenaproxima Massachusetts

Marking as fixed since, as far as I know, no more is needed here.

🇺🇸United States phenaproxima Massachusetts

The fix for the failures are in an RTBC issue

Huh? How can that be true? All PHPUnit test jobs in that issue are failing...

🇺🇸United States phenaproxima Massachusetts

I guess it had just fallen off the radar. Merged into 2.0.x. I can also commit this to 8.x-1.x if someone can open a separate MR (in this issue) to backport it.

🇺🇸United States phenaproxima Massachusetts

Removing the stable blocker tag for now; I think it should be a stable blocker, but that's not my decision to make. Stayin' in my lane for this one. :)

🇺🇸United States phenaproxima Massachusetts

This is fixed in https://git.drupalcode.org/project/drupal_cms/-/commit/9cd2a9584e2f7f0b6... (already in HEAD). Looks much better!

The actual images and URLs aren't properly wired up yet, but I'm working on that.

🇺🇸United States phenaproxima Massachusetts

This needs to be an additional XB content template that targets blog posts in that specific view mode.

🇺🇸United States phenaproxima Massachusetts

This template was removed by 📌 Remove all content type-specific templates Active so I think we can close this one out. :)

🇺🇸United States phenaproxima Massachusetts

Here's an idea -- what if we had a Composer plugin which could handle this? For the sake of description, let's call it phenaproxima/librarian.

So, first, you'd enable this plugin in your top-level composer.json: composer config allow-plugins.phenaproxima/librarian true

Then, in its top-level composer.json, Webform would depend on this plugin. And it would add the contents of composer.libraries.json's repositories section to its own extra.librarian section. Example:

{
  "extra": {
    "librarian": {
      "choices": {...},
      "codemirror": {...},
      etc.
    }
}

So what would happen here is: when Webform is about to be installed, the librarian plugin kicks into action. If it finds a package (Webform, in this case) that has a set of libraries defined this way, it exposes them to Composer as packages, generated on-the-fly, and adjusts Webform's requirements dynamically so that it depends on them.

I need to try this out, but I suspect it would work.

🇺🇸United States phenaproxima Massachusetts

Leave this one with me for a bit. I have a few mad-science ideas that might bear fruit here.

🇺🇸United States phenaproxima Massachusetts

Don't do a recipe - do a metapackage. I don't see how a recipe is useful here.

But the repositories key has no effect in any composer.json except for the top-level (project-level) one. Source: https://getcomposer.org/doc/04-schema.md#repositories and https://getcomposer.org/doc/04-schema.md#root-package

🇺🇸United States phenaproxima Massachusetts

Escalating this. Drupal CMS needs this, because without it, we cannot ship site templates that work in a consistent way. We can patch XB for now, but this will need to get fixed before Drupal CMS can release an XB-ready version.

🇺🇸United States phenaproxima Massachusetts

The only downside here is that you'll have a 404 for a homepage if you start with drupal_cms_starter.

On the other hand, you're not supposed to start with that recipe, as it is now a basis for site templates. So indeed, the site template is expected to provide the homepage. Therefore, this makes sense to me.

🇺🇸United States phenaproxima Massachusetts

Came from XB. Should be in core.

🇺🇸United States phenaproxima Massachusetts

Merged into 2.x.

🇺🇸United States phenaproxima Massachusetts

According to https://www.drupal.org/node/3363700 , this was changed in Drupal 10.2 -- I think we will probably need to bump the core_version_requirement in the info file to ^10.2 || ^11.

🇺🇸United States phenaproxima Massachusetts

phenaproxima made their first commit to this issue’s fork.

🇺🇸United States phenaproxima Massachusetts

phenaproxima made their first commit to this issue’s fork.

🇺🇸United States phenaproxima Massachusetts

the recipe data contains a UUID (which is not a best practice, but it's also not forbidden, and sometimes, there may be a good reason for it)

Wim, can you elaborate on this bit? It's not forbidden but that might just be an oversight on our part. What are some good reasons for a recipe to include config UUIDs?

🇺🇸United States phenaproxima Massachusetts

Is there any reason we shouldn't just unconditionally remove the uuid and _core keys from the incoming recipe data?

The recipe system assumes those keys aren't there, but that's not a safe assumption -- lots of people make that mistake. We definitely do not want to have those keys be considered in the comparison.

On the other hand, we could remove those keys from the recipe data during the comparison, but if the recipe still has those keys, they'll get imported anyway...which could be bad.

I think what needs to happen here is that RecipeConfigStorageWrapper should unconditionally remove those keys from any config it reads in. Either that, or the FileStorage object created by \Drupal\Core\Recipe\ConfigConfigurator::getConfigStorage() needs to do it.

Production build 0.71.5 2024