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.

Sign up for GitHub

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

Jump to bottom

Add right border bar to Dark and Light theme #74504

Merged
merged 2 commits into from
Jul 24, 2020
Merged

Add right border bar to Dark and Light theme #74504

merged 2 commits into from
Jul 24, 2020

Conversation

tesuji
Copy link
Contributor

@tesuji tesuji commented Jul 19, 2020 •
edited
Loading

Demo:
Light theme: #74504 (comment)
Dark theme: #74504 (comment)
Ayu theme: #74504 (comment)

@tesuji
Copy link
Contributor Author

tesuji commented Jul 19, 2020

r? @jyn514

@jyn514
Copy link
Member

jyn514 commented Jul 19, 2020

@bors delegate=@Cldfire

@bors
Copy link
Contributor

bors commented Jul 19, 2020

✌️ @@Cldfire can now approve this pull request

@Cldfire
Copy link
Contributor

Cldfire commented Jul 19, 2020 •
edited
Loading

So, I intentionally added this as a feature. The highlight color I used is very subtle (to keep contrast high on the text and make it easy to read), and I added the highlight bar to provide an "anchor" of sorts that you can quickly scan for with your eyes when scrolling through the page.

Normally when the docs window is larger the highlight bar is much less tall which makes it less distracting:

image

I've found it very handy in my own usage and I would be sad to see it go. I'm curious if other people agree that it is distracting, though 🙂.

@tesuji
Copy link
Contributor Author

tesuji commented Jul 19, 2020 •
edited
Loading

Personally, I don't get used to this highlight color bar yet. The first time I saw this bar, I wonder why is it highlighted?
Is something special about this function? I keep asking myself about it. Another potential reason is that I don't find
the similar feature in other document sites like MSDN, MDN or Java Oracle, ...

That's just my reason. If people like the highlighted bar, let's keep it.

@Cldfire
Copy link
Contributor

Cldfire commented Jul 19, 2020 •
edited
Loading

The first time I saw this bar, I wonder why is it highlighted? Is something special about this function? I keep asking myself about it.

Hmm, interesting. My thinking here is that the bar appears alongside the highlight color which makes it clear that it's part of the highlighted function style.

I can see this being the case though. Let's see what other people think!

@jyn514
Copy link
Member

jyn514 commented Jul 19, 2020

I like the highlight because it makes the function stand out a little more. Without the bar the yellow on black is a little hard to see.

@tesuji
Copy link
Contributor Author

tesuji commented Jul 19, 2020 •
edited
Loading

If people like it, I would expect we make the highlighting bar on other themes too for consistences.

@tesuji
Copy link
Contributor Author

tesuji commented Jul 22, 2020

How do we continue the discussion? Maybe I ping other rustdoc devs like @GuillaumeGomez , @ollie27 .

@GuillaumeGomez
Copy link
Member

I have to admit that I like the right border highlighted too. So I'm curious what it would look like in the other themes as well.

@tesuji
Copy link
Contributor Author

tesuji commented Jul 22, 2020

With the same CSS rule:
On Dark theme:
image
On Light theme:
image

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jul 22, 2020 •
edited
Loading

Looks good to me. :)

Someone from @rust-lang/rustdoc wants to comment here maybe?

@jyn514
Copy link
Member

jyn514 commented Jul 22, 2020

I think adding the highlight bar to the light theme so it's consistent is a great idea :)

@tesuji tesuji changed the title Remove distracting right-border on selected functions Add right border bar to Dark and Light theme Jul 22, 2020
@tesuji
Copy link
Contributor Author

tesuji commented Jul 22, 2020

Ok, I changed the patch to include CSS rule for Dark and Light theme.

@GuillaumeGomez
Copy link
Member

On second thought: can you make the border color a bit less luminous in the dark theme please?

@tesuji
Copy link
Contributor Author

tesuji commented Jul 22, 2020 •
edited
Loading

My color picking is nightmare: Is #bb7410 OK ?
Old:
image
New:
image

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jul 22, 2020 •
edited
Loading

Oh nice! Just need to have @Cldfire ok with this since it's the ayu theme. Can you do something equivalent with the dark theme too please? 😛

@Manishearth
Copy link
Member

I have no opinion on the color choice, it seems fine.

@tesuji
Copy link
Contributor Author

tesuji commented Jul 22, 2020 •
edited
Loading

Dark theme:
#ffb44c :
image

#bb7410: image

@GuillaumeGomez
Copy link
Member

Let's go for #bb7410 then! :)

@Cldfire
Copy link
Contributor

Cldfire commented Jul 22, 2020

Looks great!

As far as my thoughts on the colors, I would say we should do the following:

  1. #ffb44c for the light theme (because the theme is very bright and the border color needs to be bright to fit in there)
  2. #bb7410 for the dark theme (or whatever @GuillaumeGomez thinks is best)
  3. rgba(255, 180, 76, 0.85) for the ayu theme:

Before:

Screen Shot 2020-07-22 at 2 48 15 PM

After:

Screen Shot 2020-07-22 at 2 47 54 PM

I would like to avoid using #bb7410 for the ayu theme because that color has a different tone to it that doesn't mesh with the rest of the color set.

@lzutao can you try those three colors out please? 😄

@tesuji
Copy link
Contributor Author

tesuji commented Jul 23, 2020 •
edited
Loading

Updated the PR.

@tesuji
Copy link
Contributor Author

tesuji commented Jul 23, 2020 •
edited
Loading

Just a note: @Cldfire You have bors right in this PR, you could comment @bors r+ to approve this PR.

@GuillaumeGomez
Copy link
Member

Looks good to me, thanks a lot!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jul 23, 2020

📌 Commit da9c77e62fd08cf715929e713d7ac87866206fa9 has been approved by GuillaumeGomez

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 23, 2020
tesuji and others added 2 commits July 24, 2020 02:09
@tesuji @Cldfire
ayu: Change to less luminous color
ccbb024 
Co-authored-by: Cldfire <cldfire@3grid.net>
@tesuji
Add right border bar to Dark and Light theme
7005ddb 
Ayu has it. Adding similar rule to other themes makes users less
surprised and makes GUI more consistent.
@tesuji
Copy link
Contributor Author

tesuji commented Jul 24, 2020

Rebased and added co-author commit.

@GuillaumeGomez
Copy link
Member

Let's go again!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jul 24, 2020

📌 Commit 7005ddb has been approved by GuillaumeGomez

@JohnTitor JohnTitor mentioned this pull request Jul 24, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 24, 2020
@bors
Auto merge of rust-lang#74710 - JohnTitor:rollup-bdz4oee, r=JohnTitor
9008693 
Rollup of 12 pull requests

Successful merges:

 - rust-lang#74361 (Improve doc theme logo display)
 - rust-lang#74504 (Add right border bar to Dark and Light theme)
 - rust-lang#74572 (Internally unify rustc_deprecated and deprecated)
 - rust-lang#74601 (Clean up E0724 explanation)
 - rust-lang#74623 (polymorphize GlobalAlloc::Function)
 - rust-lang#74665 (Don't ICE on unconstrained anonymous lifetimes inside associated types.)
 - rust-lang#74666 (More BTreeMap test cases, some exposing undefined behaviour)
 - rust-lang#74669 (Fix typo)
 - rust-lang#74677 (Remove needless unsafety from BTreeMap::drain_filter)
 - rust-lang#74680 (Add missing backticks in diagnostics note)
 - rust-lang#74694 (Clean up E0727 explanation)
 - rust-lang#74703 (Fix ICE while building MIR with type errors)

Failed merges:

r? @ghost
@bors bors merged commit 52476f5 into rust-lang:master Jul 24, 2020
@tesuji tesuji deleted the ayu-border-selected-fn branch July 24, 2020 14:00
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jyn514 jyn514

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.47.0
Development

Successfully merging this pull request may close these issues.
7 participants
@tesuji @jyn514 @bors @Cldfire @GuillaumeGomez @Manishearth @cuviper