-
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
chore(merge): fc3f4b6 #1757
chore(merge): fc3f4b6 #1757
Conversation
c21f67b
to
4ef9b9a
Compare
f718249
to
2d5a8db
Compare
2d5a8db
to
5ee422d
Compare
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
81c73bd
to
795a5f9
Compare
795a5f9
to
fd5a983
Compare
@@ -135,7 +135,7 @@ hr { | |||
font-style: $headings-font-style; | |||
font-weight: $headings-font-weight; | |||
line-height: $headings-line-height; | |||
color: $headings-color; | |||
color: var(--#{$prefix}heading-color, inherit); |
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.
[Bootstrap side] Couldn't we initialize this variable to inherit
instead of null
? Bs side ?
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.
See twbs/bootstrap#37904.
@@ -336,11 +336,11 @@ sup { top: -.5em; } | |||
// Links | |||
|
|||
a { | |||
color: var(--#{$prefix}link-color); | |||
color: rgba(var(--#{$prefix}link-color-rgb), var(--#{$prefix}link-opacity, 1)); |
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.
[Bootstrap side] Same question for link-opacity, BS side
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.
See twbs/bootstrap#37904.
@@ -163,6 +163,11 @@ $utilities: map-merge( | |||
), | |||
values: $utilities-border-colors | |||
), | |||
"subtle-border-color": ( | |||
property: border-color, | |||
class: border, |
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.
[Bootstrap side] No opacity as in "border-color"
same for text-color.
Why not merge both maps ?
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.
Comes from the fact that this map is hexa code and the one above is rgb. Rethinking of this, I wonder if should propose it or not 🤷
@@ -36,6 +69,17 @@ $utilities-text: ( | |||
"body": to-rgb($body-color), | |||
) !default; | |||
$utilities-text-colors: map-loop($utilities-text, rgba-css-var, "$key", "text") !default; | |||
|
|||
$utilities-text-emphasis-colors: ( | |||
"primary-emphasis": var(--#{$prefix}primary-text), |
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.
[Bootstrap side] Why no emphasis
inside the variable name ? Bs side, change variables, variables-dark, doc, maps, root
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.
See twbs/bootstrap#37907.
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.
We must not forget what'd be reintegrated into Boosted until we add the dark mode support.
Don't forget the migration guide.
[Bootstrap side] Need to check on Bs for $body-bg
, $body-color
, $border-radius
, etc...
Kudos, SonarCloud Quality Gate passed!
|
This PR integrates the first huge colors mode commit from Bootstrap: twbs/bootstrap@fc3f4b6.
Live preview
How to test this PR
Approach
Let me try to summarize the changes and what I've done...
.bundlewatch.config.json
: nothing to do here since we don't have any dark mode for accordionsscss/_accordion.scss
scss/_alert.scss
: nothing to do here since we don't have any dark mode for carousels nor dark variantsscss/_carousel.scss
: nothing to do here since we don’t have any dark mode for button closescss/_close.scss
scss/_dropdown.scss
scss/_list-group.scss
—bs-list-group-action-hover-bg
which uses$list-group-hover-bg
added in _variables.scss and set background-color to use this custom propscss/_maps.scss
: exact same changes as Bootstrap. All these maps are used to create utilities and we want the same in Boosted.scss/_mixins.scss
: integrated as is: nothing to do here since we don't have any dark mode for navbarsscss/_navbar.scss
: nothing to do, was already done in a previous PRscss/_pagination.scss
scss/_reboot.scss
: reintroduced everything as isscss/_root.scss
--*-dark
with--*-inverted
in the dark variant selectorscss/_utilities.scss
scss/_utilities.scss
to avoid Sass compilation errors when colors where defined with CSS varsbody-secondary
,body-tertiary
andbody-emphasis
for text colors as isbody-secondary
,body-tertiary
andbody-emphasis
for text colors as is + removedbody
andwhite
which are already generated from$utilities-bg-colors
subtle-background-color
$utilities-text-emphasis-colors
as isscss_variables_dark.scss
$form-*
and$accordion-*
vars since we won't need to handle these specificities without dark mode in Boostedscss/_variables.scss
${color}-text
but by respecting the text colors already existing in Boosted (so no colors for danger, warning, etc.)${color}-bg-subtle
but replaced all the values by the same as non-subtle values${color}-border-subtle
but replaced all the values by the same as non-subtle values$enable-dark-mode
and$color-mode-type
have been introduced$enable-dark-mode
should befalse
by default + explanation of why in the description of this PR + check thatdist/*
generated files are not impacted (not growing)$*-dark
variables have been renamed to$*-inverted
for dark variant.$body-color
and$body-bg
have been moved (kept the same values)$body-emphasis
and$emphasis-color
introduced as is$body-tertiary-color
and$body-tertiary-bg
as is$body-secondary-bg
changes its value to$gray-300
because used by disabled form controls which had this value$text-muted
now has—bs-secondary-color
as value so it means that$body-secondary-color
must take the old value of$text-muted
->$gray-700
$box-shadow*
staynull
$input-bg
can use—bs-form-control-bg
since it’s defined by—bs-body-bg
and that$input-bg
was previously defined by$body-bg
which hasn’t changed in this PR$input-disabled-bg
can use—bs-form-control-disabled-bg
because it depends on$body-secondary-bg
value which now takes the same value as$input-disabled-bg
before$input-border-color
now uses—bs-color-translucent
as other components with non-black borders. That’s why we don’t use—bs-border-color
which is black in Boosted by default$input-placeholder-color
can safely by transformed to—bs-secondary-color
which depends on$body-secondary-color
which is$gray-700
$form-range-thumb-disabled-bg
must remain$gray-500
. Could$gray-500
define—bs-tertiary-bg
?$gray-500
would deserve a custom property for disabled-colour ; maybe $gray-500 could be —bs-tertiary-bg -> $body-tertiary-colorscss/bootstrap-grid.scss
: reintroduced as isscss/bootstrap-reboot.scss
: reintroduced as isscss/bootstrap-utilities.scss
: reintroduced as isscss/bootstrap.scss
: reintroduced as isscss/forms/_form-check.scss
scss/forms/_form-select.scss
Introduced exactly the same modification except the
@if $enable-dark-mode
part at the endscss/mixins/_alert.scss
: integrated as isscss/mixins/_color-mode.scss
scss/mixins/_forms.scss
--#{$prefix}form-select-bg-icon: #{escape-svg($icon)};
and remove of background-imagescss/mixins/_list-group.scss
: same modificationsite/assets/js/color-modes/index.js
: nothing to do here, we don't have this filesite/assets/scss/_ads.scss
: nothing to do since we don’t have anysite/assets/scss/_brand.scss
.bd-brand-item
in Boosted docs: nothing to do in this file but sincesite/assets/scss/_buttons.scss
.btn-bd-light
wasn’t defined anywhere, I’ve removed its usage fromsite/layouts/_default/docs.html
site/assets/scss/_callouts.scss
: Introduced the same architecture and changed some colors so that there's no regression on light mode and so that it works a bit more on dark modesite/assets/scss/_clipboard-js.scss
color: var(--bs-body-color);
background: transparent
and removedbg-transparent
from the markup for consistency and easy maintenance with Bootstrapcolor: var(--bs-link-hover-color);
instead ofcolor: var(--bs-link-color);
since it is probably an error in Bootstrap. Otherwise, it won't work in our case. (see Docs: fix.btn-clipboard
and.btn-edit
link hover color twbs/bootstrap#37823)site/assets/scss/_component-examples.scss
.bd-example-row [class^="col"], +.bd-example-cols [class^="col"] > *, .bd-example-cssgrid [class*="grid"] > * {
selectorsite/assets/scss/_content.scss
: introduced most of it as is + some Boosted mod or removed stuff[data-bs-theme="blue”]
bd-summary-link
since not useful ingetting-started/introduction.md
page: just enhanced the comment to precise that we already use our footer component and not a specific stylesite/assets/scss/_footer.scss
site/assets/scss/_masthead.scss
: integrated mostly as issite/assets/scss/_navbar.scss
: Mostly handled the changes by adding Boosted mods since we use our Orange navbar componentsite/assets/scss/_sidebar.scss
: gathered mostly the same modificationssite/assets/scss/_syntax.scss
site/assets/scss/_toc.scss
: integrated the.bd-toc-collapse
since the other modification is based on a block which doesn’t exist in Boostedsite/assets/scss/_variables.scss
: Integrated the structure, removed$dropdown-active-icon
and mostly Boosted moded elements unused in Boosted: Boosted has a different content so this change cannot be appliedsite/content/docs/5.2/about/brand.md
: nothing to do here because we don't have flush accordions in Boostedsite/content/docs/5.2/components/accordion.md
site/content/docs/5.2/components/alerts.md
: integrated as is: nothing to do here because we don't have any dark variant for carouselssite/content/docs/5.2/components/carousel.md
: nothing to do here because we will keep our dark variant close buttonsite/content/docs/5.2/components/close-button.md
site/content/docs/5.2/components/dropdowns.md
site/content/docs/5.2/components/list-group.md
: integrated as is except message to change $theme-colors to $background-colors used by Boosted contrary to Bootstrapsite/content/docs/5.2/components/modal.md
: integrated as issite/content/docs/5.2/components/navbar.md
No need to change
.bg-light
to.bg-body-tertiary
since we didn’t use.bg-light
in the first placeNo need to add a warning callout about the depreciation of
.navbar-dark
since it will still be effective in Boostedsite/content/docs/5.2/components/offcanvas.md
: integrated as is: nothing to do here since we prefer to keep a white bg instead of a grayish bg in this casesite/content/docs/5.2/components/scrollspy.md
site/content/docs/5.2/components/toasts.md
: integrated as issite/content/docs/5.2/content/reboot.md
By checking the values, added some Boosted mods in scss/_variables.scss for $spacer and $headings-margin-bottom
Integrated the content as is except for the
margin-bottom
value mentioned in the Headings description which is slightly different by default in Boosted: chose not to integrate this message since the dark mode is not yet available in Boosted so IMO the doc can stay as is for now without this informationsite/content/docs/5.2/content/tables.md
site/content/docs/5.2/customize/color-modes.md
site/content/docs/5.2/customize/color.md
site/content/docs/5.2/customize/css-variables.md
: introduced as issite/content/docs/5.2/customize/options.md
site/content/docs/5.2/customize/sass.md
_variables-dark.scss
is optional in Bootstrap/Boosted_variables-dark.scss
is not necessary for now. But does it matter? If the bundle size doesn’t change, that’s not really an issue isn’t it?site/content/docs/5.2/forms/form-control.md
: introduced as is: nothing to do here since we probably want to keep the same rendering. Meaning thatsite/content/docs/5.2/getting-started/introduction.md
.bd-summary-link
is useless and doesn’t need to be integrated insite/assets/scss/_content.scss
site/content/docs/5.2/helpers/stacks.md
: integrated as is; changed all.bg-light
to.bg-body-tertiary
site/content/docs/5.2/helpers/stretched-link.md
: integrated as issite/content/docs/5.2/helpers/vertical-rule.md
: replaced .bg-light with .bg-tertiary.bg-tertiary
+ border for consistencysite/content/docs/5.2/layout/columns.md
: introduced exactly as issite/content/docs/5.2/layout/gutters.md
: introduced exactly as issite/content/docs/5.2/migration.md
site/content/docs/5.2/utilities/background.md
:.bg-body-secondary
and.bg-body-tertiary
.bg-body-secondary
and.bg-body-tertiary
are allowed. Otherwise either change their color or don't display them and mention them in the callout.bg-light
has been changed into.bg-body-tertiary
rather everywhere in the docs!site/content/docs/5.2/utilities/borders.md
Instead of diplaying the renderings, made the choice to display an info callout. Otherwise developers won’t know which utility to use
site/content/docs/5.2/utilities/colors.md
.text-body-emphasis
,.text-body-secondary
and.text-body-tertiary
.text-body-emphasis
,.text-body-secondary
and.text-body-tertiary
are compliant with ODS. Otherwise, we need no to display them in the list.text-muted
should have the same color as.text-secondary
. Let's see with other commits after.text-{{name}}-emphasis
elements, displayed an info callout to say that these classes have the same rendering as normal ones.text-muted
: didn’t change anything cause there was no background colors but borders in Boostedsite/content/docs/5.2/utilities/overflow.md
site/content/docs/5.2/utilities/position.md
: integrated the content almost as is, tweaking some tiny things so that it could correspond to our brandsite/content/docs/5.2/utilities/shadows.md
: integrated the new bgs as issite/content/docs/5.2/utilities/sizing.md
.bd-example-flex
site/content/docs/5.2/utilities/spacing.md
.bd-example-cssgrid
site/content/docs/5.2/utilities/text.md
: changed.bg-light
to.bg-body-secondary
site/data/sidebar.yml
site/data/theme-colors.yml
site/layouts/_default/baseof.html
site/layouts/_default/docs.html
: integrated the change of.text-*
utility but didn’t need to changelink-dark
which wasn’t theresite/layouts/partials/docs-navbar.html
{{ if eq. Layout "docs" }}
like to display or not the selector. Same thing for the script insite/layouts/partials/docs-versions.html
+$enable-dark-mode: true
+ npm specific command optionCompletely adapted this new dropdown so that it uses the same structure as our version dropdown (particularly an
<a>
instead of a<button>
)Removed the vertical/horizontal separator
site/layouts/partials/docs-versions.html
: integrated as is: nothing to do here since we use our own Footer componentsite/layouts/partials/footer.html
site/layouts/partials/header.html
:site/layouts/partials/home/masthead.html
.lh-1
is already set with the title in Boostedsite/layouts/partials/icons.html
Introduced the 3 icons from Bootstrap as is but we might need to use Solaris icons in the future when the dark mode will be implemented
: nothing to modify in Boosted cause we use tags within added-in shortcakesite/layouts/shortcodes/added-in.html
: this file is useless since we chose to keep oursite/layouts/shortcodes/callout-deprecated-dark-variants.html
.*-dark
for now for our dark variantssite/layouts/shortcodes/deprecated-in.html
site/layouts/shortcodes/example.html
: just removed the comment in our code because we already removed.bg-light
in the past