Remove DEFAULT_THEME handling

[ufundo]

Overview

Remove DEFAULT_THEME handling and Greenwich dependency. Follow up to #31985

Before

• default = "Automatic" theme option, which gives you Greenwich
• Greenwich is required

After

• no default = "Automatic" theme option
• people previously on "default" switched to explicit Greenwich on upgrade
• Riverlea should be installed during install/uprade
• "minetta" is the default for each theme setting. If not available will give you no theme.
• Greenwich is no longer hidden/required, can be disabled if desired

Comments
--------‐‐------------
In previous PR's @totten concluded that the theme setting should have an explicit value. Based on this I think its reasonable that an invalid value falls back to no theme (just core css) => indicates to the user they need to pick a theme. This also avoids any dependency on any specific theme.

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers...

• ➡️ PR Demo site (can be rebuilt with the command test this please).
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[ufundo]

If we really think that the theme setting should be explicit, then maybe this should be 'none'?

[ufundo]

Hmmm test gods angry again 🙄

[ufundo]

Jenkins retest this please

[ufundo]

I'm not sure why in the context of themeTest it still seems Greenwich is available but Minetta is not.

I tried to simulate hook_civicrm_themes when both Greenwich and Riverlea are enabled in getThemeExamples like this:

$hookGreenwichRiverlea = []
\greenwich_civicrm_themes($hookGreenwichRiverlea);
\riverlea_civicrm_themes($hookGreenwichRiverlea);

greenwich_civicrm_themes seems to be available, but riverlea_civicrm_themes is not... 😢

edit Conclusion: even if tests pass, they aren't testing anything Riverlea.

[totten]

This is a tricky one. Kudos for keeping it moving.

The idea of no_theme (different from none and different from greenwich) is confusing to me:

1. "No Theme" vs "No Styles" – In the theme-selector, these sound like the same thing (even if they're functionally different).

   

2. "No Theme" vs "Greenwich" - Historically, css/civicrm.css defined the canonical look-and-feel - and this was later dubbed "Greenwich". With the current revision, the same look-and-feel from css/civicrm.css is also branded as "No Theme". But substantively, they're the same thing.

   Except that "Greenwich" has gotten maintenance for compatibility with Bootstrap classes (ext/greenwich/*). That's the part omitted from "No Theme".

   But we kind of need that part -- we're on a course where screens are incrementally converted (civicrm_admin_ui etc). The portion of the UI that works with Bootstrap shrinks over time.

   I have trouble getting my head around displaying an option that starts half-broken (and gets worse from there).

3. Unrecognized Theme Scenario – It sounds you've been looking at it from the angle of Minetta, which is a valid consideration, but the functionality is not really about Minetta. (Like... Minetta is bundled-in, so it should be available...)

   It's more relevant for a GUI admin using a contrib theme -- e.g. you install Shoreditch, play around, and disable it. Unless they do the deactivation steps just-right, the system is now misconfigured (theme_backend=shoreditch). They need the UI to work so that they can navigate to fix it (either re-enable Shoreditch or change the theme setting).

   The mechanism is basically failsafe so that the GUI admin can do recovery. The failsafe should give a well-defined behavior; but no_theme is only half-defined.

   Can't we loosen the dependency while still choosing a fully-defined theme -- e.g. random item from the registry, or first item by weight? (As long as the user has any valid theme installed, the system can run with it?)

[ufundo]

Sorry I should have put this back to draft. I was mainly trying to get my head around some of the logic in \Civi\Core\Themes and related tests.

"No Theme" vs "No Styles" – In the theme-selector, these sound like the same thing (even if they're functionally different).

Yes I don't like the naming either.

But what I was trying to achieve was:

• "No Styles" = suppress all css from CiviCRM. Advanced option that allows you to do something completely different if you so choose. (tbh I'd hide this from the GUI setting options if poss)

• "No Theme" = fallback position which would ideally be just "functional" css. I don't think I'd quite appreciated how much Greenwich spans the Greenwich extension and css/civicrm.css.

Unrecognized Theme Scenario – It sounds you've been looking at it from the angle of Minetta, which is a valid consideration, but the functionality is not really about Minetta. (Like... Minetta is bundled-in, so it should be available...)

This is the scenario I'm trying to address, but my thought process was different. I was thinking instead of a random theme fallback, you should get unthemed (functional) css only. This should allow the admin to navigate the site to rectify the situation. But it's very obvious that you haven't got the theme you wanted, and you need to do something about it.

The concern with "pick another available theme" is a situation where you might not realise it's switched. E.g. for some reason you've disabled Riverlea, and then your switched from Minetta to Greenwich, and as an admin you don't actually realise because on the surface they are quite similar. Meanwhile, on some frontend page that was built based on Riverlea, users are getting a borked site.

If we think "admins should always make an explicit theme choice", then having something that on the face of it looks safe might actually have hidden dangers.

All that said, it seems the boundary between civicrm.css and Greenwich isn't where I thought it was, so we don't have a "functional" css layer...

[ufundo]

no_theme is only half-defined.

In what sense is it only half defined? It seems well defined to me. (Though not doing quite what I'd like.)

[totten]

In what sense is it only half defined? It seems well defined to me. (Though not doing quite what I'd like.)

In this sense: the contract between app-html and theme-css is a list of tags/classes. no_theme delivers one large part of the contract (civicrm.css) but omits another large part (bootstrap.css).

The practical impact is circumstantial. For example:

• Today, you can use the "Manage Extensions" screen with just civicrm.css. (So our errant site-builder who disabled the theme-extension can navigate back and fix it.)
• However, that page needs major rework. Whenever that happens (in, say, 4 months), it will likely depend on some mix of SearchKit/FormBuilder/Bootstrap -- and all dev/QA will assume the presence of a full theme like RiverLea or Greenwich. (At that point, our errant site-builder goes up a creek without a paddle.)

...thinking... you should get unthemed (functional) css only...

Ah, OK, I think that's an idealized distinction. For example, using no_theme, I see this kind of thing:

On the left, there's the obvious aesthetic effect of being unthemed (tabs become bullets; inputs misalign). But also (on the right) the More... link doesn't work. (CSS is both aesthetic and functional -- at least in the cases of civicrm.css and bootstrap.css.)

The concern with "pick another available theme" is a situation where you might not realise it's switched.

True!

CRM_Core_Session::setStatus(
  ts('The configured theme (%1) is unavailable. The system has auto-selected an emergency fallback. Please update the <a %2>Display Preferences</a> or enable a <a %3>suitable extension</a>.', [
    1 => htmlentities($theme),
    2 => sprintf('href="%s"', Civi::url('backend://civicrm/admin/setting/preferences/display?reset=1', 'h'),
    3 => sprintf('href="%s"', Civi::url('backend://civicrm/admin/extensions?reset=1', 'h'),
  ]),
  ts('Theme Warning')
);

[ufundo]

unthemed (functional) css only... an idealized distinction

Idealised, sure, but important.

The Manage Extensions is a case in point. Today you can use with just civicrm.css, but maybe it will require more in future.

However, if your fallback is "pick any theme" and the only one left is NO_STYLES, then the site builder will get stuck unable to re-enable a theme.

Essentially I think whilst stripping out the dependency on Greenwich (specific theme) we should be aiming to retain a minimal layer of functional css. (In an ideal ideal world the html markup would be such that it would be basically functional without css, but obviously we're not there.)

Two further thoughts:

• for angular components, I think it would be great to have the minimal css in the angular module css files
• if we're accepting that core requires bootstrap, I think it would be good if we could move the bootstrap css to somewhere more generically accessible (ie. move from ext/greenwich to bower_components).
  • That would make it more reasonable to include in NO_THEME
  • Also make more sense for riverlea, which is currenly using the "Greenwich's" copy

[ufundo]

@totten - nudge on this, particular the most pertinent point:

if your fallback is "pick any theme" and the only one left is NO_STYLES, then the site builder will get stuck unable to re-enable a theme [if the extension page requires bootstrap]