Skip to content

Conversation

@MartijnCuppens
Copy link
Member

@MartijnCuppens MartijnCuppens commented Apr 11, 2020

This PR simplifies the checkboxes a bit. A lot of properties are very unlikely to change, like background-position or background-repeat, so I think it's better to remove them from the variables file to keep things clear.

  • Define background properties in .form-check-input
  • Use background-size: contain to simplify background sizes (especially for those with linear gradients)
  • Remove double color-adjust
  • Adjust svgs to 20x20 box. This way the strokes aren't rescaled.

Also tested this with the gradients enabled.

Fixes #30528 (comment)

Demo: https://deploy-preview-30557--twbs-bootstrap.netlify.com/docs/4.3/forms/checks/

- Define background properties in `.form-check-input`
- Use `background-size: contain` to simplify background sizes (especially for those with linear gradients)
- Remove double `color-adjust`
- Adjust svgs to 20x20 box. This way the strokes aren't rescaled.
@MartijnCuppens
Copy link
Member Author

MartijnCuppens commented Apr 14, 2020

Ping @twbs/css-review for review

Copy link
Member

@ffoodd ffoodd left a comment

Choose a reason for hiding this comment

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

LGTM. Being curious again, is there a reason not to use background shorthand here? Seems pretty safe in that case, isn't it?

@MartijnCuppens
Copy link
Member Author

MartijnCuppens commented Apr 14, 2020

We try to live with the rule: Only use shorthand properties if all properties are covered. background is a shorthand of 8 background properties.

Another thing, someone might have configured something like background-position: inherit, in the shorthand, inherit doesn't work.

I also once came across an issue where someone used custom properties, similar to the inherit issue.

Also, I do prefer clarity over a negligible improvement in filesize

@ffoodd
Copy link
Member

ffoodd commented Apr 14, 2020

Only use shorthand properties if all properties are covered.

That's a good one :)

@MartijnCuppens MartijnCuppens merged commit ed8fd68 into master Apr 14, 2020
@MartijnCuppens MartijnCuppens deleted the master-mc-simplify-checkbox-radio branch April 14, 2020 13:05
@mdo
Copy link
Member

mdo commented Apr 14, 2020

Missed this entirely, nice!

olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
- Define background properties in `.form-check-input`
- Use `background-size: contain` to simplify background sizes (especially for those with linear gradients)
- Remove double `color-adjust`
- Adjust svgs to 20x20 box. This way the strokes aren't rescaled.
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.

4 participants