Greenwich: Add BootstrapCSS support via core extension

[totten]

Overview

The defines a core-extension, greenwich, to represent the "Greenwich" theme and includes an implementation of the bootstrap3 bundle.

See also: #18190, https://lab.civicrm.org/dev/user-interface/-/issues/24, https://lab.civicrm.org/dev/user-interface/-/issues/27

Before

civicrm-core has a built-in theme called "Greenwich", which is enmeshed with the main code.

It does not have support for bootstrap3 bundle.

After

There is a new core extension, greenwich, for defining the theme.

The extension is auto-enabled (via install or upgrade), and it is hidden - in the same way that sequentialcreditnotes is. So greenwich is always available.

It provides an implementation of the bootstrap3 bundle.

Technical Details

The extension folder (ext/greenwich) includes a few key input-files:

• scss/main.scss: Top-level source file
• scss/_greenwich.scss: Configuration options (colors, padding, fonts, etc)
• extern/bootstrap3: The library of raw code for Bootstrap3-SASS

To pre-compile SCSS in a way that works for multiple deployment styles (eg D7/WP/BD and D8/D9), we had to write a new plugin https://github.com/civicrm/composer-compile-plugin. The plugin essentially allows us to define post-install hooks within civicrm-core:composer.json. I've tried it in combination with several relevant plugins (e.g. civicrm/composer-downloads-plugin, civicrm/civicrm-asset-plugin, cweagans/composer-patches) and left integration-tests to show it works.

The key bit of glue is:

// FILE: ext/greenwich/composer.compile.json
{
  "compile": [
    {
      "title": "Greenwich CSS (<comment>dist/bootstrap3.css</comment>)",
      "php-method": "\\Civi\\Compile\\Scss::build",
      "watch-files": ["scss"],
      "scss-files": {
        "scss/main.scss": "dist/bootstrap3.css"
      },
      "scss-includes": ["scss", "extern/bootstrap3/assets/stylesheets"]
    }
  ]
}

The output of this task is (scss-files: {...: "ext/greenwich/dist/bootstrap3.css"}. Greenwich uses this file to define its implementation of the bootstrap3 bundle, i.e.

// FILE: ext/greenwich/greenwich.php
function greenwich_civicrm_alterBundle(CRM_Core_Resources_Bundle $bundle) {
  $theme = Civi::service('themes')->getActiveThemeKey();
  switch ($theme . ':' . $bundle->name) {
    case 'greenwich:bootstrap3':
      $bundle->clear();
      $bundle->addStyleFile('greenwich', 'dist/bootstrap3.css');
      $bundle->addScriptFile('greenwich', 'extern/bootstrap3/assets/javascripts/bootstrap.min.js', [
        'translate' => FALSE,
      ]);
      break;
  }
}

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

I downloaded this & recreated - I think the idea of 'success' here is mostly no change. I did check the search extension & it still gives the no bootstrap message & apiv4 explorer looks pretty rough still.

I thought I detected some slight theming changes. I figured the best way to check was to load the test site from this PR & another one but per product mtce channel - the tests sites are a bit broken today

[MegaphoneJon]

Am I correct in understanding that the results of composer install will be bundled with the tarball?

[totten]

@MegaphoneJon - Yup, that's the idea - this PR gives a CSS file to include in the tarball.

There are some trade-offs using a pre-compiled CSS versus compiling on the fly - eg initial load-time vs runtime customization.. The feedback I got before was that it was important to have the pre-compiled CSS, given the number of people who have dev-builds and who disable asset-caching. (However, if it takes our fancy to have runtime-customization option... that can coexist.)

@eileenmcnaughton If you look at APIv4 Explorer screenshots side-by-side, the changes should be a little less subtle. Here's how they appear for me:

Default Appearance: Before

Default Appearance: After

Shoreditch

All the tabs/padding/headers look better. The alignment in the initial action-bar is a bit off, but it's also a bit off in Shoreditch. I think @colemanw mentioned that the select2 widgets may require an extra file to look better.

For the search extension, it needs (at a minimum) a patch similar to 8266ce8.

[eileenmcnaughton]

@totten OK - I was focussed on whether non-bootstrap css looks the same as I figure anything is better on bootstrap & we can iterate. So I see this as

1. we've discussed the approach a lot - this was the 'lots of effort by @totten approach without any real trade offs as we decided to do it properly' - so I'm happy on that front.
2. given we agree the direction & will need to keep working on the final look I'm going for 'do no harm'
3. I tried to visually compare the test sites for this PR & other PRs on the same day to try to confirm 2 - but the test sites are broken at the moment :-( Locally I spotted a couple of things but I couldn't be sure they related (buttons seemed not latest, visual line under menu) so I figured comparing 2 test builds made most sense

[eileenmcnaughton]

The test sites issue was transient - it's now possible to compare

Without this change
http://core-18471-88heg.test-3.civicrm.org:8001/civicrm/dashboard

With this change
http://core-18465-879kp.test-1.civicrm.org:8001/civicrm/dashboard

My criteria for merging this is 'no change to non-bootstrap screens' and after a quick look the things I thought I saw locally are not different between the 2 screens

I'm adding 'merge-ready' since I think it is good-to-merge.

I would like to see as many related issues as possible fixed in the same release to make 5.31 'the theming release'

Top of mind at the moment is the bootstrap alignment per below, search (per above) and #17675 & https://lab.civicrm.org/dev/core/-/issues/2030

These things can all ideally follow this merge & we can also do a call in dev-digest for any theming related issues to be highlighted

[agh1]

This doesn't necessarily look bad, but there was a lot of discussion on dev/user-interface#24 about what direction to go in, and if this is merged without engaging everyone there, I think there will be surprise and frustration.

[colemanw]

@totten this needs rebase. Also can you please clarify whether this will load the bootstrap css on every page, or only those pages which opt-in or "require" bootstrap?

[JoeMurray]

So, not sure that this is helpful, but Bootstrap 3 was released in August 2013 with work on it ceasing in Sep 2016, Bootstrap 4 came out in Jan 2018, and Bootstrap 5 alpha 1 came out June 2020.

[artfulrobot]

So I think that the idea of including BS3 in the core Greenwich theme is that it fixes the bits that have been written assuming BS3 and are really broken without it. If so then I'm 💯 in.

If it's because venerable BS3 💀 is about to become Civi's new thing ✨ that we're going to standardise on going forward, then I have reservations!

[eileenmcnaughton]

@artfulrobot I pushed for bootstrap 4 (& @colemanw for BS5!) but there was a call with ? a few people who felt that BS3 would be less disruptive at this stage - I think existng themes do BS3 & markup changes are needed for BS4

[eileenmcnaughton]

@agh1 OK - this was in the dev-digest but I can comment on that task

[eileenmcnaughton]

Ah Coleman already did

[agh1]

@eileenmcnaughton I'm with @artfulrobot in being hesitant about doubling down on Bootstrap 3. I understand that this is the easiest lift for making the handful of new Bootstrap 3 interfaces work in the short term. However, I'm only comfortable if this is in the context of mid- to long-term planning to ditch Bootstrap 3.

I know that there's this cascade of projects:

• why move to Bootstrap 4 when you'll be stuck with it when 5 comes out?
• why hardcode Bootstrap-specific markup when we need an actual theme layer?
• why build out a theme layer in Smarty when it's more obsolete than Bootstrap?
• why insert a theme layer in front of Quickform elements when that's even more obsolete?

The natural reaction is to throw up your hands and focus on the immediate task. Of course everything won't be fixed this year or next. I just want to be sure that our decisions to solve short-term problems don't cement us further to obsolete libraries.

Most importantly, if there are things we can do now to accommodate a to-be-determined post-Bootstrap, post-Smarty, post-Quickform world (that isn't AngularJS, because that's no less obsolete), I'd like to be sure any framework changes like this incorporate them.

[eileenmcnaughton]

@agh1 I'm not sure I see this as a doubling down. I don't think we are any more locked in with this change - the lock-in is our existing ecosystem around this.

I think this does help us towards the larger goal just by starting to decouple theming & improving the way the theme interacts & the flexibility of including bootstrap

[agileware-justin]

Any idea what the transitional step is to move from Bootstrap 3 to Bootstrap 4/5 or another CSS framework entirely, like Zurb Foundation?

[eileenmcnaughton]

@jusfreeman @agh1 @artfulrobot we should make sure there is a gitlab for @jusfreeman's question - not sure if there is an existing one but I know it's been asked before

[agh1]

@eileenmcnaughton I agree that this in itself isn't causing more lock-in, but I just want to be sure that this is being approached from the perspective "is this a decent stop-gap that won't hinder a post-Bootstrap future?" rather than "is this a good framework for our transition to Bootstrap?" The latter was the mindset for a while, but I think it's clear that Bootstrap (3 at least) is the past, not the future.

[eileenmcnaughton]

@agh1 my understanding is this doesn't put a pin on the scale for the future & it's about a more manageable present + better support for 'whatever we decide' later

[agh1]

@eileenmcnaughton yeah I agree, particularly as it seems this doesn't affect 99% of CiviCRM (and the 100% that works without Shoreditch). Most importantly, my impression is that there is a bit of urgency as there are some features designed for Shoreditch that folks thought worked without it but turn out not to (at least not very well). This can resolve that.

[eileenmcnaughton]

@agh1 right - the new search extension ships with core now but it's hidden because it doesn't look OK without bootstrap - apiv4 is already in core but a bit munged without bootstrap. - absent some serious investment new forms will continue o use anglularJS/ afform but the markup in them can be altered to better support whatever we decide.

[totten]

For anyone coming into this PR cold, it's probably better to read the intro to #18190 or https://lab.civicrm.org/dev/user-interface/-/issues/27 before this.

(@colemanw) Also can you please clarify whether this will load the bootstrap css on every page, or only those pages which opt-in or "require" bootstrap?

Right, good question. It requires an opt-in. It's the same as the "Experimental" branch (#18190) and the extracted #18354, ie

Civi::resources()->addBundle('bootstrap3');

It is a little extra code from an app-dev perspective, but it also provides a means to phase-out bootstrap3 and phase-in other vocabularies. Trying to have one vocabulary automatically used for everything raises the bar for migrations pretty high.

(@agh1) This doesn't necessarily look bad, but there was a lot of discussion on dev/user-interface#24 about what direction to go in, and if this is merged without engaging everyone there, I think there will be surprise and frustration.

Ooof, yeah, the trail wasn't linked when I first opened this PR (and then, naturally, this is the PR that got attention). Let me try to recount it (at least from my POV): that Gitlab issue led to a conference call with a half-dozen Civi theming folks. (There were a few more people who I wish had come, and I wish we had kept 'minutes', but I think the group had reasonably diverse POVs and did a good job of discussing multiple angles). This, in turn led, to another issue (with lots of detail but not as much feedback; https://lab.civicrm.org/dev/user-interface/-/issues/27), and then an exploratory branch (#18190), which in turn got a round of feedback, leading to a few break-out PRs/subprojects (ie #18247, #18354, composer-compile-plugin, and finally this #18465).

(@artfulrobot) So I think that the idea of including BS3 in the core Greenwich theme is that it fixes the bits that have been written assuming BS3 and are really broken without it. If so then I'm 100 in.

If it's because venerable BS3 skull is about to become Civi's new thing sparkles that we're going to standardise on going forward, then I have reservations!

I really like the distinction between the two paragraphs.

Yes, right, there's a certain corpus (Shoreditch, CiviCase 5/CC, Mosaico, Search Builder, ad nauseum) that's built with Bootstrap3 CSS, and we want to round-out/clear-up support for that corpus. That's the motivator.

I don't think anyone considers "Bootstrap3" to be sparkly dev-branding in 2020. (TBH, I haven't seen anyone make an argument that BS4 or BS5 is substantively/materially better. They probably are... it just hasn't come up here. But that's by-the-by.)

For anyone who's concerned about BS3, it may be worth emphasizing that the approach from #18190 (etal) makes a subtle but purposeful shift:

• In the original releases of Shoreditch, it loaded BS3 CSS file on all pages, and we tended to talk about "Bootstrap" as a monolith, and this led to confusion when people tried to substitute Shoreditch's BS3 with BS4. It also raised the stakes on picking the "right" version.

• The addBundle('foo') mechanism is more explicit. It gives an identity for the contract (bootstrap3 vs bootstrap4 vs zurb6 etc). As an app-developer working with BS3 screens, you can consume the bootstrap3 bundle. As a theme-developer working with BS3 screens, you can provide (or clear/veto) the bootstrap3 bundle.

  In this structure, multiple contracts can coexist (used for different pages); they can be phased in/out; they can be measured (grep -r \'bootstrap3\'). Of course, we shouldn't add more contracts lightly (given the concrete costs and network-effects). The game becomes, "Well, we could add a BS5 contract - but is it worth the effort? Or maybe the cost/benefit is better if we wait for BS6 or Zurb8?"

[seamuslee001]

The other aspect to mention as well is that apart from the bundles we also now have better handling for what is public and what is non public via the theme subsystem and that also means that in theory you could have 2 separate themes that provided bootstrap but have one only for public pages (contribution pages etc) and one providing back office pages.

To try and Answer Justin's question, I would say that if you wanted to transfer to say bootstrap 4 you could in your extension define a new bundle

hook_civcrm_container($container) {
    $container->setDefinition('bundle.bootstrap4', new Definition('CRM_Core_Resources_Bundle', ['bootstrap4']))
      ->setFactory('CRM_MyExtension_Resources::createBootstrap4Bundle')->setPublic();
}

Then as tim points out then add it onto your pages by doing

Civi::resources()->addBundle('bootstrap4');

It may be more trickier if your new CSS framework requires some different markup but at the very least this hopefully helps extension authors go a bit faster

[homotechsual]

The other aspect to mention as well is that apart from the bundles we also now have better handling for what is public and what is non public via the theme subsystem and that also means that in theory you could have 2 separate themes that provided bootstrap but have one only for public pages (contribution pages etc) and one providing back office pages.

To try and Answer Justin's question, I would say that if you wanted to transfer to say bootstrap 4 you could in your extension define a new bundle

hook_civcrm_container($container) {
    $container->setDefinition('bundle.bootstrap4', new Definition('CRM_Core_Resources_Bundle', ['bootstrap4']))
      ->setFactory('CRM_MyExtension_Resources::createBootstrap4Bundle')->setPublic();
}

Then as tim points out then add it onto your pages by doing

Civi::resources()->addBundle('bootstrap4');

It may be more trickier if your new CSS framework requires some different markup but at the very least this hopefully helps extension authors go a bit faster

I mean chances are close to 98% changing to any other framework would require markup changes to varying degrees.

That's my main issue with any attempt to standardise core onto BS3 compliant markup without providing the ability to abstract entirely the presentation layer from the API/data layer. I get that we have to support BS3 because of contrib and previous decisions around design language but it's hard to deny that we've kind of let the tail wag the dog in this area...

Ideally it should be roughly the same process to generate markup in core (via built-in theme extensions) as it is for a contrib theme. Including the templates/markup/styles and JS assets.

[eileenmcnaughton]

@MikeyMJCO those points seem valid but I'm a bit worried about these ideas being captured here, since they are not about this PR but about the bigger issue of how we deal with markup for themers - is the gitlab issue appropriate for that?

[eileenmcnaughton]

Just noting that I'll merge this early next week if someone else hasn't - nothing in the discussion is blocking & it's been in the last 2 dev-digests

[eileenmcnaughton]

Per previous I'm merging this - I'm just trying to figure out how to raise a docs issue since the link from here is broken https://github.com/civicrm/civicrm-dev-docs/blob/master/README.md

@totten documentation task on you!

[eileenmcnaughton]

Docs issue https://github.com/civicrm/civicrm-dev-docs/issues/860

[eileenmcnaughton]

Note that per dev-digest - we should aim to ensure other theming related things are merged into the same release (5.31) - this mostly means @agh1's work so far but there is also #18397 open

[artfulrobot]

Few points to add:

• like that code has to add the css library, however if it's ok to require this couldn't we namespace the css and require extensions also use this? Otherwise suggestions like @seamuslee makes about adding other Frameworks are likely to cause collisions and broken UI. Minimal namespacing could be achieved by a class prefix on selectors, so you wrap dependent html in a div.civi-bs3 container.

• I've found that to get the bs3 stuff to work you also need selected bits of bs3 js. No mention of that here?

• @totten bs4 is importantly better than bs3 in at least because it allows for responsive sizing (rems) and doesn't impose a global resetting of the rem unit (from 16px to 10px) which usually makes other themes go all small. So yes, it's not just about panel changing to card 🙂

[seamuslee001]

@artfulrobot just to note re your point 2 that is covered here https://github.com/civicrm/civicrm-core/pull/18465/files#diff-4e53e2a6f4147fc734c2dec0a1cebcfdR56

[demeritcowboy]

Hmm I can no longer install civi on windows, but not sure yet if it's from here or the compile plugin. I'm thinking probably the plugin. It runs a command somewhere using php -r and a ; to separate two commands, but that doesn't work - it needs && which should work on linux too.

[demeritcowboy]

Ok it's not the ; but there's something about the shell command it doesn't like. Will see if I can track it down.

[demeritcowboy]

civicrm/composer-compile-plugin#2