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

Improve scrollbar display in rustdoc #70656

Merged
merged 2 commits into from
Apr 13, 2020

Conversation

GuillaumeGomez
Copy link
Member

The scrollbar of the left sidebar in rustdoc looks very bad on firefox (on dark theme). This PR improves it:

With light theme:

old-firefox-light
firefox-light

And on chrome:

chrome-light
chrome-dark

Small extra question: should I extend it to all scrollbars? I think it'd be better but just in case...

r? @kinnison

@kinnison
Copy link
Contributor

kinnison commented Apr 4, 2020

I think this looks reasonable, and it wouldn't hurt to improve the other scrollbars in the pages too.

r=me when you decide either way.

@GuillaumeGomez
Copy link
Member Author

I'll make them a bit larger by default then.

@GuillaumeGomez
Copy link
Member Author

I extended the changes to all other scrollbars (there is actually just another one but just in case...). It looks like this now:

webkit-based:

Screenshot from 2020-04-07 22-06-20
Screenshot from 2020-04-07 22-06-15

firefox:

Screenshot from 2020-04-07 22-06-58
Screenshot from 2020-04-07 22-06-53

If it looks ok to you @kinnison, then please r+. :)

@JohnTitor JohnTitor added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 12, 2020
@kinnison
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 13, 2020

📌 Commit cbee6c5 has been approved by kinnison

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#70656 (Improve scrollbar display in rustdoc)
 - rust-lang#71051 (Suggest .into() over try_into() when it would work)
 - rust-lang#71087 (Remove `FnCtxt::impl_self_ty`)
 - rust-lang#71097 (Pattern docs)
 - rust-lang#71101 (Miri: let machine hook dynamically decide about alignment checks)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Apr 13, 2020

⌛ Testing commit cbee6c5 with merge 8e18e26...

@bors bors merged commit 3f61c73 into rust-lang:master Apr 13, 2020
@GuillaumeGomez GuillaumeGomez deleted the scrollbar-display branch April 16, 2020 14:55
@maxbla
Copy link
Contributor

maxbla commented Jul 8, 2020

I rather dislike this change and I'm considering opening an issue about it. I think the stylistic consistency between scroll bars in Firefox and other (gtk+) apps is important, and unfortunately changing their color completely changes their look. Also I can't find another example of a website with a dark mode that changes my scrollbar like this. For example, Stack Overflow and YouTube don't
StackOverflowDark

@nagisa
Copy link
Member

nagisa commented Feb 6, 2021

This makes the scrollbars look significantly worse and very distracting on my system as well. Here are some screenshots:

native scrollbar look
new scrollbar look

The new scrollbar for the main content is unexpectedly thick. It also stands out more than usual. I think this change should be either backed out or adjusted.

(The reason I only notice this now is because https://phabricator.services.mozilla.com/D78400 finally got to me)

@GuillaumeGomez
Copy link
Member Author

@nagisa: We had issues because of the different handlings of the sidebars depending on the system so I'd prefer improvement over reverting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants