From 78b5767a6d983b3463d98a1d31a683457744a664 Mon Sep 17 00:00:00 2001 From: Michael Myers Date: Sun, 9 Jun 2024 09:40:37 -0700 Subject: [PATCH 1/3] Fix theme picker text when unspecified theme --- web/packages/design/src/ThemeProvider/index.tsx | 17 +++++++++++++++++ .../src/components/UserMenuNav/UserMenuNav.tsx | 11 +++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/web/packages/design/src/ThemeProvider/index.tsx b/web/packages/design/src/ThemeProvider/index.tsx index e2c6517dda2b9..2f792b97f0809 100644 --- a/web/packages/design/src/ThemeProvider/index.tsx +++ b/web/packages/design/src/ThemeProvider/index.tsx @@ -39,6 +39,23 @@ function themePreferenceToTheme(themePreference: Theme) { return themePreference === Theme.LIGHT ? lightTheme : darkTheme; } +// because unspecific can exist but only used as a fallback and not an option, +// we need to get the current/next themes with getPrefersDark in mind. +// TODO (avatus) when we add user settings page, we can add a Theme.SYSTEM option +// and remove the checks for unspecified +export function getCurrentTheme(currentTheme: Theme): Theme { + if (currentTheme === Theme.UNSPECIFIED) { + return getPrefersDark() ? Theme.DARK : Theme.LIGHT; + } +} + +export function getNextTheme(currentTheme: Theme): Theme { + if (currentTheme === Theme.UNSPECIFIED) { + return getPrefersDark() ? Theme.LIGHT : Theme.DARK; + } + return currentTheme === Theme.LIGHT ? Theme.DARK : Theme.LIGHT; +} + export function getPrefersDark(): boolean { return ( window.matchMedia && diff --git a/web/packages/teleport/src/components/UserMenuNav/UserMenuNav.tsx b/web/packages/teleport/src/components/UserMenuNav/UserMenuNav.tsx index f8ca291cccc82..f59075f25f294 100644 --- a/web/packages/teleport/src/components/UserMenuNav/UserMenuNav.tsx +++ b/web/packages/teleport/src/components/UserMenuNav/UserMenuNav.tsx @@ -22,6 +22,7 @@ import styled, { useTheme } from 'styled-components'; import { Moon, Sun, ChevronDown, Logout as LogoutIcon } from 'design/Icon'; import { Text } from 'design'; import { useRefClickOutside } from 'shared/hooks/useRefClickOutside'; +import { getCurrentTheme, getNextTheme } from 'design/ThemeProvider'; import { Theme } from 'gen-proto-ts/teleport/userpreferences/v1/theme_pb'; @@ -124,11 +125,10 @@ export function UserMenuNav({ username, iconSize }: UserMenuNavProps) { const ctx = useTeleport(); const clusterId = ctx.storeUser.getClusterId(); const features = useFeatures(); + const currentTheme = getCurrentTheme(preferences.theme); + const nextTheme = getNextTheme(preferences.theme); const onThemeChange = () => { - const nextTheme = - preferences.theme === Theme.LIGHT ? Theme.DARK : Theme.LIGHT; - updatePreferences({ theme: nextTheme }); setOpen(false); }; @@ -182,10 +182,9 @@ export function UserMenuNav({ username, iconSize }: UserMenuNavProps) { - {preferences.theme === Theme.DARK ? : } + {currentTheme === Theme.DARK ? : } - Switch to {preferences.theme === Theme.DARK ? 'Light' : 'Dark'}{' '} - Theme + Switch to {currentTheme === Theme.DARK ? 'Light' : 'Dark'} Theme )} From 2b112c0096800e4aab4eb61273c0d651c9e18830 Mon Sep 17 00:00:00 2001 From: Michael Myers Date: Mon, 10 Jun 2024 17:20:38 -0500 Subject: [PATCH 2/3] Add tests and fix undefined return --- .../design/src/ThemeProvider/index.tsx | 4 +- .../userPreferences/userPreferences.test.ts | 43 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/web/packages/design/src/ThemeProvider/index.tsx b/web/packages/design/src/ThemeProvider/index.tsx index 2f792b97f0809..747d128970d5a 100644 --- a/web/packages/design/src/ThemeProvider/index.tsx +++ b/web/packages/design/src/ThemeProvider/index.tsx @@ -39,7 +39,7 @@ function themePreferenceToTheme(themePreference: Theme) { return themePreference === Theme.LIGHT ? lightTheme : darkTheme; } -// because unspecific can exist but only used as a fallback and not an option, +// because unspecified can exist but only used as a fallback and not an option, // we need to get the current/next themes with getPrefersDark in mind. // TODO (avatus) when we add user settings page, we can add a Theme.SYSTEM option // and remove the checks for unspecified @@ -47,6 +47,8 @@ export function getCurrentTheme(currentTheme: Theme): Theme { if (currentTheme === Theme.UNSPECIFIED) { return getPrefersDark() ? Theme.DARK : Theme.LIGHT; } + + return currentTheme; } export function getNextTheme(currentTheme: Theme): Theme { diff --git a/web/packages/teleport/src/services/userPreferences/userPreferences.test.ts b/web/packages/teleport/src/services/userPreferences/userPreferences.test.ts index d32b766245060..0595e3a19ecbd 100644 --- a/web/packages/teleport/src/services/userPreferences/userPreferences.test.ts +++ b/web/packages/teleport/src/services/userPreferences/userPreferences.test.ts @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +import { getCurrentTheme, getNextTheme } from 'design/ThemeProvider'; import { Theme } from 'gen-proto-ts/teleport/userpreferences/v1/theme_pb'; import { UserPreferences } from 'gen-proto-ts/teleport/userpreferences/v1/userpreferences_pb'; @@ -72,3 +73,45 @@ test('should convert the user preferences back to the old format when updating', actualUserPreferences.clusterPreferences.pinnedResources.resourceIds ); }); + +test('getCurrentTheme', () => { + mockMatchMediaWindow('dark'); + let currentTheme = getCurrentTheme(Theme.UNSPECIFIED); + expect(currentTheme).toBe(Theme.DARK); + + mockMatchMediaWindow('light'); + currentTheme = getCurrentTheme(Theme.UNSPECIFIED); + expect(currentTheme).toBe(Theme.LIGHT); + + currentTheme = getCurrentTheme(Theme.LIGHT); + expect(currentTheme).toBe(Theme.LIGHT); + + currentTheme = getCurrentTheme(Theme.DARK); + expect(currentTheme).toBe(Theme.DARK); +}); + +test('getNextTheme', () => { + mockMatchMediaWindow('dark'); + let nextTheme = getNextTheme(Theme.UNSPECIFIED); + expect(nextTheme).toBe(Theme.LIGHT); + + mockMatchMediaWindow('light'); + nextTheme = getNextTheme(Theme.UNSPECIFIED); + expect(nextTheme).toBe(Theme.DARK); + + nextTheme = getNextTheme(Theme.LIGHT); + expect(nextTheme).toBe(Theme.DARK); + + nextTheme = getNextTheme(Theme.DARK); + expect(nextTheme).toBe(Theme.LIGHT); +}); + +function mockMatchMediaWindow(prefers: 'light' | 'dark') { + return Object.defineProperty(window, 'matchMedia', { + writable: true, + value: jest.fn().mockImplementation(query => ({ + matches: query === `(prefers-color-scheme: ${prefers})`, + media: query, + })), + }); +} From 63b7dc39ed459ae139e7272f12a1302bb8bc2fad Mon Sep 17 00:00:00 2001 From: Michael Myers Date: Tue, 11 Jun 2024 11:26:21 -0500 Subject: [PATCH 3/3] Use getCurrentTheme --- web/packages/design/src/ThemeProvider/index.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/web/packages/design/src/ThemeProvider/index.tsx b/web/packages/design/src/ThemeProvider/index.tsx index 747d128970d5a..af212d8ab3862 100644 --- a/web/packages/design/src/ThemeProvider/index.tsx +++ b/web/packages/design/src/ThemeProvider/index.tsx @@ -52,10 +52,9 @@ export function getCurrentTheme(currentTheme: Theme): Theme { } export function getNextTheme(currentTheme: Theme): Theme { - if (currentTheme === Theme.UNSPECIFIED) { - return getPrefersDark() ? Theme.LIGHT : Theme.DARK; - } - return currentTheme === Theme.LIGHT ? Theme.DARK : Theme.LIGHT; + return getCurrentTheme(currentTheme) === Theme.LIGHT + ? Theme.DARK + : Theme.LIGHT; } export function getPrefersDark(): boolean {