From f7f0afa672fc59f7e65be01c0ff48b2b5b6797fa Mon Sep 17 00:00:00 2001 From: adamleithp Date: Thu, 19 Dec 2024 10:48:41 +0000 Subject: [PATCH] fix: moved our theme switching from theme= to class= based --- .storybook/decorators/withTheme.tsx | 5 +++-- .storybook/preview.tsx | 9 ++++++++- .../src/layout/navigation-3000/Navigation.scss | 4 ++-- frontend/src/lib/hooks/useThemedHtml.ts | 3 ++- frontend/src/scenes/themes/CustomCssScene.tsx | 4 ++-- frontend/src/styles/global.scss | 4 ++-- frontend/src/toolbar/Toolbar.stories.tsx | 10 +++++++--- frontend/src/toolbar/ToolbarContainer.tsx | 14 ++++++-------- frontend/src/toolbar/bar/Toolbar.scss | 2 +- tailwind.config.js | 1 + 10 files changed, 34 insertions(+), 22 deletions(-) diff --git a/.storybook/decorators/withTheme.tsx b/.storybook/decorators/withTheme.tsx index 90699cd60ffab5..496878b56c788b 100644 --- a/.storybook/decorators/withTheme.tsx +++ b/.storybook/decorators/withTheme.tsx @@ -6,8 +6,9 @@ import type { Decorator } from '@storybook/react' export const withTheme: Decorator = (Story, context) => { const theme = context.globals.theme - // set the theme - document.body.setAttribute('theme', theme === 'dark' ? 'dark' : 'light') + // Update class instead of attribute + document.body.classList.remove('theme-light', 'theme-dark') + document.body.classList.add(theme === 'dark' ? 'theme-dark' : 'theme-light') return } diff --git a/.storybook/preview.tsx b/.storybook/preview.tsx index e81ceec38df610..e63a49546bcfe5 100644 --- a/.storybook/preview.tsx +++ b/.storybook/preview.tsx @@ -110,10 +110,17 @@ const preview: Preview = { ), }, + backgrounds: { + default: 'light', + values: [ + { name: 'light', value: 'var(--bg-light)' }, + { name: 'dark', value: 'var(--bg-3000-dark)' } + ] + } }, globalTypes: { theme: { - description: '', + description: 'Global theme for components', defaultValue: 'light', toolbar: { title: 'Theme', diff --git a/frontend/src/layout/navigation-3000/Navigation.scss b/frontend/src/layout/navigation-3000/Navigation.scss index 42bb779a54d820..8eca5678be3939 100644 --- a/frontend/src/layout/navigation-3000/Navigation.scss +++ b/frontend/src/layout/navigation-3000/Navigation.scss @@ -187,7 +187,7 @@ // Hidden when the sidebar is closed border-right: min(1px, var(--sidebar-width)) solid transparent; - [theme='dark'] & { + .theme-dark & { --sidebar-background: var(--accent-3000); } @@ -360,7 +360,7 @@ flex-shrink: 0; min-height: var(--accordion-row-height); - [theme='dark'] & { + .theme-dark & { --accordion-header-background: var(--bg-3000); } diff --git a/frontend/src/lib/hooks/useThemedHtml.ts b/frontend/src/lib/hooks/useThemedHtml.ts index f09efd2659b327..3acfc48f7547b8 100644 --- a/frontend/src/lib/hooks/useThemedHtml.ts +++ b/frontend/src/lib/hooks/useThemedHtml.ts @@ -17,7 +17,8 @@ export function useThemedHtml(overflowHidden = true): void { document.head.removeChild(oldStyle) } - document.body.setAttribute('theme', isDarkModeOn ? 'dark' : 'light') + document.body.classList.remove('theme-light', 'theme-dark') + document.body.classList.add(isDarkModeOn ? 'theme-dark' : 'theme-light') if (customCss) { const newStyle = document.createElement('style') diff --git a/frontend/src/scenes/themes/CustomCssScene.tsx b/frontend/src/scenes/themes/CustomCssScene.tsx index affe0430cb67af..8ade552564caeb 100644 --- a/frontend/src/scenes/themes/CustomCssScene.tsx +++ b/frontend/src/scenes/themes/CustomCssScene.tsx @@ -18,7 +18,7 @@ const TRON_THEME = `:root { --radius: 0px; } -body[theme=dark] { +.theme-dark { --border: rgba(0, 255, 1, 0.5); --link: #00FF01; --border-bold: #00FF01; @@ -49,7 +49,7 @@ const BARBIE_THEME = `:root { --radius: 16px; } -body[theme=light] { +.theme-light { --border: rgba(255, 105, 180, 0.5); --border-3000: #ff409f; --link: #E306AD; diff --git a/frontend/src/styles/global.scss b/frontend/src/styles/global.scss index 7b26ed870f2860..e63b3cc72b11f3 100644 --- a/frontend/src/styles/global.scss +++ b/frontend/src/styles/global.scss @@ -648,11 +648,11 @@ body { touch-action: manipulation; // Disable double-tap-to-zoom on mobile, making taps slightly snappier background: var(--bg-3000); - &[theme='light'] { + &.theme-light { @include light-mode-3000-variables; } - &[theme='dark'] { + &.theme-dark { // Semantic colors (Dark mode) WIP --content-primary: var(--neutral-cool-100); --content-warning: var(--orange-300); diff --git a/frontend/src/toolbar/Toolbar.stories.tsx b/frontend/src/toolbar/Toolbar.stories.tsx index 8c7a898fb5cfe8..da1de8ad4dcd17 100644 --- a/frontend/src/toolbar/Toolbar.stories.tsx +++ b/frontend/src/toolbar/Toolbar.stories.tsx @@ -2,7 +2,7 @@ import '~/styles' import '~/toolbar/styles.scss' import { Meta, StoryFn } from '@storybook/react' -import { useActions, useMountedLogic } from 'kea' +import { useActions, useMountedLogic, useValues } from 'kea' import { useEffect } from 'react' import { useStorybookMocks } from '~/mocks/browser' @@ -89,13 +89,17 @@ const BasicTemplate: StoryFn = (props) => { const theToolbarLogic = toolbarLogic() const { setVisibleMenu, setDragPosition, toggleMinimized, toggleTheme } = useActions(theToolbarLogic) + const { theme: currentTheme } = useValues(theToolbarLogic) useEffect(() => { setDragPosition(50, 50) setVisibleMenu(props.menu || 'none') toggleMinimized(props.minimized ?? false) - toggleTheme(props.theme || 'light') - }, [Object.values(props)]) + + if (props.theme && props.theme !== currentTheme) { + toggleTheme() + } + }, [props.menu, props.minimized, props.theme]) return (
diff --git a/frontend/src/toolbar/ToolbarContainer.tsx b/frontend/src/toolbar/ToolbarContainer.tsx index 06a395d3edcd64..66ae76a6ebace9 100644 --- a/frontend/src/toolbar/ToolbarContainer.tsx +++ b/frontend/src/toolbar/ToolbarContainer.tsx @@ -1,3 +1,4 @@ +import clsx from 'clsx' import { useValues } from 'kea' import { Fade } from 'lib/components/Fade/Fade' import { FloatingContainerContext } from 'lib/hooks/useFloatingContainerContext' @@ -13,20 +14,17 @@ import { toolbarConfigLogic } from './toolbarConfigLogic' export function ToolbarContainer(): JSX.Element { const { buttonVisible } = useValues(toolbarConfigLogic) const { theme } = useValues(toolbarLogic) - - // KLUDGE: if we put theme directly on the div then - // linting and typescript complain about it not being - // a valid attribute. So we put it in a variable and - // spread it in. 🤷‍ - const themeProps = { theme } - const ref = useRef(null) return ( -
+
diff --git a/frontend/src/toolbar/bar/Toolbar.scss b/frontend/src/toolbar/bar/Toolbar.scss index 73d5edf974a5cc..8382e7140c9a79 100644 --- a/frontend/src/toolbar/bar/Toolbar.scss +++ b/frontend/src/toolbar/bar/Toolbar.scss @@ -33,7 +33,7 @@ z-index: 2147483020; } - &[theme='dark'] { + &.theme-dark { // override the mark value so that feature flag overrides are visible _and legible_ in dark mode --mark: var(--secondary-3000-hover-dark); diff --git a/tailwind.config.js b/tailwind.config.js index fe0262567e3ee5..005bb37144abaa 100644 --- a/tailwind.config.js +++ b/tailwind.config.js @@ -102,6 +102,7 @@ const config = { }, }, plugins: [require('@tailwindcss/container-queries')], + darkMode: 'class', } module.exports = config