-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Custom scrollbars for Windows based on win 8 #6305
Conversation
I'd suggest this content for .platform-win {
::-webkit-scrollbar {
width: 15px;
height: 15px;
background-color: rgb(240, 240, 240);
}
::-webkit-scrollbar-thumb {
box-shadow: none;
border: 1px solid rgb(240, 240, 240);
background-color: rgb(206, 206, 206);
}
:hover::-webkit-scrollbar-thumb,
:focus::-webkit-scrollbar-thumb {
background-color: rgb(166, 166, 166);
}
:active::-webkit-scrollbar-thumb {
background-color: rgb(96, 96, 96);
}
::-webkit-scrollbar-track:vertical {
margin: 0 0 15px 0;
min-height: 20px;
}
::-webkit-scrollbar-track:horizontal {
margin: 0 15px 0 0;
min-width: 20px;
}
::-webkit-scrollbar-corner {
background-color: transparent;
}
} It adds a hover/active effect, just like in Win8. There is a little border as well to differ the scrollbar from the right sidebar. And the width is set to 15px, just as it is in Win8. |
Woops, I totally forgot about that other PR, but it hasn't been updated in a long time, and this one fixes most of the issues the other one had. So I guess that is ok. Thanks for the suggestions, but I don't think that we need to replicate the scrollbars, making some based on that win8 style seems good enough to me. The hover state looks good, I didn't notices it before, but I don't really like the extra border or that the bars are so wide. Maybe @larz0 could give a final say on this? |
Look at this Win8 screenshot where you can see the original Win8 scrollbars. They have a little border (or margin, whatever) so you can differ the scrollbar-thumb from the right/bottom sidebar. EDIT: .platform-win {
::-webkit-scrollbar {
width: 12px;
height: 12px;
background-color: rgb(240, 240, 240);
}
::-webkit-scrollbar-thumb {
box-shadow: none;
background-color: rgb(206, 206, 206);
}
::-webkit-scrollbar-thumb:vertical {
border-right: 1px solid rgb(240, 240, 240);
}
::-webkit-scrollbar-thumb:horizontal {
border-bottom: 1px solid rgb(240, 240, 240);
}
:hover::-webkit-scrollbar-thumb,
:focus::-webkit-scrollbar-thumb {
background-color: rgb(166, 166, 166);
}
:active::-webkit-scrollbar-thumb {
background-color: rgb(96, 96, 96);
}
::-webkit-scrollbar-track:vertical {
margin: 1px 0 12px 0;
min-height: 20px;
}
::-webkit-scrollbar-track:horizontal {
margin: 0 12px 0 0;
min-width: 20px;
}
::-webkit-scrollbar-corner {
background-color: transparent;
}
} |
I know how the win8 scrollbars look... I got a Windows 8 machine. I am still not convinced about the border... the rest looks good, although you need to move the hover, focus and active after the pseudoclass so that it acts when you hover over the scrollbar and not when you hover the content. |
@TomMalbran ahhh you figured it out! Looks good! Review complete. |
@@ -39,6 +40,7 @@ | |||
} | |||
|
|||
::-webkit-scrollbar-thumb { | |||
background-color: transparent; |
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.
Does this break anything on other platforms? It seems like this means the thumb will be invisible on Linux and on Macs where the mouse is plugged in, since background-color is only overridden in the .platform-win
rules below.
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.
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 plugged in a mouse on my 10.8.5 laptop, and the thumb appears fine.
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.
That line is only applied on windows. So it shouldn't affect any other platform.
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.
Actually, we are using box-shadow to add the background color of the scrollbar. This is done as a "hack" to make it possible to have a margin around the scrollbar. Since I used background colors for the Windows scrollbars I had to overwrite it here with transparent so that that background isn't shown on this bars. But thinking it better, maybe I could just use box-shadow on win too.
Done with code review. However, on Windows only, I noticed that the scrollbar thumb does not highlight when you hover the mouse cursor over or drag the thumb. Makes it difficult to see on Windows. Submitted issue #6352. |
@bchintx See #6305 (comment) for a Win8-like hover effect. |
@SAplayer Yup -- that CSS seems to implement the highlighting as I'd expect. |
Yes. I was waiting for a full review before adding the hover and active effects. @LarZ is the width ok? Do we need the border? |
But @bchintx, on Linux we have no hover effect, too (and we never had). |
@TomMalbran So far your change looks fine. IMHO, if you can go ahead and add the highlighted effects, it'll be ready to merge. BTW, @larz0 already posted a comment above that he approved of the changes, but I'll let him respond to your most recent question re: the width and border. @SAplayer I haven't checked Linux, but Mac and Win both currently have hover effects. (On Mac, I think you need a plugged-in mouse, as @peterflynn already noted above.) Without the highlighting, the scrollbar thumb is very faint, so the extra contrast is very helpful. |
@bchintx Done. Added the hover and active effects. I switched to box-shadow so I didn't had to change to overwrite the backgrounds in the quiet scrollbars for all the effects. Should the colors be moved to variables? Linux doesn't have a hover effect. You can check it in the code, but can be added in the same way as I just added it for windows. |
@TomMalbran Thanks for the hover effects. Yes, actually, they probably should be in variables. How about in 'brackets_theme_default.less'? @peterflynn Good point. I've confirmed that the latest changes address my concerns about the highlighting. |
@bchintx @TomMalbran checked it out just then and it's good. |
@bchintx Added the variables. I tried to improve the corner when we have both scrollbars, since I didn't liked having a margin when there was just 1 scrollbar. The solution might look ok, but isn't perfect, but the problem has to first be fixed in CodeMirror before we completely fix it here. Since we use custom scrollbars CodeMirror doesn't know the size of it, so it extends the scrollbars to the bottom/right, instead of before the corner. I think that a good solution would be to have a class added somewhere to tell us when there are both scrollbars, so we can use it to add the margin in css when required. |
I found an old user story related to this: https://trello.com/c/jhCb7dvV/909-windows-scroll-bars. I think we can move it to the 'Community Done' list once this PR lands! :-) |
One other thought: given the UI's sensitivity around scrolling performance, should we do some testing to verify that the effects like box-shadow, etc. don't impact scrolling speed? (We'd probably have to use the high-speed camera for this though). |
@TomMalbran Thanks for adding the variables. The change looks great. @peterflynn I've played around with the scrolling performance both with and without this change, and I don't see any noticeable difference in responsiveness or speed as I scroll using the thumb. Was this an issue in the past for a particular system or is there something specific we can do to validate your concern? |
Thanks. I still don't like that I can't make it stop at the corner when there are both vertical and horizontal scrollbars, but that is more of a problem with CodeMirror. Box shadows can give bad scroll performance, but I think the main issue is when it is applied to elements and is semi-transparent. Since here is just applied to the scrollbar and opaque, it shouldn't give much problems. I can easily switch this one to background colors, but we can't easily do the same with the linux custom scrollbars since those have a margin around them. |
@TomMalbran based on @peterflynn's concerns, yes, can you please go ahead and switch to background colors on Windows? Since there's not an easy alternative on Linux, just leave those as is. |
After further discussion, since there doesn't seem to be a noticeable performance impact, we're ok with merging this now. Thanks for the work, @TomMalbran! |
Custom scrollbars for Windows based on win 8
This PR adds custom scrollbars to windows based on the windows 8 scrollbars. It also fixes the gutter filler for Linux.