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 shortcut for theme picker menu #78773

Merged
merged 1 commit into from
Nov 12, 2020
Merged

Add shortcut for theme picker menu #78773

merged 1 commit into from
Nov 12, 2020

Conversation

GuillaumeGomez
Copy link
Member

Follow-up of #78584

Just like you can focus the search input by pressing "S", you can now access the theme picker menu by pressing "T" and navigate through the options only using the keyboard.

cc @notriddle

r? @jyn514

@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 5, 2020
@notriddle
Copy link
Contributor

notriddle commented Nov 5, 2020 •
edited
Loading

Is there a reason for not using showThemeButtonState?

Edit: Wait a minute. It’s because you need to put focus within the menu, too, right?

@GuillaumeGomez
Copy link
Member Author

You edited before I could answer! ><

@pickfire
Copy link
Contributor

pickfire commented Nov 5, 2020

But don't they still need to use the tab and shift-tab to change the theme right? Is there some sort of preview theme when they put hover/focus over it?

@GuillaumeGomez
Copy link
Member Author

@pickfire The other PR I linked added the events handling on the theme-picker menu. So you can go through the different themes by only using the keyboard (which is super cool hehe).

@pickfire
Copy link
Contributor

pickfire commented Nov 5, 2020

@pickfire The other PR I linked added the events handling on the theme-picker menu. So you can go through the different themes by only using the keyboard (which is super cool hehe).

But I don't see that the other patch is handling tab, did I miss it? The old one seemed to support it.

Nice, the fallthrough thing above is just my guess (I thought I was wrong), I find it weird since I thought you explicitly do so.

@jyn514 jyn514 added A-rustdoc-themes Area: Themes for HTML pages generated by rustdoc T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 5, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 12, 2020

@bors r+

Sorry for the delay.

@bors
Copy link
Contributor

bors commented Nov 12, 2020

📌 Commit 8b20e9b has been approved by jyn514

@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 Nov 12, 2020
@bors
Copy link
Contributor

bors commented Nov 12, 2020

⌛ Testing commit 8b20e9b with merge a07877138d105fefbd49cc3565b59618c5d9e7de...

@bors
Copy link
Contributor

bors commented Nov 12, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 12, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 12, 2020

Network error (rust-lang/cargo#8517). This is the same error as #78826 (comment) so going to avoid retrying for now.

@kennytm
Copy link
Member

kennytm commented Nov 12, 2020

@bors retry

@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 Nov 12, 2020
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 12, 2020
@GuillaumeGomez
Rollup merge of rust-lang#78773 - GuillaumeGomez:theme-picker-shortcu…
7305584 
…t, r=jyn514

Add shortcut for theme picker menu

Follow-up of rust-lang#78584

Just like you can focus the search input by pressing "S", you can now access the theme picker menu by pressing "T" and navigate through the options only using the keyboard.

cc `@notriddle`

r? `@jyn514`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Nov 12, 2020
Closed
@bors
Copy link
Contributor

bors commented Nov 12, 2020

⌛ Testing commit 8b20e9b with merge 12f0dba...

@bors
Copy link
Contributor

bors commented Nov 12, 2020

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 12f0dba to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 12, 2020
@bors bors merged commit 12f0dba into rust-lang:master Nov 12, 2020
@rustbot rustbot added this to the 1.49.0 milestone Nov 12, 2020
@GuillaumeGomez GuillaumeGomez deleted the theme-picker-shortcut branch November 12, 2020 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pickfire pickfire pickfire left review comments

Assignees

@jyn514 jyn514

Labels
A-rustdoc-themes Area: Themes for HTML pages generated by rustdoc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.49.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@GuillaumeGomez @rust-highfive @notriddle @pickfire @jyn514 @bors @kennytm @rustbot