Add Theme Switching Functionality to Translate View #3002
Conversation
| .toggle-button button { | ||
| background: var(--black-3); | ||
| border: 1px solid var(--light-grey-3); | ||
| border-radius: 3px; | ||
| box-sizing: border-box; | ||
| color: var(--grey-6); | ||
| font-size: 14px; | ||
| font-weight: 100; | ||
| padding: 4px; | ||
| width: 78px; | ||
| margin: 0; | ||
| float: none; | ||
| } | ||
|
|
||
| .toggle-button button:first-child { | ||
| border-right-color: var(--grey-3); | ||
| border-top-right-radius: 0; | ||
| border-bottom-right-radius: 0; | ||
| } | ||
|
|
||
| .toggle-button button:nth-child(2) { | ||
| margin: 0 8px; | ||
| } | ||
|
|
||
| .toggle-button button:last-child { | ||
| border-left: 1px solid var(--light-grey-3); | ||
| border-top-left-radius: 0; | ||
| border-bottom-left-radius: 0; | ||
| } | ||
|
|
||
| .toggle-button button:hover { | ||
| color: var(--light-grey-6); | ||
| } | ||
|
|
||
| .toggle-button button.active { | ||
| background: var(--dark-grey-1); | ||
| border-color: var(--grey-3); | ||
| color: var(--light-grey-7); | ||
| font-weight: 400; | ||
| } |
There was a problem hiding this comment.
We'll need to sync these colors with the ones used in https://github.com/mozilla/pontoon/pull/2997/files#diff-30336dc4648b06dd8f0228e1c1013ab8260d8a5404cb86cba8187ed141519f1bR1461, but that should be done in the theme CSS PR.
| }, [theme]); | ||
|
|
||
| const handleThemeButtonClick = (selectedTheme: string) => { | ||
| setTheme(selectedTheme); // Update the local state |
There was a problem hiding this comment.
State already gets updated through the onThemeChange() method, which ends up calling the saveTheme() action.
The only thing we should call here is applyTheme(). We should factor that function out and then import it here and in Theme.tsx. Then you can remove lines 43-87.
There was a problem hiding this comment.
We should instead call applyTheme() here.
| @@ -1,4 +1,4 @@ | |||
| # Generated by Django 3.2.15 on 2023-10-18 21:47 | |||
| # Generated by Django 3.2.15 on 2023-10-25 16:35 | |||
translate/src/hooks/useTheme.ts
Outdated
| function applyTheme(newTheme: string) { | ||
| if (newTheme === 'system') { | ||
| newTheme = getSystemTheme(); | ||
| } | ||
| document.body.classList.remove( | ||
| 'dark-theme', | ||
| 'light-theme', | ||
| 'system-theme', | ||
| ); | ||
| document.body.classList.add(`${newTheme}-theme`); | ||
| } |
There was a problem hiding this comment.
That and getSystemTheme() is all that should remain in this hook.
Everything else should go back to Theme.tsx, because it's only needed there.
| @@ -37,6 +40,19 @@ export function UserMenuDialog({ | |||
| const ref = useRef<HTMLUListElement>(null); | |||
| useOnDiscard(ref, onDiscard); | |||
|
|
|||
| const [theme, setTheme] = useState<string>( | |||
| () => document.body.getAttribute('data-theme') || 'dark', | |||
| ); | |||
There was a problem hiding this comment.
We can get the theme from the user state, so no need for this block.
| }, [theme]); | ||
|
|
||
| const handleThemeButtonClick = (selectedTheme: string) => { | ||
| setTheme(selectedTheme); // Update the local state |
There was a problem hiding this comment.
We should instead call applyTheme() here.
| onClick={() => handleThemeButtonClick('dark')} | ||
| > | ||
| <i className='icon far fa-moon'></i>Dark | ||
| </button> |
There was a problem hiding this comment.
We should make the buttons and the label above localizable. Ideally, we should also factor out the button template.
translate/src/context/Theme.tsx
Outdated
| const mediaQuery = window.matchMedia('(prefers-color-scheme: dark)'); | ||
| function handleThemeChange(e: MediaQueryListEvent) { | ||
| let userThemeSetting = document.body.getAttribute('data-theme') || 'dark'; | ||
|
|
||
| if (userThemeSetting === 'system') { | ||
| applyTheme(e.matches ? 'dark' : 'light'); | ||
| } | ||
| } | ||
|
|
||
| mediaQuery.addEventListener('change', handleThemeChange); | ||
|
|
||
| applyTheme(theme); | ||
|
|
||
| return () => { | ||
| mediaQuery.removeEventListener('change', handleThemeChange); | ||
| }; | ||
| }, [theme]); |
There was a problem hiding this comment.
We still need the event handlers for system theme changes.
mathjazz
force-pushed
the
2936-user-profile-toggle
branch
from
10bc55d to
8f0d730
Compare
This pull request introduces theme switching functionality directly within the translate view, allowing users to easily switch between light, dark, and system themes in the profile menu. (Issue #2936 )
Changes: