-
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
Fix images on auto in download app example page #2261
Fix images on auto in download app example page #2261
Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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'm fine landing it as a patch for a bug for v5.3.3 which will arrive soon. I think we don't need any defensive CSS on this.
However, maybe we can fix the color-mode javascript on Bootstrap directly because right now we have:
color-mode on Bootstrap |
color-mode on device |
Result in HTML |
---|---|---|
dark |
* |
data-bs-theme="dark" |
light |
* |
data-bs-theme="light" |
auto |
dark |
data-bs-theme="dark" |
auto |
light |
data-bs-theme="auto" |
For instance, I was able to reproduce the behavior you patch here by patching color-mode.js
.
Here is the diff:
const setTheme = theme => {
- if (theme === 'auto' && window.matchMedia('(prefers-color-scheme: dark)').matches) {
- document.documentElement.setAttribute('data-bs-theme', 'dark')
+ if (theme === 'auto') {
+ document.documentElement.setAttribute('data-bs-theme', (window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light'))
} else {
Defensive CSS was removed as requested. A PR has been opened on Bootstrap's side to update The plan here is to keep the CSS fix presented in the current PR to allow the example page to work correctly. Then: |
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.
LGTM, adding something in #2223 to track it
Related issues
Closes #2243
Description
Following the issue explained in #2243:
(1) I made some tests and it appears that the bug described in the issue only happens on a specific occasion: when your computer is set to 'Light mode' and you use the 'Auto color mode' on the page.
(2) If your your computer is set to 'Dark mode' and you use the 'Auto color mode' on the page the it works perfectly fine.
=> I made a fix in CSS for (1)
=> I also made an additional piece of code (see comment) for (2) as defensive CSS.
Motivation & Context
Prevent having both light and dark images in 'Auto color mode'.
Types of change
Live previews
Checklist
Contribution
Accessibility
Design
Development
Documentation
Checklist (for Core Team only)
After the merge