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

Blacklist border-radius property (use mixin instead) #27900

Merged
merged 3 commits into from
Dec 23, 2018

Conversation

MartijnCuppens
Copy link
Member

Continuing on #27883 (comment). Blacklist the border radius property in stylelint, so we don't accidentally add this like happend with the toasts.

@@ -200,10 +200,12 @@

.card-img-top,
.card-header {
// stylelint-disable-next-line property-blacklist
border-top-right-radius: 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if we should add a mixin for border-top-right-radius and the other specific border radii?

Copy link
Member

Choose a reason for hiding this comment

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

Eh, probably not necessary? It'd save on a bit of CSS when folks compile without rounded corners enabled, but probably not worth it right now.

@@ -144,6 +144,7 @@

.custom-radio {
.custom-control-label::before {
// stylelint-disable-next-line property-blacklist
Copy link
Member Author

Choose a reason for hiding this comment

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

I guess the custom radios should always be round, independent of $enable-rounded? That's why I didn't use the mixin here.

} @else {
border-radius: 0;
}
@include border-radius($custom-select-border-radius, 0);
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't really like the if/else statement here, so I tweaked the border-radius mixin a bit to simplify this code here.

@XhmikosR XhmikosR merged commit 39b14c9 into v4-dev Dec 23, 2018
@XhmikosR XhmikosR deleted the v4-dev-martijncuppens-border-radius branch December 23, 2018 07:11
@mdo mdo mentioned this pull request Dec 23, 2018
bkdotcom added a commit to bkdotcom/bootstrap that referenced this pull request Jan 3, 2019
* v4-dev: (56 commits)
  Remove unnecessary brackets for consistency (twbs#27966)
  Fix Typo on docs (twbs#27962)
  Add a few more redirects.
  Remove wrapping paragraph in readme (twbs#27948)
  Configurable display utility classes (twbs#27917)
  Fix custom select font sizes (twbs#27929)
  Match input font size for `.input-group-text` (twbs#27941)
  reboot: Fix comment (twbs#27937)
  Fix form-feedback-icon-invalid color (twbs#27935)
  toasts: mention that positioning is manual. (twbs#27931)
  stretched-link.md: fix typo (twbs#27918)
  Add Open Collective to header (twbs#27916)
  Add horizontal card example (twbs#27906)
  getSelectorFromElement return null on bad selectors (twbs#27912)
  versions: sort from newer to older. (twbs#27898)
  Fix 4:3 embed (twbs#27910)
  Simplify card group css (twbs#27901)
  Blacklist border-radius property (use mixin instead) (twbs#27900)
  move to 4.2
  bootstrap-stack.svg: remove unneeded space.
  ...
pascalpepe referenced this pull request in pascalpepe/bootstrap-webpack-template Jan 1, 2020
* Upgrade Bootstrap to version 4.3.1.
* Upgrade the following development dependencies:

  - **autoprefixer** to version 9.4.7
  - **nodemon** to version 1.18.10"
  - **stylelint** to version 9.10.1"
  - **stylelint-config-twbs-bootstrap** to version 0.3.0

* Move PostCSS config file from the ``build/`` directory to the repository
  root.
* Added new rules in the config of **stylelint** in order to blacklist some
  ``border-radius`` properties (use Bootstrap's mixin instead, see
  `Bootstrap issue #27900 <https://github.com/twbs/bootstrap/pull/27900>`_).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants