Support VS System theme #74432
Support VS System theme #74432
Conversation
VSColorTheme_ThemeChanged fires multiple times in a row when the Windows theme is changed. We only need to apply the color scheme colors once in response.
The System theme has a unique ThemeId but reuses the editor colors from the Light and Dark themes. When the System theme is selected we will check the registry to see whether Apps should be using the light theme and then apply the appropriate color scheme.
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
Another way to handle this, which may be cleaner, is to use an AsyncBatchingWorkQueue to queue up the work. That way if there is a flurry of notifications, it will only end up causing one update to happen. |
@CyrusNajmabadi Updated to use a work queue. Thanks for the suggestion. |
| @@ -56,6 +61,11 @@ internal sealed partial class ColorSchemeApplier | |||
| _settings = new ColorSchemeSettings(threadingContext, _serviceProvider, globalOptions); | |||
| _colorSchemes = ColorSchemeSettings.GetColorSchemes(); | |||
| _classificationVerifier = new ClassificationVerifier(threadingContext, fontAndColorStorage, _colorSchemes); | |||
| _workQueue = new( | |||
| TimeSpan.FromMilliseconds(1000), | |||
There was a problem hiding this comment.
💭 It seems like Idle could leave a really strange visual state for 1.5 seconds, where Short would be equally effective but with less transition time.
| @@ -56,6 +61,11 @@ internal sealed partial class ColorSchemeApplier | |||
| _settings = new ColorSchemeSettings(threadingContext, _serviceProvider, globalOptions); | |||
| _colorSchemes = ColorSchemeSettings.GetColorSchemes(); | |||
| _classificationVerifier = new ClassificationVerifier(threadingContext, fontAndColorStorage, _colorSchemes); | |||
| _workQueue = new( | |||
| TimeSpan.FromMilliseconds(1000), | |||
| async (token) => await QueueColorSchemeUpdateAsync().ConfigureAwait(false), | |||
There was a problem hiding this comment.
| async (token) => await QueueColorSchemeUpdateAsync().ConfigureAwait(false), | |
| async _ => await QueueColorSchemeUpdateAsync().ConfigureAwait(false), |
There was a problem hiding this comment.
or, just replace this entirely with QueueColorSchemeUpdateAsync as the entire argument.
| TimeSpan.FromMilliseconds(1000), | ||
| async (token) => await QueueColorSchemeUpdateAsync().ConfigureAwait(false), | ||
| listenerProvider.GetListener(FeatureAttribute.ColorScheme), | ||
| CancellationToken.None); |
There was a problem hiding this comment.
| CancellationToken.None); | |
| threadingContext.DisposalToken); |
| { | ||
| _workQueue.AddWork(); | ||
| return Task.CompletedTask; | ||
| } | ||
|
|
||
| private async Task QueueColorSchemeUpdateAsync() |
There was a problem hiding this comment.
| private async Task QueueColorSchemeUpdateAsync() | |
| private async ValueTask QueueColorSchemeUpdateAsync(CancellationToken cancellationToken) |
There was a problem hiding this comment.
also, use thei cancellationToken in the StartOnIdle call below.
|
LMK when you make the changes, and i'll quickly review. |
|
@CyrusNajmabadi That cleaned up nicely. Thanks! |
Co-authored-by: Cyrus Najmabadi <[email protected]>
The System theme has a unique ThemeId but reuses the editor colors from the Light and Dark themes. When the System theme is selected we will now check the registry to see whether Apps should be using the light theme and then apply the appropriate color scheme.
I also noticed while testing that VSColorTheme_ThemeChanged fires multiple times in a row when the Windows theme is changed. We only need to apply the color scheme colors once in response. I have added a field to track whether we have an update pending.
Resolves #74416