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

feat: prefers-color-scheme: dark feature implement. Issue #645 #934

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Baleksas
Copy link
Collaborator

@Baleksas Baleksas commented Jul 13, 2022

#645

If media property matches of prefers-color-scheme: dark is true and CSS variables are set to defaults, dark background colour and white text colour will be applied. If the CSS variables are not default (customized by user), dark background or/and white text will not be applied (relatively to what was customized).

@christianp
Copy link
Member

super, I'll look at this after lunch

@christianp
Copy link
Member

It looks like this sets some default dark colours for the background and text inside the observable that watches the display options and sets the corresponding CSS properties.

This means that the display options modal can look misleading: with dark mode on, if you select a white background you instead get a dark one. If the student was looking to override their OS's preference, that would be impossible.

It also doesn't deal with prefers-color-scheme changing, which some devices do based on the time of day.

I think that when the display options modal is shown, the defaults should be set based on the current value of prefers-color-scheme. If the user selects the default values, then we are in a "default behaviour" mode and the colours should be handled entirely by CSS. The JS code should only set the CSS properties if the user selected something other than the default.

The default values can be given in CSS by something like this:

:root {
    --background-colour: #ffffff;
    --text-colour: #000000;
}
@media (prefers-color-scheme: dark) {
    --background-color: #000000;
    --text-colour: #ffffff;
}

When the JS code sets properties on the body element, they will take precedence over these.

@Baleksas
Copy link
Collaborator Author

Right, that's what I tried at first. I just tied choosing specific elements and setting their colours manually. I forgot that media queries can overwrite global variables. I overthinked this issue perhaps.

@Baleksas
Copy link
Collaborator Author

It now should change default values which can be seen in a modal and only uses default if default is chosen accordingly.

this also sets --main-colour and --main-darker to significantly darker shades of the same hue as the light scheme.

There are lots of other bits that need to change in dark scheme,
including but not limited to:

* links (and `btn-link` buttons)
* buttons
* input boxes with feedback styling
@christianp
Copy link
Member

Thanks, I've tried it and it now seems to work how I intended.

It turns out the defaults are set in bootstrap.css, so I've moved the dark-mode defaults there too. I changed the default dark-mode background to pure black: #515151 is far too light, and will make ensuring AAA constrast difficult.

There are lots of other colours that need to change in dark mode, including but not limited to:

  • links
  • buttons
  • input boxes with feedback styling (e.g. unsubmitted, incorrect, correct)

If you have any more time, please try to fix some of these. Use the accessibility inspector to aim for AAA contrast ratios, and ensure it's at least AA.

@Baleksas
Copy link
Collaborator Author

I am trying to reinstall the editor, it's still doesn't work but I will try as soon as I do

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.

2 participants