Skip to content

Support color-scheme in Servo - #438

Draft
TimvdLippe wants to merge 1 commit into
servo:mainfrom
TimvdLippe:support-color-scheme
Draft

Support color-scheme in Servo#438
TimvdLippe wants to merge 1 commit into
servo:mainfrom
TimvdLippe:support-color-scheme

Conversation

@TimvdLippe

Copy link
Copy Markdown
Contributor

With these changes, both the color-scheme CSS property is supported, as well as the required logic to act upon it.

Supersedes and closes #411

With these changes, both the `color-scheme` CSS property
is supported, as well as the required logic to act upon it.

Co-authored-by: Nico Burns <nico@nicoburns.com>
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
@mrobinson

Copy link
Copy Markdown
Member

Is there a corresponding Servo PR for this change? Typically for Stylo PRs they should have a link to a Servo PR unless it is guaranteed to not change test results / affect the build.

@TimvdLippe

Copy link
Copy Markdown
Contributor Author

It's still a draft as I ran out of time to create the Servo PR. Once I have that finished, I will mark this PR as ready

@TimvdLippe

Copy link
Copy Markdown
Contributor Author

Stylo is currently being upgraded in servo/servo#46987 which means that test results will be muddied by the upgrade. Will wait on that

Comment thread style/device/servo.rs
Comment on lines +343 to +356
// If both are supported, then use the prefered color scheme to determine
// whether the user wants dark mode
if supports_dark_mode && supports_light_mode {
return prefers_color_scheme == PrefersColorScheme::Dark;
}

// If only one is chosen, then it should only support dark mode if it was dark
// mode enabled.
if supports_dark_mode || supports_light_mode {
return supports_dark_mode;
}

// Neither was chosen. In that case, fallback to the preferred color scheme
return prefers_color_scheme == PrefersColorScheme::Dark;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this would be simpler as

Suggested change
// If both are supported, then use the prefered color scheme to determine
// whether the user wants dark mode
if supports_dark_mode && supports_light_mode {
return prefers_color_scheme == PrefersColorScheme::Dark;
}
// If only one is chosen, then it should only support dark mode if it was dark
// mode enabled.
if supports_dark_mode || supports_light_mode {
return supports_dark_mode;
}
// Neither was chosen. In that case, fallback to the preferred color scheme
return prefers_color_scheme == PrefersColorScheme::Dark;
// If only one is supported, then use dark mode if it was the supported one.
if supports_dark_mode != supports_light_mode {
return supports_dark_mode;
}
// If either both or none are supported, then use the preferred color scheme
// to determine whether the user wants dark mode.
return prefers_color_scheme == PrefersColorScheme::Dark;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants