From b23f579c9dfb4774b093aa61c8167d2f760051b8 Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 25 Oct 2023 12:42:33 +0100 Subject: [PATCH] Use theme preference from IDE when embedded instead of preference (#6581) * Use theme preference from IDE when embedded instead of preference Currently the theme always comes from the preference and ignores what the IDE passes. Since there's no way to un-set the preference (or change it in the UI when embedded) this may prevent it matching the IDE. This change always uses the IDE preference when embedded. It requires a change in VS Code to pass embed=true when embedding the sidebar since this was missing originally. --- packages/devtools_app/lib/src/app.dart | 17 +++++++++++------ .../release_notes/NEXT_RELEASE_NOTES.md | 5 +++++ .../lib/src/ui/theme/_ide_theme_web.dart | 1 + .../lib/src/ui/theme/ide_theme.dart | 2 ++ .../lib/src/ui/theme/theme.dart | 6 ++++-- 5 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/devtools_app/lib/src/app.dart b/packages/devtools_app/lib/src/app.dart index 2905854fa27..e52b52e4635 100644 --- a/packages/devtools_app/lib/src/app.dart +++ b/packages/devtools_app/lib/src/app.dart @@ -111,10 +111,15 @@ class DevToolsAppState extends State with AutoDisposeMixin { (e) => DevToolsScreen(ExtensionScreen(e)).screen, ); - // TODO(dantup): This does not take IDE preference into account, so results - // in Dark mode embedded sidebar in VS Code. - bool get isDarkThemeEnabled => _isDarkThemeEnabled; - bool _isDarkThemeEnabled = true; + bool get isDarkThemeEnabled { + // We use user preference when not embedded. When embedded, we always use + // the IDE one (since the user can't access the preference, and the + // preference may have been set in an external window and differ from the + // IDE theme). + return ideTheme.embed ? ideTheme.isDarkMode : _isDarkThemeEnabledPreference; + } + + bool _isDarkThemeEnabledPreference = true; bool get denseModeEnabled => _denseModeEnabled; bool _denseModeEnabled = false; @@ -161,10 +166,10 @@ class DevToolsAppState extends State with AutoDisposeMixin { }, ); - _isDarkThemeEnabled = preferences.darkModeTheme.value; + _isDarkThemeEnabledPreference = preferences.darkModeTheme.value; addAutoDisposeListener(preferences.darkModeTheme, () { setState(() { - _isDarkThemeEnabled = preferences.darkModeTheme.value; + _isDarkThemeEnabledPreference = preferences.darkModeTheme.value; }); }); diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 035821c8411..354a626761c 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -59,6 +59,11 @@ TODO: Remove this section if there are not any general updates. TODO: Remove this section if there are not any general updates. +## VS Code Sidebar updates + +* When using VS Code with a light theme, the embedded sidebar provided by DevTools will now also show in the light +theme [#6581](https://github.com/flutter/devtools/pull/6581) + ## Full commit history To find a complete list of changes in this release, check out the diff --git a/packages/devtools_app_shared/lib/src/ui/theme/_ide_theme_web.dart b/packages/devtools_app_shared/lib/src/ui/theme/_ide_theme_web.dart index b2362c403d6..3e143d6e93e 100644 --- a/packages/devtools_app_shared/lib/src/ui/theme/_ide_theme_web.dart +++ b/packages/devtools_app_shared/lib/src/ui/theme/_ide_theme_web.dart @@ -25,6 +25,7 @@ IdeTheme getIdeTheme() { fontSize: _tryParseDouble(queryParams['fontSize']) ?? unscaledDefaultFontSize, embed: queryParams['embed'] == 'true', + isDarkMode: queryParams['theme'] != 'light', ); // If the environment has provided a background color, set it immediately diff --git a/packages/devtools_app_shared/lib/src/ui/theme/ide_theme.dart b/packages/devtools_app_shared/lib/src/ui/theme/ide_theme.dart index 04e9b372110..6f4189eafaf 100644 --- a/packages/devtools_app_shared/lib/src/ui/theme/ide_theme.dart +++ b/packages/devtools_app_shared/lib/src/ui/theme/ide_theme.dart @@ -15,12 +15,14 @@ final class IdeTheme { this.foregroundColor, this.fontSize = unscaledDefaultFontSize, this.embed = false, + this.isDarkMode = true, }); final Color? backgroundColor; final Color? foregroundColor; final double fontSize; final bool embed; + final bool isDarkMode; double get fontSizeFactor => fontSize / unscaledDefaultFontSize; } diff --git a/packages/devtools_app_shared/lib/src/ui/theme/theme.dart b/packages/devtools_app_shared/lib/src/ui/theme/theme.dart index 8c9caf4f710..62fe725f3c9 100644 --- a/packages/devtools_app_shared/lib/src/ui/theme/theme.dart +++ b/packages/devtools_app_shared/lib/src/ui/theme/theme.dart @@ -203,9 +203,11 @@ Color alternatingColorForIndex(int index, ColorScheme colorScheme) { /// override the default DevTools themes with. /// /// A value of 0.5 would result in all colours being considered light/dark, and -/// a value of 0.1 allowing around only the 10% darkest/lightest colours by +/// a value of 0.12 allowing around only the 12% darkest/lightest colours by /// Flutter's luminance calculation. -const _lightDarkLuminanceThreshold = 0.1; +/// 12% was chosen becaues VS Code's default light background color is #f3f3f3 +/// which is a little under 11%. +const _lightDarkLuminanceThreshold = 0.12; bool isValidDarkColor(Color? color) { if (color == null) {