-
Notifications
You must be signed in to change notification settings - Fork 54
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
Release Boosted 5.1.0 #772
Conversation
scss/mixins/_utilities.scss
Outdated
// FIXME: color-contrast($value) throws an error | ||
// @if "background-color" == $property and "transparent" != inspect($value) { | ||
// @debug "#{$property}: #{$value}"; | ||
// color: color-contrast($value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$value
is not anymore only an hexadecimal value; it can be for example rgba(var(--bs-body-rgb), var(--bs-text-opacity))
so color-contrast($value)
(in reality luminance()
) throws the following error: Error: $color: rgba(var(--bs-primary-rgb), var(--bs-text-opacity)) is not a color.
Any help would be appreciated on this subject. For the moment, I struggle a little bit with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the safer way would be to parse the value as a string to get the color name (eg. primary
) and call the corresponding Sass variable ($primary
).
Not quite sure it'd work though, but give it a try 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ffoodd.
I'm not sure it would work but I'll give it a try.
Indeed, if I'm not mistaken, there won't be corresponding Sass variable all the time:
var(--bs-primary-rgb)
->$primary
var(--bs-body-rgb)
->$body-bg
(not the same name)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using the css variables, current solution can't be still valid. If the user change the css variable value (it has been done for that) then color contrast calculated during sass compilation can be wrong.
Maybe we need to give a try to pure css luminance calculation. I've found that interesting ressource : https://css-tricks.com/switch-font-color-for-different-backgrounds-with-css/ don't know what will be the effort to convert our variable and implement it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't matter, this check was done only for Orange brand colors. Boosted is not supposed to support any other color palette.
As a matter of fact, custom property names shouldn't matter either: simply check if it contains any of the theme colors, grab it, and get its contrasted color.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got a naive code that seems to work (and that could probably be improved) but I'm not sure to be in the right direction given your last comment @ffoodd.
// Boosted mod: ensure contrasts in color utilities
@if "background-color" == $property and "transparent" != inspect($value) {
// @debug "#{$property}: #{$value}";
@if type-of($value) == "string" {
@if (str-index($value, "var(--bs-primary-rgb)")) {
$value: $primary;
}
@else if (str-index($value, "var(--bs-secondary-rgb)")) {
$value: $secondary;
}
@else if (str-index($value, "var(--bs-success-rgb)")) {
$value: $success;
}
@else if (str-index($value, "var(--bs-info-rgb)")) {
$value: $info;
}
@else if (str-index($value, "var(--bs-warning-rgb)")) {
$value: $warning;
}
@else if (str-index($value, "var(--bs-danger-rgb)")) {
$value: $danger;
}
@else if (str-index($value, "var(--bs-light-rgb)")) {
$value: $light;
}
@else if (str-index($value, "var(--bs-white-rgb)")) {
$value: $white;
}
@else if (str-index($value, "var(--bs-body-rgb)")) {
$value: $body-bg;
}
@else if (str-index($value, "var(--bs-dark-rgb)")) {
$value: $dark;
}
@else if (str-index($value, "var(--bs-black-rgb)")) {
$value: $black;
}
}
color: color-contrast($value);
} @else if "color" == $property and "inherit" != inspect($value) and $accessible-orange != $value and (type-of($value) == "string" and null == (str-index($value, "var(--bs-primary-rgb)"))) {
// @debug "#{$property}: #{$value}";
@if type-of($value) == "string" {
@if (str-index($value, "var(--bs-white-rgb)")) {
$value: $white;
}
@else if (str-index($value, "var(--bs-light-rgb)")) {
$value: $light;
}
@else if (str-index($value, "var(--bs-body-rgb)")) {
$value: $body-color;
}
}
background-color: color-contrast($value);
}
// End mod
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be simplified indeed:
- first extract the custom property name's middle key:
$name = str-slice($value, 10, -6)
to dropvar(--bs-
(9 chars) and-rgb)
(5 chars). - then get the key from
$theme-colors
if it exists:@if map-has-key($theme-colors, $name) { $value = $map-get($theme-colors, $name); }
.
Didn't test this out but you get the idea :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplified it this way: 78df309
Just coming back and seeing commit names, what is it with them ? These are Bootstrap commits ? |
Yep, it is easier to track for me during the development and for the review. |
8321c30
to
65e90a8
Compare
c5d1508 fixes pa11y issues. Here is my proposal:
The main goal of my proposal is to have a "green" pa11y for the |
} | ||
#{$property}: $value if($enable-important-utilities, !important, null); | ||
|
||
// Boosted mod: ensure contrasts in color utilities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There must be some kind of regression here because pa11y shows us problems related to text-muted
(e.g. https://deploy-preview-772--boosted.netlify.app/docs/5.0/examples/checkout/ vs https://boosted.orange.com/docs/5.0/examples/checkout/ where text-muted
was displayed with a white bg).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed via 3e1e3da
Find in https://deploy-preview-772--boosted.netlify.app/docs/5.0/layout/css-grid/#customizing
We need to update the doc |
3041bf9
to
2c8a87b
Compare
54fca49
to
9b22548
Compare
Tasks
aria-label="Close"
and replace it by<span class="visually-hidden">Close</span>
inside the buttonsAt the end, regenerate thepackage-lock.json
Release
npm run release-version $current_version $next_version
to bump version numberversion_short
inpackage.json
site/data/docs-versions.yml
docs_version
inconfig.yml
and other references to the previous versionsite/content/docs/_index.html
?)site/content/docs/5.x
tosite/content/docs/5.x+1
site/static/docs/{version}
versionnuget/boosted.nuspec
CHANGELOG.md
, and maybesite/content/docs/<version>/migration.md
package-lock.json
content, only "boosted" should have its version changed!.scss
main files (reboot, grid, utilities and main file) as well as inNOTICE.txt
.npm run release
to compile dist, update SRI hashes in doc and package the releaseconventional-changelog-cli
globallyconventional-changelog -p angular -i CHANGELOG.md -s
dist
with achore(release)
commit messagev4-dev
orv5-dev
)runbuild/ship.sh x.x.x
to… (v4 only)npm pack
thennpm publish
.npmrc
file--tag
, eg for v5-alpha1npm publish boosted-5.0.0-alpha1.tgz --tag next
gh-pages
:../_site
(v5-dev) or../bs-docs
(v4-dev) to thegh-pages
branchindex.html
used as redirections to be redirecting to the new releasedist
URLs in examples' HTML has changed