Skip to content

Commit cdd3cd7

Browse files
smgvchaitanyapotti
andauthored
fix: Improved useTheme Hook and Unlock Page Logo Theme Handling (#38002)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/38002?quickstart=1) This PR improves the `useTheme` hook implementation to provide more robust theme resolution and updates the unlock page logo to properly respond to theme changes. ### 1. **Enhanced `useTheme` Hook** (`ui/hooks/useTheme.ts`) - **Refactored theme resolution logic** into a separate `resolveTheme()` function for better code organization and testability - **Improved fallback mechanism** with the following priority: 1. User's explicit theme setting (`light` or `dark`) 2. System theme when "OS" is selected 3. Document theme attribute on initial load 4. System preference as final fallback ### 2. **Unlock Page Logo Component** (`ui/pages/unlock-page/`) - Created new `MetaMaskWordmarkLogo` component that responds to theme changes - Updated styles to properly display logo based on theme - Integrated `useTheme` hook for dynamic theme-aware rendering ### 3. **Test Snapshots Updates** Updated Jest snapshots to reflect the new consistent theme behavior: - `ui/components/multichain-accounts/smart-contract-account-toggle/__snapshots__/smart-contract-account-toggle.test.tsx.snap` - `ui/components/multichain/network-list-menu/__snapshots__/network-list-menu.test.tsx.snap` - `ui/pages/settings/advanced-tab/__snapshots__/advanced-tab.component.test.js.snap` - `ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap` **Why snapshots changed:** The improved `useTheme` hook now properly resolves to `light` theme by default in tests (instead of `undefined`), causing `ToggleButton` components to consistently render with light theme colors (`rgb(183, 187, 200)` = `#b7bbc8` = `lightTheme.colors.icon.muted`). ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Updated useTheme hook and logo for unlock page ## **Related issues** Fixes: ## **Manual testing steps** 1. Open Extension 2. Create Wallet 3. Lock Wallet 4. Validate the changes ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/d14095eb-b4e5-4db7-a93f-bad6682f91c6 ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Refactors `useTheme` for reliable light/dark resolution and updates the unlock page wordmark logo to respond to theme, with minor CSS tweaks and snapshot updates. > > - **Hooks**: > - **`useTheme`**: Extracts `resolveTheme(settingTheme)` with clear priority (explicit setting > OS when selected > document on initial load > system), validates against `validThemes`, warns and defaults to `light` if invalid; initializes state via resolver and applies on setting changes. > - **Unlock Page UI**: > - **New `MetamaskWordmarkLogo`**: Uses `useTheme` and renders `MetaFoxHorizontalLogo` with theme awareness; replaces direct logo usage in `unlock-page.component.js`. > - **Styles**: Adjusts `unlock-page__mascot-container__horizontal-logo--popup` margins with responsive override. > - **Tests**: > - Snapshot updates across settings and multichain components reflecting consistent toggle colors (e.g., off `rgb(183, 187, 200)`, on `rgb(68, 89, 255)`). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 839eae4. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Chaitanya Potti <[email protected]>
1 parent 6a69624 commit cdd3cd7

File tree

10 files changed

+103
-60
lines changed

10 files changed

+103
-60
lines changed

ui/components/multichain-accounts/smart-contract-account-toggle-section/__snapshots__/smart-contract-account-toggle-section.test.tsx.snap

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ exports[`SmartContractAccountToggleSection Disabled State Handling disables all
5151
style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
5252
>
5353
<div
54-
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(139, 153, 255);"
54+
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(68, 89, 255);"
5555
>
5656
<div
5757
style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 1; width: 26px; height: 20px; left: 4px;"
@@ -101,7 +101,7 @@ exports[`SmartContractAccountToggleSection Disabled State Handling disables all
101101
style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
102102
>
103103
<div
104-
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(75, 80, 92);"
104+
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(183, 187, 200);"
105105
>
106106
<div
107107
style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"
@@ -313,7 +313,7 @@ exports[`SmartContractAccountToggleSection Network Integration displays toggle f
313313
style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
314314
>
315315
<div
316-
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(139, 153, 255);"
316+
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(68, 89, 255);"
317317
>
318318
<div
319319
style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 1; width: 26px; height: 20px; left: 4px;"
@@ -363,7 +363,7 @@ exports[`SmartContractAccountToggleSection Network Integration displays toggle f
363363
style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
364364
>
365365
<div
366-
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(75, 80, 92);"
366+
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(183, 187, 200);"
367367
>
368368
<div
369369
style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"

ui/components/multichain-accounts/smart-contract-account-toggle/__snapshots__/smart-contract-account-toggle.test.tsx.snap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ exports[`SmartContractAccountToggle Basic Rendering shows toggle as OFF for unsu
1818
style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
1919
>
2020
<div
21-
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(75, 80, 92);"
21+
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(183, 187, 200);"
2222
>
2323
<div
2424
style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"
@@ -73,7 +73,7 @@ exports[`SmartContractAccountToggle Basic Rendering shows toggle as enabled for
7373
style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
7474
>
7575
<div
76-
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(139, 153, 255);"
76+
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(68, 89, 255);"
7777
>
7878
<div
7979
style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 1; width: 26px; height: 20px; left: 4px;"
@@ -128,7 +128,7 @@ exports[`SmartContractAccountToggle Disabled State disables toggle during pendin
128128
style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
129129
>
130130
<div
131-
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(139, 153, 255);"
131+
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(68, 89, 255);"
132132
>
133133
<div
134134
style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 1; width: 26px; height: 20px; left: 4px;"

ui/components/multichain/network-list-menu/__snapshots__/network-list-menu.test.tsx.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,7 @@ exports[`NetworkListMenu renders properly 1`] = `
759759
style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
760760
>
761761
<div
762-
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(75, 80, 92);"
762+
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(183, 187, 200);"
763763
>
764764
<div
765765
style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"
@@ -1867,7 +1867,7 @@ exports[`NetworkListMenu should match snapshot when editing a network 1`] = `
18671867
style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
18681868
>
18691869
<div
1870-
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(75, 80, 92);"
1870+
style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(183, 187, 200);"
18711871
>
18721872
<div
18731873
style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"

ui/hooks/useTheme.ts

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,39 +4,65 @@ import { getTheme } from '../selectors';
44
import { ThemeType } from '../../shared/constants/preferences';
55

66
/**
7-
* List of valid themes. Should return an array with only the values ThemeType.light and ThemeType.dark
8-
* unless there is a future we add more themes.
7+
* List of valid themes.
98
*/
109
const validThemes = Object.values(ThemeType).filter((theme) => {
1110
return theme !== ThemeType.os;
1211
});
1312

13+
/**
14+
* Resolves the theme based on the user's theme setting.
15+
*
16+
* @param settingTheme - The theme setting from user preferences
17+
* @returns The resolved theme (light or dark)
18+
*/
19+
function resolveTheme(
20+
settingTheme: string | undefined,
21+
): ThemeType.light | ThemeType.dark {
22+
const systemTheme = window.matchMedia('(prefers-color-scheme: dark)').matches
23+
? ThemeType.dark
24+
: ThemeType.light;
25+
26+
const documentTheme = document.documentElement.getAttribute('data-theme');
27+
28+
let resolvedTheme;
29+
if (settingTheme === ThemeType.os) {
30+
resolvedTheme = systemTheme;
31+
} else if (settingTheme) {
32+
resolvedTheme = settingTheme;
33+
} else {
34+
// Initial load
35+
resolvedTheme = documentTheme || systemTheme;
36+
}
37+
38+
const isValidTheme = validThemes.includes(
39+
resolvedTheme as ThemeType.light | ThemeType.dark,
40+
);
41+
42+
if (!isValidTheme) {
43+
console.warn(
44+
`useTheme: Invalid theme resolved to "${resolvedTheme}". Defaulting to "${ThemeType.light}".`,
45+
);
46+
return ThemeType.light;
47+
}
48+
49+
return resolvedTheme as ThemeType.light | ThemeType.dark;
50+
}
51+
1452
/**
1553
* Returns the current theme based on the user's theme setting.
1654
*
1755
* @returns theme
1856
*/
19-
export function useTheme() {
57+
export function useTheme(): ThemeType.light | ThemeType.dark {
2058
const settingTheme = useSelector(getTheme);
21-
const [theme, setTheme] = useState(settingTheme);
59+
const [theme, setTheme] = useState<ThemeType.light | ThemeType.dark>(() =>
60+
resolveTheme(settingTheme),
61+
);
2262

2363
useEffect(() => {
24-
const result =
25-
!settingTheme || settingTheme === ThemeType.os
26-
? document.documentElement.getAttribute('data-theme')
27-
: settingTheme;
28-
const isValidTheme = validThemes.includes(
29-
result as ThemeType.light | ThemeType.dark,
30-
);
31-
32-
if (!isValidTheme) {
33-
console.warn(
34-
`useTheme: Invalid theme resolved to "${result}". Defaulting to "${ThemeType.light}".`,
35-
);
36-
setTheme(ThemeType.light);
37-
}
38-
39-
setTheme(result);
64+
const resolved = resolveTheme(settingTheme);
65+
setTheme(resolved);
4066
}, [settingTheme]);
4167

4268
return theme;

0 commit comments

Comments
 (0)