Sitelet https://web.archive.org/web/20210326212605/https://github.com/mui-org/material-ui/issues/25016
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

[theme] Improve darkScrollbar helper #25016

Open
Primajin opened this issue Feb 20, 2021 · 7 comments
Open

[theme] Improve darkScrollbar helper #25016

Primajin opened this issue Feb 20, 2021 · 7 comments

Comments

@Primajin
Copy link
Contributor

@Primajin Primajin commented Feb 20, 2021 •

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

This is just an idea, but I think we should not set the scrollbar colors "arbitrarily" but rather via the theme. That way the colors have a connection to the used theme.

Examples 🌈

I was trying out something like this: https://github.com/Primajin/material-ui/commit/adb389074887567832a46d7f36f613ec61ec6736 this for sure needs more tweaking but it should convey the idea.

We use custom scrollbars with Material UI 4 both for dark and light - so for 5 it would be good if the scrollbars respect the theme.

Motivation 🔦

For example we use a different paper and background color in dark mode than the default theme. The scrollbars should match the paper color (or something from background) so that it fit's with the overall design.

@oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Feb 20, 2021 •

I think that .background.level2, .background.level1 are debt, we should remove them. The documentation using a different theme is a red-flag that the default values are wrong #22112.

On a different note. How about we add a logic to allow computing the other track and active values? It can save time.

diff --git a/packages/material-ui/src/darkScrollbar/index.ts b/packages/material-ui/src/darkScrollbar/index.ts
index af4a3ccc90..72a691c0a1 100644
--- a/packages/material-ui/src/darkScrollbar/index.ts
+++ b/packages/material-ui/src/darkScrollbar/index.ts
@@ -1,11 +1,31 @@
+import { lighten, darken, getLuminance, emphasize } from '@material-ui/core/styles';
+
 // track, thumb and active are derieved from macOS 10.15.7
 const scrollBar = {
-  track: '#2b2b2b',
   thumb: '#6b6b6b',
-  active: '#959595',
 };

-export default function darkScrollbar(options = scrollBar) {
+// Opposite of emphasize
+function diminish(color: string, coefficient = 0.15) {
+  return getLuminance(color) > 0.5 ? lighten(color, coefficient) : darken(color, coefficient);
+}
+
+interface Options {
+  track?: string;
+  thumb: string;
+  active?: string;
+}
+
+export default function darkScrollbar(options: Options = scrollBar) {
+  options = { ...options };
+
+  if (!options.track) {
+    options.track = diminish(options.thumb, 0.6);
+  }
+  if (!options.active) {
+    options.active = emphasize(options.thumb, 0.15);
+  }
+
   return {
     scrollbarColor: `${options.thumb} ${options.track}`,
     '&::-webkit-scrollbar, & *::-webkit-scrollbar': {

I have set the coefficients to reproduce the same outcome as before, using macOS default colors.

@oliviertassinari oliviertassinari changed the title Set scrollbar colors from the theme [theme] Improve darkScrollbar helper Feb 20, 2021
@Primajin
Copy link
Contributor Author

@Primajin Primajin commented Feb 22, 2021

I also wonder if we should force a macOS~sy kind of style to the scrollbars or go the Github way of just changing the colors without the style of the OS.

Github on MacOS

Screenshot 2021-02-22 at 09 30 17Screenshot 2021-02-22 at 09 30 59

Github on Windows

@oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Feb 23, 2021

@Primajin The current output of darkScrollbar is not platform-dependent but it would probably be better. Is there a way to detect if we are on Windows/Linux?

@Primajin
Copy link
Contributor Author

@Primajin Primajin commented Feb 23, 2021

in CSS not but in JS one could sniff the navigator or so. But maybe we can get around it by only applying (background-)colors and let the rest handle the OS. So we might want to remove the border radius and things like that.

I couldn't really figure out yet how Github does it here but I'm investigating.
I will play around how it will look like across platforms if we only tinker with the colors and share screenshots.

@Primajin
Copy link
Contributor Author

@Primajin Primajin commented Feb 23, 2021

This one seems quite interesting approach - but maybe there is a shorter way than styling everything:
https://codepen.io/patrikx3/pen/ZEBQQyV

@Primajin
Copy link
Contributor Author

@Primajin Primajin commented Feb 26, 2021

OK I got something working like this:

scrollbars-windows-mac

It has one small sniffer for adding a border-radius when it's on mac, otherwise the browser decides whether to show arrows or not.

What do you think?

@oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Feb 26, 2021

It looks great 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants