Skip to content

Conversation

GuillaumeGomez
Copy link
Member

To reproduce the bug currently: click on the theme picker button twice (to show it then hide it). Then click anywhere else: the dropdown menu appears again.

The problem was coming from a confusion of what the hideThemeButtonState and showThemeButtonState were supposed to do. I switched their codes and updated the switchThemeButtonState function. It now works as expected.

r? @kinnison

@kinnison
Copy link
Contributor

I can confirm the bug as described, and the change looks okay, but I would like to see a rendered example to confirm for myself that the issue is resolved before I approve. Can you publish a trivial crate's docs somewhere with this fix applied so I can confirm?

Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+

Confirmed good.

@GuillaumeGomez
Copy link
Member Author

@bors: r=kinnison rollup

@bors
Copy link
Collaborator

bors commented Aug 22, 2019

📌 Commit 3375b05 has been approved by kinnison

@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 Aug 22, 2019
Centril added a commit to Centril/rust that referenced this pull request Aug 22, 2019
…kinnison

Fix confusion in theme picker functions

To reproduce the bug currently: click on the theme picker button twice (to show it then hide it). Then click anywhere else: the dropdown menu appears again.

The problem was coming from a confusion of what the `hideThemeButtonState` and `showThemeButtonState` were supposed to do. I switched their codes and updated the `switchThemeButtonState` function. It now works as expected.

r? @kinnison
bors added a commit that referenced this pull request Aug 22, 2019
Rollup of 7 pull requests

Successful merges:

 - #63624 (When declaring a declarative macro in an item it's only accessible inside it)
 - #63737 (Fix naming misspelling)
 - #63767 (Use more optimal Ord implementation for integers)
 - #63782 (Fix confusion in theme picker functions)
 - #63788 (Add amanjeev to rustc-guide toolstate)
 - #63796 (Tweak E0308 on opaque types)
 - #63805 (Apply few Clippy suggestions)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Aug 22, 2019
Rollup of 7 pull requests

Successful merges:

 - #63624 (When declaring a declarative macro in an item it's only accessible inside it)
 - #63737 (Fix naming misspelling)
 - #63767 (Use more optimal Ord implementation for integers)
 - #63782 (Fix confusion in theme picker functions)
 - #63788 (Add amanjeev to rustc-guide toolstate)
 - #63796 (Tweak E0308 on opaque types)
 - #63805 (Apply few Clippy suggestions)

Failed merges:

r? @ghost
@bors bors merged commit 3375b05 into rust-lang:master Aug 22, 2019
@GuillaumeGomez GuillaumeGomez deleted the theme-switch-fix branch August 23, 2019 09:30
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.

3 participants