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

Convert border utilities to CSS variables #35428

Merged
merged 3 commits into from Feb 28, 2022
Merged

Convert border utilities to CSS variables #35428

merged 3 commits into from Feb 28, 2022

Conversation

mdo
Copy link
Member

@mdo mdo commented Nov 30, 2021

  • Updates the utilities mixin to check for specific CSS variable names via css-variable
  • Bonus fix: we now prevent local variables for 0 value utilities (e.g., .border-top-0 no longer sets --bs-border-opacity: 1
  • Adds new .border-opacity-* classes
  • Adds new root variables: --bs-border-color, --bs-border-style, --bs-border-width
  • Documents the new variable changes
  • Replaces instances of the Sass variables with their new CSS variables

Todo:

  • Clean up language and comments in the diff

@mdo mdo added this to In progress in v5.3.0 via automation Nov 30, 2021
@mdo mdo marked this pull request as ready for review November 30, 2021 06:41
@mdo mdo requested a review from a team as a code owner November 30, 2021 06:41
scss/_utilities.scss Outdated Show resolved Hide resolved
@alecpl
Copy link
Contributor

alecpl commented Nov 30, 2021

This #{$variable-prefix} everywhere is kinda distracting. Have you considered something like https://dev.to/felipperegazio/css-custom-properties-vars-with-sass-scss-a-practical-architecture-strategy-1m88 ?

@mdo
Copy link
Member Author

mdo commented Nov 30, 2021

Yup, that's coming next :).

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.

Those are nice additions, almost there 👌

site/content/docs/5.1/utilities/borders.md Outdated Show resolved Hide resolved
site/content/docs/5.1/utilities/api.md Outdated Show resolved Hide resolved
@mdo
Copy link
Member Author

mdo commented Feb 15, 2022

Changes addressed @ffoodd, ready for another review :D.

@mdo mdo requested a review from ffoodd February 15, 2022 01:09
@mdo mdo added this to In progress in v5.2.0 via automation Feb 24, 2022
@mdo mdo mentioned this pull request Feb 24, 2022
41 tasks
@mdo mdo force-pushed the css-vars-borders branch 2 times, most recently from 03df7d8 to 9ecdfcf Compare February 25, 2022 22:57
@mdo
Copy link
Member Author

mdo commented Feb 25, 2022

Added another commit to replace instances of $border-color and $border-width with their new CSS variable counterparts.

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.

One of the new sentence sounds weird, but apart from that it works great!

site/content/docs/5.1/utilities/api.md Outdated Show resolved Hide resolved
v5.2.0 automation moved this from In progress to Reviewer approved Feb 28, 2022
- Updates the utilities mixin to check for specific CSS variable names via `css-variable`
- Bonus fix: we now prevent local variables for `0` value utilities (e.g., `.border-top-0` no longer sets `--bs-border-opacity: 1`
- Adds new `.border-opacity-*` classes
- Adds new root variables: `--bs-border-color`, `--bs-border-style`, `--bs-border-width`
- Documents the new variable changes
@mdo mdo merged commit efc5914 into main Feb 28, 2022
v5.2.0 automation moved this from Reviewer approved to Done Feb 28, 2022
@mdo mdo deleted the css-vars-borders branch February 28, 2022 19:40
v5.3.0 automation moved this from In progress to Done Feb 28, 2022
@mdo mdo removed this from Done in v5.3.0 Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v5.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants