Add in bootstrap button classes to CiviMail interface

[seamuslee001]

Overview

This adds in bootstrap button classes to CiviMail Buttons to allow them to be themed up by bootstrap based themes

ping @mukeshcompucorp you may have an interest in this

[seamuslee001]

ping @colemanw Coleman you may want to have a look at this too and there is this PR from me in shoreditch civicrm/org.civicrm.shoreditch#223 which uses these new classes and makes the buttons look good in a traditional new mailing

[colemanw]

@seamuslee001 the CiviCRM markup does not generally include these classes, but Shoreditch is nonetheless able to style them. Why is it necessary to add those classes in this one place specifically?

[seamuslee001]

@colemanw the reason was otherwise they were all coming out as blue by default in shoreditch and there needed some extra styling in shoreditch to make them look good on initial load. The classes i picked aimed to make it as close to the Moasico style as possible https://github.com/veda-consulting/uk.co.vedaconsulting.mosaico/blob/2.x/ang/crmMosaico/EditMailingCtrl/mosaico.html#L47 https://github.com/veda-consulting/uk.co.vedaconsulting.mosaico/blob/2.x/ang/crmMosaico/BlockPreview.html#L11

[totten]

@seamuslee001 I like the idea of making it easier to theme with Bootstrap, but adhoc additions of BootstrapCSS aren't a good idea. Specifically:

• r-code: The code style of any given file becomes unclear. If I open a file and start adding another button, I should be able to infer from the context what rulebook to follow. If the file has crm-*, then I'll use other crm-* conventions. If the file has BootstrapCSS classes, then I'll use those conventions. But if the file is a mix, then my brain is going to explode.
• r-tech: Shoreditch doesn't have a monopoly on BootstrapCSS. The CMS backend theme may also define BootstrapCSS. If we happen to be using Shoreditch, then we're covered... because (I assume) you developed/tested in a Shoreditch environment (and Shoreditch has defenses to prevent conflicts with the CMS). However, if we don't use Shoreditch, and if our CMS does use BootstrapCSS, then it's going to be a crapshoot -- the appearance of our button might look like the CMS styling, or it might look like the crm-* styling, or it might look like some blend (e.g. coloring from crm-* and padding from the CMS theme).

But I agree we need a clearer way to manage the styling transition. There's some more work in this area... let me see if I can find it...

[seamuslee001]

@totten @colemanw more than happy if we can try and find something in between.

[totten]

See also:

• Add template for crm-* UI's veda-consulting-company/uk.co.vedaconsulting.mosaico#236
• https://chat.civicrm.org/civicrm/pl/w4mrj4tqrig5brqd3r7sg7m3uy

But perhaps the topic really deserves its own issue or wiki page? (It's a little hard to read/reference that recap in Mattermost.)

[seamuslee001]

@totten i just stumbled on that as well and i notice that even in the crm.* files like https://github.com/veda-consulting/uk.co.vedaconsulting.mosaico/pull/236/files#diff-abc34f8fca591a33d5542a8d4e166308R14 i can see that your still using btn classes. Also https://github.com/veda-consulting/uk.co.vedaconsulting.mosaico/pull/236/files#diff-7b4adde1160e4928bf27ff71a1781b6eR8

Perhaps that is intentional but i would also suggest whatever markup is there for the crm.* in Mosaico should be synced to core right? that way then at least people might get a standard approach

[totten]

Oh, you're right about veda-consulting-company/uk.co.vedaconsulting.mosaico#236 using btn. I'd forgotten about that.

The r-code critique applies equally to my Mosaico PR. A new developer reading ang/crmMosaico.crmstar/BlockPreview.html would be getting mixed-signals about the coding convention.

The r-tech situation has been mitigated in the Mosaico PR -- because mosaico-crmstar.css defines rules for .crm-mosaico-page { .btn { ...} } (https://github.com/veda-consulting/uk.co.vedaconsulting.mosaico/pull/236/files#diff-5f7f90af70b65ab3c02cbc63f92fe27e). This should provide some high(ish)-priority styling to make the layout less ambiguous. It's not as powerful as Shoreditch's defenses, and it's not a pattern I'd care to repeat a lot, but it's a mitigation.

IIRC, in developing veda-consulting-company/uk.co.vedaconsulting.mosaico#236, I started out with a pretty messy split between Bootstrap and crmstar content, and it took a while to get a clean separation of the folders. But now that they are separated, I think it'd be easy to rename btn (e.g. s/btn/crm-mosaico-btn/ in any crmstar files) -- which should address both both r-code and r-tech.

[seamuslee001]

@totten @colemanw i have altered this now to be more namespaced classes, This would mean anyone adding a bootstrap theme onto it wouldn't get any issues as these classes aren't in the standard bootstrap markup. further more as with the crm-ui-icon directive adding crm-button class onto everything the only ones before this that are different in class style is the submit button ones, Everything else gets the same styling when delete and save draft are different to that of the submit, this allows themes like shoreditch to style them differently as needed

[eileenmcnaughton]

test this please

[seamuslee001]

Jenkins re test this please

[eileenmcnaughton]

@guanhuan can you maybe get someone to take a look - unless @totten or @colemanw feel they can make a call on @seamuslee001 last update

[totten]

OK, the last update addressed my r-code style concerns.

Once the tests pass, it should be mergeable. (To be safe, I'd suggest opening the autobuild test site and glancing at the "New Mailing" screen -- but that's a really small barrier.)

[seamuslee001]

@eileenmcnaughton i have busted caches locally on an AUG test box opened up dev tools confirmed the page loaded confirmed the new markup was there and confirmed no ui changes occurred because of the new markup.

[eileenmcnaughton]

Per most recent comments - adding merge-on-pass

[seamuslee001]

Merging as per the tag