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

Add high contrast schemes #1110

Merged
merged 5 commits into from
Sep 28, 2023
Merged

Add high contrast schemes #1110

merged 5 commits into from
Sep 28, 2023

Conversation

LukasKalbertodt
Copy link
Member

Draft as there is still more work to be done. @narickmann will probably take it from here.

@narickmann
Copy link
Contributor

narickmann commented Sep 21, 2023

The high contrast mode (hc mode) should now be finished.

Here are some more thoughts I have:

  1. The boxShadow was removed everywhere for hc mode. A value for this was set for darkmode and lightmode (--shadow-color), but not in hc mode and it is actually ignored.
    I still set it to none for hc mode because I'm not sure whether that could otherwise cause problems at some point and in some places the --shadow-color is not used, but a hardcoded value which would be used in hc mode.
    I think we shouldn't use boxShadow in hc mode because it might be confusing and might not have enough contrast. But I couldn't find anything explicit about it in the WCAG.

  2. Let me know what you think about the play-/pause button in the "Review and trim" step when you hover over it.

  3. I'm not sure what we should do with the "video-settings" button. I think we should we leave it as it is? (I didn't changed it). If anyone has other thoughts or suggestions, leave a comment.

  4. If the hc mode comes into Studio like this, we should adjust the hc mode (for some elements) in the editor. The goal with the new design was to make the applications look related/similar (correct me if I'm wrong). At first glance, this would affect the colors of the drop-down menus (in Studio they have a black/white bg when hovering and in the editor the accent color) and some buttons.

And also let me know if I missed something.

@github-actions

This comment was marked as resolved.

@github-actions github-actions bot added the status:conflicts Conflicts with another pull request or issue label Sep 26, 2023
Copy link
Member Author

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

Sorry, I missed the notification for your push+comment!

I like what you've done! I tested it in light hc and dark hc and basically everything looks very high contrast and good (well, for a high contrast mode). The play/pause button you asked about seems fine to me. I only have a few things I noticed while testing:

  • The cut buttons could use an outline on hover? Their hover effect is very subtle. Well ok, there is the tooltip, which isn't very subtle... maybe that's enough. You decide.
  • In the trimming step: the contrast/difference between the timeline that's already played and that is about to play is fairly low. I think it would be possible to make the "about to be played" part a bit brighter (in light hc) and still have enough contrast to the background.
  • The "Discard" button has high contrast, but the red is not really coming through. So these users would miss out on the warning color. Maybe it's worth it making the button background red? Then it would be super heavy of course. Just an idea, no opinion I think 🤷
  • In dark hc, the floating boxes for shortcuts and info have the same background than the main part and no border. Thus its not really clear where the box starts and ends. I would probably just add a bright border? Alternatively the overlay should be white-semi-transparent instead of black-semi-transparent.
  • In dark hc, the links in the "info" box are not colored. But they could easily be colored like in light hc right?

And a few code comments, but these should all be quick to fix, and most of them are the same "kind of comment" just in multiple places.

src/steps/audio-setup/mic-preview.tsx Outdated Show resolved Hide resolved
src/steps/audio-setup/source-select.tsx Outdated Show resolved Hide resolved
src/steps/elements.tsx Outdated Show resolved Hide resolved
src/steps/recording/controls.tsx Outdated Show resolved Hide resolved
src/steps/recording/controls.tsx Outdated Show resolved Hide resolved
src/steps/video-setup/index.tsx Outdated Show resolved Hide resolved
src/steps/video-setup/prefs.tsx Outdated Show resolved Hide resolved
src/steps/video-setup/preview.tsx Outdated Show resolved Hide resolved
src/ui/SourceOptionButton.tsx Outdated Show resolved Hide resolved
@LukasKalbertodt LukasKalbertodt marked this pull request as ready for review September 27, 2023 07:47
@LukasKalbertodt LukasKalbertodt changed the title Add high contrast schemes (WIP) Add high contrast schemes Sep 27, 2023
LukasKalbertodt and others added 5 commits September 28, 2023 16:26
This commit only lays the foundation and makes the header usable. There
is still more work required, checking all of Studio for good UX.
The previous one was a very very bright green tone. I changed it to a
very bright version of our normal accent color. It would be weird to
suddenly have green where otherwise Studio only uses blue. Now I
understand that with green you can get brighter while maintaining
high saturation, better than with blue. But I think this blue is
bright enough. Further, the only reason I noticed this is that I,
as someone with strong Protonopia did not even see that that was green
at all. I assume the accent color was just white. I think by making it
a tiny bit less bright and using blue, the chance of that happening to
other people is a lot less.
@github-actions github-actions bot removed the status:conflicts Conflicts with another pull request or issue label Sep 28, 2023
@LukasKalbertodt
Copy link
Member Author

LukasKalbertodt commented Sep 28, 2023

Since Nadine will be absent for the coming two weeks, I quickly fixed most of my comments (and rebased). Some of the less opinionated comments I didn't change. It's still fine like this.

Since I only added very little extra stuff and already reviewed otherwise, I will merge.

@LukasKalbertodt LukasKalbertodt merged commit 37c04eb into master Sep 28, 2023
5 checks passed
@LukasKalbertodt LukasKalbertodt deleted the high-contrast branch September 28, 2023 16:03
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.

None yet

2 participants