Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve bootstrap3 checkbox theming #19006

Merged
merged 1 commit into from
Nov 25, 2020
Merged

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Nov 20, 2020

Overview

Adds some reusable styling for checkboxes in Bootstrap.

Before

image

After

Checkboxes appear in form-control styled boxes. Their label text is bold when checked.

image

Technical Details

This moves us toward the following markup as a standard:

<div class="checkbox-inline form-control">
  <label>
    <input type="checkbox"> <span>The Label</span>
  </label>
</div>

This differs from vanilla bootstrap markup in 2 ways:

  1. The added class form-control on the outer div. It's kinda hacking the theme a little bit, but it adds a nice box around the checkbox & label that visually matches other form elements.
  2. The extra <span> is so css rules can style the label text based on the checkbox state (using the + operator).

Both of these tweaks are compatible with vanilla bootstrap themes and do not require extra css (it just won't get the cool text effect).

@civibot
Copy link

civibot bot commented Nov 20, 2020

(Standard links)

@totten
Copy link
Member

totten commented Nov 21, 2020

This moves us toward the following markup as a standard:

<label class="form-control">
  <input type="checkbox"> <span>The Title</span>
</label>

The extra <span> is so css rules can style the label text based on the checkbox state (using the + operator)

OK, I think adding the <span> in the markup is mostly harmless. If you mixed this markup with a non-Greenwich theme, the <span> should have no effect. On the flip-side, if Greenwich required the <span>, then that might be problematic (when mixing Greenwich with more conventional markup). But if the <span> is optional, then great.

To my reading of all the form examples from BS3, there are a couple problems:

  • The use of <label class="form-control"> is incorrect. They're pretty consistent in only applying form-control to <input type=text>, <textarea>, <select>.
  • To combine an <input type=checkbox> (or <input type=radio>) with its label, they use a <div> with one of these classes: checkbox, checkbox-inline, radio, or radio-inline. So, eg, <div class="checkbox"> is the container for checkbox+text.

It took me a bit to feel like I grokked the official BS3 guide. The BS3 guide isn't quite as clear about some definitions as the BS4/BS5 guides. I looked for an alternative BS3 guide, and this one is probably my favorite because the excerpts are framed more consistently/equivalently, and it speaks to some more edge-cases.

But I think maybe an infographic can summarize it better. In these pictures, the sections are color-coded (e.g. a green tag corresponds to a green region).

Form (Horizontal) On a wide/canonical screen, each label is horizontally adjacent to its input element. (On a narrow screen, they're split onto separate lines.)

BS3-Horiz

Form (Inline) One a wide/canonical screen, all labels and input elements form a continuous line.

BS3-Inline

@colemanw
Copy link
Member Author

@totten thanks for the feedback. I've tweaked my PR to use the native bootstrap checkbox-inline class.
Now the markup works on vanilla bootstrap css.
I understand that adding the form-control class to the checkbox-inline div is hacking the bootstrap theme a little bit, but, it works! And it's a nice effect.

@totten
Copy link
Member

totten commented Nov 24, 2020

For comparison, here's a screenshot from my local dmaster (Garland+Greenwich) of APIv4 Explorer with the extra class="form-control" (2c410cb):

Screen Shot 2020-11-23 at 4 40 59 PM

And without it:

Screen Shot 2020-11-23 at 4 43 43 PM

FWIW, in Seven+Greenwich and Seven+Shoreditch, the checkboxes have the same problem. (The tangential issue about action-buttons is worst in Garland+Greenwich; it's not-as-bad/not-good in Seven+Greenwich; it's fine in Seven+Shoreditch.)

(All observations in Firefox 82.)

Aside: in #19005, when I used a toggle <button> with coloring, it was sort-of a 'make lemonade from lemons' thing. But if we can get basic checkboxes to look fine, then we don't need to change the appearance (for my purposes).

Standardizes our styling of checkboxes in boostrap markup to:
`<label class="form-control"><input type="checkbox"> <span>Title</span></label>`
@totten
Copy link
Member

totten commented Nov 24, 2020

Yeah, that does look a little better:

Screen Shot 2020-11-23 at 7 09 33 PM

If you're aiming for that look/feel, then I can see how form-control is short/easy and how it's guaranteed to match the styling of text fields. You don't have to worry that (eg) someone changes the border/padding on the text-field but forgets to update the checkbox widget.

Personally, for me, the look/feel gets into an "uncanny valley" - it seems to me that most UIs with a toggle would either be less stylized (plain text) or more stylized (eg changing color-scheme or moving icons based on selection-status). But that's pure bike-shedding... and I'll get used to it in a day.... and I don't want to block cleanup on bike-shedding the layout.

But let's put ourselves in the shoes of a themer aiming to use another BS3 theme (where the visual language of the checkbox is more traditional). These 3 checkboxes are going to look different (more-text-input-like) than any other checkbox on the site, and it's going to require an extra intervention to undo the effects of form-control, and they'd (rightly) feel that our markup diverged from the style-guide.

Here's an alternative way to make sure the styles match-up -- target .checkbox-inline and use the same variables as .form-control. (Pushed up a patch to do this.) This should make it easier to switch among themes -- and it still provides the control-ish appearance on Greenwich. This looks the same to me (Seven+Greenwich):

Screen Shot 2020-11-23 at 10 03 31 PM

Screen Shot 2020-11-23 at 10 23 07 PM

// height: $input-height-base; // Make inputs at least the height of their button counterpart (base line-height + padding + border)
padding: $padding-base-vertical $padding-base-horizontal $padding-base-vertical (20+$padding-base-horizontal);
// font-size: $font-size-base;
// line-height: $line-height-base;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are copied from the .form-control rule. I commented out a few blurbs because I was checking to see which ones were really necessary. Could flip a coin on whether it's better to reproduce or remove these bits.

@colemanw colemanw merged commit 7166e1d into civicrm:master Nov 25, 2020
@colemanw colemanw deleted the bsCheckbox branch November 25, 2020 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants