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

Move theme picker button to the right and display it on all pages #94607

Closed

Conversation

GuillaumeGomez
Copy link
Member

Fixes #93216.
Kind of linked to #94048.

So more explanations about this: we received a lot of complains about the removal of the theme picker button. The idea behind this change was to reduce the visual distraction. We have a more global plan about settings to have them displayed like the "firefox pocket". To do so, the first step is #93097 (it's blocked on #90630 for the time being). Then once it's fully switched to JS, we can easily change how it's displayed.

So for the moment and as suggested, I move the theme picker button with the other ones one the right. It now looks like this:

Screenshot from 2022-03-04 14-59-27
Screenshot from 2022-03-04 14-59-24

You can test it online here.

r? @jsha

@GuillaumeGomez GuillaumeGomez 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 Mar 4, 2022
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez,@Folyd,@jsha

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2022
@GuillaumeGomez GuillaumeGomez changed the title Theme picker display Move theme picker button to the right and display it on all pages Mar 4, 2022
@steffahn
Copy link
Member

steffahn commented Mar 4, 2022

You can test it online here.

Does not appear to display the theme picker in mobile. Is this intentional? I would prefer having it in mobile, too.

@GuillaumeGomez
Copy link
Member Author

It's intentional. I'll make it appear on mobile as well and we can talk about it with @jsha then.

@GuillaumeGomez
Copy link
Member Author

I made the mobile change in a separate commit and updated the online docs. It looks like this:

Screenshot from 2022-03-04 15-35-03

@Urgau
Copy link
Member

Urgau commented Mar 9, 2022

Should this maybe be consider for back-porting ?

@GuillaumeGomez
Copy link
Member Author

We already need to discuss it (the mobile part too).

@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 11, 2022
@bors
Copy link
Contributor

bors commented Apr 30, 2022

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

@notriddle
Copy link
Contributor

#96958 makes this PR obsolete, doesn't it?

@notriddle notriddle closed this May 25, 2022
@GuillaumeGomez
Copy link
Member Author

Absolutely! Thanks for closing it.

@GuillaumeGomez GuillaumeGomez deleted the theme-picker-display branch May 25, 2022 09:03
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-review Status: Awaiting review from the assignee but also interested parties. 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.

Please bring back the paintbrush icon, or an equivalent!
8 participants