Checkbox / Radio Button: Improve HTML whitespace sensitivity #4286
Conversation
Avoids issues where HTML spacing is sensitive between the checkmark pseudo-element and label text.
Since otherwise the end-result could possibly be an invalid size token
| line-height($theme-form-font-family, $theme-input-line-height) * | ||
| font-size($theme-form-font-family, $theme-body-font-size) - |
There was a problem hiding this comment.
This may need a rethink, since it assumes font styles inherited from .usa-fieldset, which are likely not guaranteed.
uswds/src/stylesheets/elements/form-controls/_global.scss
Lines 32 to 35 in f3bdc64
There was a problem hiding this comment.
This is a situation where it would be nice to have the CSS4 lh unit, since essentially what we want is margin-top: calc((1lh - #{units($theme-input-select-size)}) / 2);.
A couple alternatives to consider:
- Maybe the checkbox should explicitly set font expectations, which might be desirable anyways. i.e. Add the
%block-input-generalplaceholder tousa-checkbox. - Another I'd hoped could work is to chop up the elements so that the natural height of the label is only the label text excluding the description, so that a standard vertical centering approach could work. I've got this close to working in a CodePen prototype, but positioning the description absolutely means that it won't "push down" content that occurs after the checkbox.
There was a problem hiding this comment.
@aduth have you tried Flexbox? Seems similar to the media object pattern [solved by flexbox - philipwalton.github.io]
There was a problem hiding this comment.
@aduth have you tried Flexbox? Seems similar to the media object pattern [solved by flexbox - philipwalton.github.io]
Yeah, if we were starting fresh, flexbox could be a great option, but since most of the approaches I tried with flexbox would involve the addition of a new wrapper element (like the .Media-body element in those demos), I figured it best to find an alternative which remains backward-compatible.
There was a problem hiding this comment.
- Maybe the checkbox should explicitly set font expectations, which might be desirable anyways. i.e. Add the
%block-input-generalplaceholder tousa-checkbox.
I went this direction in 9e958ae, based on there being some precedent of this applying at the component level [1][2][3][4], where presumably it's expected that the checkbox and radio could (or should) inherit from the input-related typography settings.
So that checkbox positioning is accurate based on explicit font expectations
**Why:** Avoid future maintenance overhead incurred by version drift, incorporate latest features and updates, including upstream contributions previously implemented as overrides. Changelog: - https://github.com/uswds/uswds/releases/tag/v2.12.1 - https://github.com/uswds/uswds/releases/tag/v2.12.0 Incorporated revisions: - uswds/uswds#4199 - uswds/uswds#4286
**Why:** Avoid future maintenance overhead incurred by version drift, incorporate latest features and updates, including upstream contributions previously implemented as overrides. Changelog: - https://github.com/uswds/uswds/releases/tag/v2.12.1 - https://github.com/uswds/uswds/releases/tag/v2.12.0 Incorporated revisions: - uswds/uswds#4199 - uswds/uswds#4286
**Why:** Avoid future maintenance overhead incurred by version drift, incorporate latest features and updates, including upstream contributions previously implemented as overrides. Changelog: - https://github.com/uswds/uswds/releases/tag/v2.12.1 - https://github.com/uswds/uswds/releases/tag/v2.12.0 Incorporated revisions: - uswds/uswds#4199 - uswds/uswds#4286
3 participants
Description
This pull request seeks to maintain the existing visual appearance of the Checkbox and Radio Button components, while improving developer ergonomics regarding HTML whitespace sensitivity.
Context: 18F/identity-idp#5279 (comment)
Additional information
Because the current implementation styles the icon pseudo-element and label text as adjacent inline elements, it suffers from a common issue with unintended whitespace between the elements (see related blog post). For this reason, the code must be marked up very carefully to avoid additional whitespace within the label class (including newlines), as otherwise the text alignment between label and description is misplaced, seen in the screenshots below:
There are a few ways this could be addressed, including using Flexbox, though most of these alternative solutions would require backwards-incompatible changes to the component markup, such as the introduction of a new element to wrap the label text. A Flexbox approach can almost work with the existing markup through a combination of
flex-wrap: wrapandflex-basis: 100%on the description element, but unfortunately this implementation fails when the label text is especially long.Positioning absolutely is mostly straight-forward, with one exception: Because it is no longer in line with the label text, the checkmark icon must be manually positioned to restore the appearance of vertical alignment. The proposed implementation here uses some extensive math to achieve this, tested in the demo samples with default font size and with an adjusted
$theme-body-font-size: "xl";. Ideally there'd be a simpler solution to this, though I could not come up with one.Before you hit Submit, make sure you’ve done whichever of these applies to you:
npm testand make sure the tests for the files you have changed have passed.