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

rustdoc: fix colors for top buttons and search box #93540

Closed
wants to merge 3 commits into from

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Feb 1, 2022

Previously these had a lighter border than the search box.

Before:

After:

Also, remove background-color on .in-band

This was overwriting the style for :target on impl headings. Instead of showing the whole summary of a :target'ed heading as yellow (on light theme), it would show the summary as white.

Before:

image

After:

image

Demo: https://rustdoc.crud.net/jsha/ayu-border-color/std/string/trait.ToString.html

@jsha jsha added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: rustdoc UI (generated HTML) labels Feb 1, 2022
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez,@Folyd

@rust-highfive
Copy link
Collaborator

r? @CraftSpider

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 1, 2022
@jsha
Copy link
Contributor Author

jsha commented Feb 1, 2022

r? @GuillaumeGomez

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member

Could you add a GUI test to check that the search input has the same border color as the other buttons please?

@jsha jsha changed the title rustdoc: fix border-color for top buttons rustdoc: fix colors for top buttons and search box Feb 10, 2022
@jsha
Copy link
Contributor Author

jsha commented Feb 10, 2022

Updated to add the test. Also made the change a little bigger: The icon contents now have the same color as their border, on all themes. And the background of the search box in dark theme is now the background of the page overall, which matches the design of ayu and light. This avoids having a big white box that stands out from the rest of the page.

I also adjusted the crate-search dropdown slightly to match. It has the same background now, and gained a slight border to distinguish it.

@GuillaumeGomez
Copy link
Member

There some rules missing in the themes apparently said the CI.

@GuillaumeGomez
Copy link
Member

The new border color makes elements difficult to notice (it looks fine - for me - on the ayu theme):

Screenshot from 2022-02-10 11-20-53
Screenshot from 2022-02-10 11-20-47

Also, I don't think you added a GUI test when an impl ID is in the URLto check its background color?

Make sure the top buttons and search have the same border color and the
same color for the icon inside.
This was overwriting the style for :target on impl headings. Instead of
showing the whole summary as yellow (on light theme), it would show part
of the summary as white and part as yellow.
@jsha
Copy link
Contributor Author

jsha commented Feb 12, 2022

On light theme, the border color is the same as it's been. The only change there is that I had dimmed the button contents (? and ⚙️) to match the border color. I pushed a new revision with the button contents restored to higher contrast. What do you think?

image

Uploading image.png…

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   Compiling rustdoc-themes v0.1.0 (/checkout/src/tools/rustdoc-themes)
    Finished release [optimized] target(s) in 0.46s
rustdoc: [check-theme] Starting tests! (Ignoring all other arguments)
 - Checking "/checkout/src/librustdoc/html/static/css/themes/ayu.css"... FAILED
  Missing "#theme-picker:hover,#theme-picker:focus,#settings-menu:hover,#settings-menu:focus,#help-button:hover,#help-button:focus" rule
  Missing "#theme-picker,#settings-menu,#help-button,#crate-search,.search-input" rule
 - Checking "/checkout/src/librustdoc/html/static/css/themes/dark.css"... FAILED
  Missing "#theme-picker:hover,#theme-picker:focus,#settings-menu:hover,#settings-menu:focus,#help-button:hover,#help-button:focus" rule

@GuillaumeGomez
Copy link
Member

Still find it hard to see but better indeed.

@bors
Copy link
Contributor

bors commented Feb 18, 2022

☔ The latest upstream changes (presumably #94095) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2022
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 24, 2022
@JohnCSimon
Copy link
Member

ping from triage:
@jsha
returning to you to address build failures + conflicts

@Dylan-DPC Dylan-DPC added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 22, 2022
@Dylan-DPC
Copy link
Member

Closing this as inactive

@Dylan-DPC Dylan-DPC closed this Sep 23, 2022
@Dylan-DPC

This comment was marked as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: rustdoc UI (generated HTML) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants