Several Theme Fixes - New Preference Format, Color Tweaks (#10409)

This contains a few theme fixes, including: 1. Changed theme preference storage to be target-based 2. Change "Select Theme" to just "Theme" 3. A Few Color Fixes 4. Adding a neutral base color 5. Specifying setColorThemeById instead of just setColorTheme 6. Making it optional to save the preference when setting the theme 7. Partial changes toward enabling light icons for dark themes
microsoft · Mar 7, 2025 · 43aef17 · 43aef17
1 parent bf2278e
commit 43aef17
Show file tree

Hide file tree

Showing 27 changed files with 167 additions and 91 deletions.
diff --git a/localtypings/pxteditor.d.ts b/localtypings/pxteditor.d.ts
@@ -104,7 +104,7 @@ declare namespace pxt.editor {
         | "serviceworkerregistered"
         | "runeval"
         | "precachetutorial"
-        | "setcolortheme"
+        | "setcolorthemebyid"
 
         // package extension messasges
         | ExtInitializeType
@@ -512,8 +512,9 @@ declare namespace pxt.editor {
     }
 
     export interface EditorMessageSetColorThemeRequest extends EditorMessageRequest {
-        action: "setcolortheme";
+        action: "setcolorthemebyid";
         colorThemeId: string;
+        savePreference?: boolean;
     }
 
     /**
@@ -1100,7 +1101,7 @@ declare namespace pxt.editor {
         hasHeaderBeenPersistentShared(): boolean;
         getSharePreferenceForHeader(): boolean;
         saveSharePreferenceForHeaderAsync(anonymousByDefault: boolean): Promise<void>;
-        setColorTheme(colorThemeId: string): void;
+        setColorThemeById(colorThemeId: string, savePreference: boolean): void;
     }
 
     export interface IHexFileImporter {

diff --git a/pxteditor/editorcontroller.ts b/pxteditor/editorcontroller.ts
@@ -317,9 +317,9 @@ export function bindEditorMessages(getEditorAsync: () => Promise<IProjectView>)
                                         }
                                     });
                             }
-                            case "setcolortheme": {
+                            case "setcolorthemebyid": {
                                 const msg = data as pxt.editor.EditorMessageSetColorThemeRequest;
-                                projectView.setColorTheme(msg.colorThemeId);
+                                projectView.setColorThemeById(msg.colorThemeId, !!msg.savePreference);
                                 return Promise.resolve();
                             }
                         }

diff --git a/pxtlib/auth.ts b/pxtlib/auth.ts
@@ -44,6 +44,13 @@ namespace pxt.auth {
         badges: Badge[];
     }
 
+    /**
+     * Mapping of target id to preferred color theme id.
+     */
+    export type ColorThemeIdsState = {
+        [targetId: string]: string;
+    }
+
     export type SetPrefResult = {
         success: boolean;
         res: UserPreferences;
@@ -55,7 +62,7 @@ namespace pxt.auth {
     export type UserPreferences = {
         language?: string;
         highContrast?: boolean;
-        themeId?: string;
+        colorThemeIds?: ColorThemeIdsState;
         reader?: string;
         skillmap?: UserSkillmapState;
         badges?: UserBadgeState;
@@ -65,7 +72,7 @@ namespace pxt.auth {
     export const DEFAULT_USER_PREFERENCES: () => UserPreferences = () => ({
         language: pxt.appTarget.appTheme.defaultLocale,
         highContrast: false,
-        themeId: pxt.appTarget.appTheme.defaultColorTheme,
+        colorThemeIds: {}, // Will lookup pxt.appTarget.appTheme.defaultColorTheme for active target
         reader: "",
         skillmap: { mapProgress: {}, completedTags: {} },
         email: false

diff --git a/pxtservices/editorDriver.ts b/pxtservices/editorDriver.ts
@@ -456,12 +456,13 @@ export class EditorDriver extends IframeDriver {
         );
     }
 
-    async setColorTheme(colorThemeId: string) {
+    async setColorTheme(colorThemeId: string, savePreference: boolean) {
         await this.sendRequest (
             {
                 type: "pxteditor",
-                action: "setcolortheme",
-                colorThemeId
+                action: "setcolorthemebyid",
+                colorThemeId,
+                savePreference
             } as pxt.editor.EditorMessageSetColorThemeRequest
         );
     }

diff --git a/react-common/components/theming/themeManager.ts b/react-common/components/theming/themeManager.ts
@@ -30,6 +30,10 @@ export class ThemeManager {
         return this.currentTheme;
     }
 
+    public isKnownTheme(themeId: string): boolean {
+        return !!pxt.appTarget?.colorThemeMap?.[themeId];
+    }
+
     public getAllColorThemes(): pxt.ColorThemeInfo[] {
         const allThemes = pxt.appTarget?.colorThemeMap ? Object.values(pxt.appTarget.colorThemeMap) : [];
         return allThemes.sort((a, b) => {

diff --git a/react-common/styles/theming/base-theme.less b/react-common/styles/theming/base-theme.less
@@ -60,6 +60,7 @@
     --pxt-neutral-stencil3: #FFFFFF;
     --pxt-neutral-background3-alpha90: #4E5758E5; /* Needed for monaco toolbox */
 
+    --pxt-neutral-base: rgba(0, 0, 0, 1);
     --pxt-neutral-alpha0: rgba(0, 0, 0, 0);
     --pxt-neutral-alpha10: rgba(0, 0, 0, 0.1);
     --pxt-neutral-alpha20: rgba(0, 0, 0, 0.2);

diff --git a/skillmap/src/App.tsx b/skillmap/src/App.tsx
@@ -358,9 +358,11 @@ class AppImpl extends React.Component<AppProps, AppState> {
     protected async initColorThemeAsync() {
         // Load theme colors
         const prefThemeId = await authClient.getColorThemeIdAsync();
-        let initialTheme = this.props.highContrast ?
-                    pxt.appTarget?.appTheme?.highContrastColorTheme :
-                    prefThemeId ?? pxt.appTarget?.appTheme?.defaultColorTheme;
+        let initialTheme = this.props.highContrast
+            ? pxt.appTarget?.appTheme?.highContrastColorTheme
+            : (prefThemeId && this.themeManager.isKnownTheme(prefThemeId))
+                ? prefThemeId
+                : pxt.appTarget?.appTheme?.defaultColorTheme;
 
         if (initialTheme) {
             if (initialTheme !== this.themeManager.getCurrentColorTheme()?.id) {
@@ -420,7 +422,7 @@ class AppImpl extends React.Component<AppProps, AppState> {
     changeTheme(theme: pxt.ColorThemeInfo) {
         pxt.tickEvent(`skillmap.menu.theme.changetheme`, { theme: theme.id });
         this.themeManager.switchColorTheme(theme.id);
-        this.props.dispatchSetUserPreferences({ themeId: theme.id });
+        authClient.setColorThemeIdAsync(theme.id);
     }
 
     render() {

diff --git a/skillmap/src/components/HeaderBar.tsx b/skillmap/src/components/HeaderBar.tsx
@@ -42,8 +42,8 @@ export class HeaderBarImpl extends React.Component<HeaderBarProps> {
         if (this.props.preferences) {
             items.push({
                 id: "theme",
-                title: lf("Select Theme"),
-                label: lf("Select Theme"),
+                title: lf("Theme"),
+                label: lf("Theme"),
                 onClick: () => {
                     tickEvent("skillmap.theme");
                     this.props.dispatchShowSelectTheme();

diff --git a/skillmap/src/components/makecodeFrame.tsx b/skillmap/src/components/makecodeFrame.tsx
@@ -174,7 +174,7 @@ class MakeCodeFrameImpl extends React.Component<MakeCodeFrameProps, MakeCodeFram
         this.sendMessageAsync (
             {
                 type: "pxteditor",
-                action: "setcolortheme",
+                action: "setcolorthemebyid",
                 colorThemeId
             } as pxt.editor.EditorMessageSetColorThemeRequest
         );

diff --git a/skillmap/src/lib/authClient.ts b/skillmap/src/lib/authClient.ts
@@ -142,9 +142,26 @@ export async function getBadgeStateAsync(): Promise<pxt.auth.UserBadgeState | un
 }
 
 export async function getColorThemeIdAsync(): Promise<string | undefined> {
+    const prefs = await userPreferencesAsync();
+    if (prefs) {
+        return prefs?.colorThemeIds?.[pxt.appTarget.id];
+    }
+}
+
+export async function setColorThemeIdAsync(themeId: string): Promise<void> {
     const cli = await clientAsync();
-    const prefs = await cli?.userPreferencesAsync();
-    return prefs?.themeId;
+    if (cli) {
+        const currentPrefs = await cli.userPreferencesAsync();
+        const newColorThemePref = {
+            ...currentPrefs?.colorThemeIds,
+            [pxt.appTarget.id]: themeId
+        };
+        await cli.patchUserPreferencesAsync({
+            op: 'replace',
+            path: ['colorThemeIds'],
+            value: newColorThemePref
+        });
+    }
 }
 
 export async function userPreferencesAsync(): Promise<pxt.auth.UserPreferences | undefined> {

diff --git a/theme/color-themes/high-contrast.json b/theme/color-themes/high-contrast.json
@@ -68,6 +68,7 @@
         "pxt-neutral-stencil3": "#FFFFFF",
         "pxt-neutral-background3-alpha90": "#000000E5",
 
+        "pxt-neutral-base": "rgba(255, 255, 255, 1)",
         "pxt-neutral-alpha0": "rgba(255, 255, 255, 0)",
         "pxt-neutral-alpha10": "rgba(255, 255, 255, 0.1)",
         "pxt-neutral-alpha20": "rgba(255, 255, 255, 0.2)",

diff --git a/theme/color-themes/overrides/high-contrast-overrides.css b/theme/color-themes/overrides/high-contrast-overrides.css
@@ -9,3 +9,7 @@
     outline: 2px solid var(--pxt-colors-yellow-background) !important;
     z-index: 1;
 }
+
+.barcharticon {
+    filter: invert(1);
+}
diff --git a/theme/common.less b/theme/common.less
@@ -258,6 +258,19 @@ pre {
     width: 100% !important;
     margin: 0;
     margin-top: 1rem;
+
+    background: var(--pxt-primary-background);
+    color: var(--pxt-primary-foreground);
+
+    .item, .link.item {
+        color: var(--pxt-primary-foreground) !important; // override !important in semantic ui
+        border-radius: inherit;
+
+        &:hover, &:focus {
+            background: var(--pxt-primary-background-hover) !important;
+            color: var(--pxt-primary-foreground-hover) !important;
+        }
+    }
 }
 
 .filemenu .nested.item {

diff --git a/theme/highcontrast.less b/theme/highcontrast.less
@@ -125,9 +125,15 @@
         .blocklyTreeSelected span.blocklyTreeLabel {
             color: @HCblocklyTreeLabelSelectedColor;
         }
+        .blocklyTreeRow:not(.blocklyTreeSelected) {
+            background-color: @HCblocklyToolboxColor !important;
+            .blocklyTreeIcon, .blocklyTreeLabel {
+                color: @HCblocklyTreeLabelColor !important;
+            }
+        }
         .blocklyTreeRow:not(.blocklyTreeSelected):hover,
         .blocklyTreeRow:not(.blocklyTreeSelected):focus {
-            background-color: lighten(@HCblocklyToolboxColor, 25.0);
+            background-color: lighten(@HCblocklyToolboxColor, 25.0) !important;
         }
     }
 

diff --git a/theme/home.less b/theme/home.less
@@ -137,7 +137,7 @@
             margin: 0;
             padding-left: @carouselArrowSize;
             font-size: 20px;
-            color: var(--pxt-neutral-foreground1);
+            color: var(--pxt-target-foreground2);
             &.myproject-header {
                 cursor: pointer;
                 outline: none;

diff --git a/theme/semantic-ui-overrides.less b/theme/semantic-ui-overrides.less
@@ -8,25 +8,27 @@ This file overrides the default semantic ui theming with our own theme variables
 
 // Mixin allows easy setting for background, foreground, hover, and inverted colors for a button
 .buttonVariant(@bg; @fg; @hoverBg; @hoverFg; @border) {
-    background: @bg;
-    color: @fg;
-    border: 1px solid @border;
-
-    &.inverted {
-        background-color: @fg;
-        color: @bg;
-    }
-
-    &:hover, &:focus {
-        // Semantic UI darkens background using a filter, which isn't necessary when we have a hover color.
-        filter: none;
-
-        background-color: @hoverBg;
-        color: @hoverFg;
-
+    &:not(.disabled) {
+        background-color: @bg;
+        color: @fg;
+        border: 1px solid @border;
+
         &.inverted {
-            background-color: @hoverFg;
-            color: @hoverBg;
+            background-color: @fg;
+            color: @bg;
+        }
+
+        &:hover, &:focus {
+            // Semantic UI darkens background using a filter, which isn't necessary when we have a hover color.
+            filter: none;
+
+            background-color: @hoverBg;
+            color: @hoverFg;
+
+            &.inverted {
+                background-color: @hoverFg;
+                color: @hoverBg;
+            }
         }
     }
 }
@@ -103,7 +105,8 @@ body.pxt-theme-root {
         > .header,
         > .actions,
         > .closeIcon,
-        > .closeIcon .close {
+        > .closeIcon .close,
+        > .content > .ui.items > .item > .content > .description {
             background-color: var(--pxt-neutral-background1);
             color: var(--pxt-neutral-foreground1);
         }
@@ -137,7 +140,6 @@ body.pxt-theme-root {
         color: var(--pxt-neutral-foreground1);
     }
 
-
     .ui.menu {
         background: var(--pxt-neutral-background1);
         color: var(--pxt-neutral-foreground1);
@@ -205,6 +207,10 @@ body.pxt-theme-root {
         }
     }
 
+    .ui.raised.segment, .ui.raised.segments {
+        box-shadow: 0 2px 4px 0 var(--pxt-neutral-alpha10),0 2px 10px 0 var(--pxt-neutral-alpha10);
+    }
+
     // Colors
     .blue {
         color: var(--pxt-colors-blue-background);
@@ -283,12 +289,12 @@ body.pxt-theme-root {
         }
 
         &.gray, &.grey, &.neutral {
-            background-color: var(--pxt-neutral-backgroundbackground2) !important;
-            color: var(--pxt-neutral-background-foreground2) !important;
+            background-color: var(--pxt-neutral-background2) !important;
+            color: var(--pxt-neutral-foreground2) !important;
 
             &.inverted {
-                background-color: var(--pxt-neutral-background-foreground2) !important;
-                color: var(--pxt-neutral-background-background2) !important;
+                background-color: var(--pxt-neutral-foreground2) !important;
+                color: var(--pxt-neutral-background2) !important;
             }
         }
 

diff --git a/theme/themes/pxt/views/card.overrides b/theme/themes/pxt/views/card.overrides
@@ -149,17 +149,17 @@ a.ui.card:focus,
     border: @linkHoverBorder;
 }
 
-.ui.card.link.newprojectcard {
-    background: var(--pxt-primary-background) !important;
+.gallerysegment .ui.card.link.newprojectcard {
+    background-color: var(--pxt-primary-background) !important;
     color: var(--pxt-primary-foreground) !important;
 
     .header {
         color: var(--pxt-primary-foreground) !important;
     }
 }
 
-.ui.card.link.cloudprojectscard {
-    background: var(--pxt-secondary-accent) !important;
+.gallerysegment .ui.card.link.cloudprojectscard {
+    background-color: var(--pxt-secondary-accent) !important;
     color: var(--pxt-secondary-foreground) !important;
 
     .header {

diff --git a/theme/tutorial-sidebar.less b/theme/tutorial-sidebar.less
@@ -6,7 +6,6 @@
 @tutorialSecondaryColor: var(--pxt-colors-blue-background);
 @tutorialBarForegroundColor: @tutorialPrimaryColor;
 @tutorialBarBackgroundColor: @tutorialSecondaryColor;
-@tutorialDarkGray: var(--pxt-neutral-background3);
 
 @tutorialLinkColor: var(--pxt-link);
 @tutorialLinkHoverColor: var(--pxt-link-hover);
@@ -74,6 +73,8 @@
 
     .lang-block .ui.segment.raised, .lang-blocks .ui.segment.raised, .ui.segment.raised.codewidget {
         overflow-x: auto;
+        background: var(--pxt-neutral-background2);
+        color: var(--pxt-neutral-foreground2);
 
         code, code.hljs {
             white-space: pre;
@@ -170,7 +171,7 @@
     user-select: none;
     cursor: default;
     font-size: 1rem;
-    color: @tutorialDarkGray;
+    color: var(--pxt-neutral-foreground1);
 }
 
 .tutorial-step-title {
@@ -578,7 +579,6 @@
         float: right;
         border: 2px solid var(--pxt-neutral-stencil1);
         margin-left: 1.5rem;
-        background: @white; // Cannot theme properly until we have a way to theme the immersive reader icon
     }
 }