Use disabled icons for CheckBox in DefaultTheme #84946
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Technically, we should get rid of either the automatic modulation or the disabled icons, but this would break compatibility with existing custom themes (especially the latter). I'd steer towards removing the automatic modulation personally. |
|
If we remove modulation, all custom themes will also be changed. For example, EditorTheme:
|
DarkMessiah
force-pushed
the
default-theme-checkbox-disabled-state
branch
from
f6e2b86 to
ff59496
Compare
Calinou
approved these changes
Jan 10, 2024
|
Note: This shouldn't be cherry-picked to |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CheckBox
PopupMenu
P.S. I found that PopupMenu modulates the icon color for the disabled state. Is this necessary when using separate icons for each state?