Skip to content

feat(web): add appearance setting (follow system / dark / light) - #253

Merged
tiann merged 3 commits into
tiann:mainfrom
metowolf:feat/appearance-setting
Mar 8, 2026
Merged

feat(web): add appearance setting (follow system / dark / light)#253
tiann merged 3 commits into
tiann:mainfrom
metowolf:feat/appearance-setting

Conversation

@metowolf

@metowolf metowolf commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add user-facing appearance preference (Follow System / Dark / Light) to the settings page
  • Persist preference to localStorage, taking priority over automatic system/Telegram detection in the existing theme pipeline
  • Include i18n support for both English and Chinese locales
  • Add unit tests for the new Appearance setting

Test plan

  • All 17 existing + new tests pass (bun run test)
  • Verify appearance dropdown renders correctly on the settings page
  • Verify selecting Dark/Light applies immediately and persists across page reloads
  • Verify "Follow System" removes the override and falls back to system/Telegram detection

Add user-facing appearance preference to the settings page, allowing
users to choose between Follow System, Dark, and Light themes. The
preference is persisted to localStorage and takes priority over
automatic detection in the existing theme pipeline.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Cross-tab appearance change does not update theme — storage handler updates state only, so data-theme and useTheme subscribers stay stale until another event fires, evidence web/src/hooks/useTheme.ts:133
    Suggested fix:
    const onStorage = (event: StorageEvent) => {
        if (event.key !== APPEARANCE_KEY) return
        setAppearanceState(parseAppearance(event.newValue))
        updateScheme()
    }

Summary

  • One minor sync bug; otherwise ok. Residual risk: cross-tab theme mismatch.

Testing

  • Not run (automation)

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Cross-tab appearance change does not update theme — storage handler updates state only, so data-theme and useTheme subscribers stay stale until another event fires, evidence web/src/hooks/useTheme.ts:133
    Suggested fix:
    const onStorage = (event: StorageEvent) => {
        if (event.key !== APPEARANCE_KEY) return
        setAppearanceState(parseAppearance(event.newValue))
        updateScheme()
    }

Summary

  • One minor sync bug; otherwise ok. Residual risk: cross-tab theme mismatch.

Testing

  • Not run (automation)

Comment thread web/src/hooks/useTheme.ts

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Cross-tab appearance change does not update theme — storage handler updates state only, so data-theme and useTheme subscribers stay stale until another event fires, evidence web/src/hooks/useTheme.ts:133
    Suggested fix:
    const onStorage = (event: StorageEvent) => {
        if (event.key !== APPEARANCE_KEY) return
        setAppearanceState(parseAppearance(event.newValue))
        updateScheme()
    }

Summary

  • One minor sync bug; otherwise ok. Residual risk: cross-tab theme mismatch.

Testing

  • Not run (automation)

The storage event handler only updated React state without calling
updateScheme(), leaving data-theme and useTheme subscribers stale.
@tiann

tiann commented Mar 6, 2026

Copy link
Copy Markdown
Owner

@tiann review again

@github-actions

github-actions Bot commented Mar 6, 2026

Copy link
Copy Markdown

Reviewed again. One remaining issue:

  • Cross-tab appearance sync still only works while the settings page is mounted. The storage listener is inside useAppearance() at web/src/hooks/useTheme.ts:127, but that hook is only consumed by the settings route at web/src/routes/settings/index.tsx:85. On any other page, no listener updates currentScheme / data-theme, so changing appearance in another tab leaves the current tab stale until reload or another theme event. I’d move the storage listener into the global theme initialization/store path instead.

Testing: not run here; bun is not installed in this runner.


HAPI Bot

The storage event listener was inside useAppearance(), which is only
mounted on the settings page. Other pages never received cross-tab
theme updates. Move the listener into initializeTheme() so all pages
respond to appearance changes from other tabs.
@metowolf

metowolf commented Mar 6, 2026

Copy link
Copy Markdown
Contributor Author

image

image

@tiann

tiann commented Mar 7, 2026

Copy link
Copy Markdown
Owner

@tiann review

@github-actions

github-actions Bot commented Mar 7, 2026

Copy link
Copy Markdown

Reviewed again. I don't see a remaining concrete issue in this patch set.

  • The previous cross-tab sync bug looks addressed: initializeTheme() now installs a global storage listener, so pages outside settings also re-run updateScheme() when hapi-appearance changes in another tab: web/src/hooks/useTheme.ts:158.
  • The settings page label still stays in sync because useAppearance() keeps its own storage listener and updates the local appearance state: web/src/hooks/useTheme.ts:127.
  • The new setting is wired into the UI in web/src/routes/settings/index.tsx:241.

Testing: not run here because bun is not installed in this runner.


HAPI Bot

@tiann
tiann merged commit a0c35bc into tiann:main Mar 8, 2026
1 check passed
@metowolf
metowolf deleted the feat/appearance-setting branch March 8, 2026 02:49
jasonczc pushed a commit to jasonczc/haqi that referenced this pull request Mar 15, 2026
…nn#253)

* feat(web): add appearance setting (follow system / dark / light)

Add user-facing appearance preference to the settings page, allowing
users to choose between Follow System, Dark, and Light themes. The
preference is persisted to localStorage and takes priority over
automatic detection in the existing theme pipeline.

* fix(web): update theme on cross-tab appearance change

The storage event handler only updated React state without calling
updateScheme(), leaving data-theme and useTheme subscribers stale.

* fix(web): move cross-tab appearance sync to global initializeTheme

The storage event listener was inside useAppearance(), which is only
mounted on the settings page. Other pages never received cross-tab
theme updates. Move the listener into initializeTheme() so all pages
respond to appearance changes from other tabs.

(cherry picked from commit a0c35bc)
SeeFlowerX pushed a commit to SeeFlowerX/hapi that referenced this pull request Apr 1, 2026
…nn#253)

* feat(web): add appearance setting (follow system / dark / light)

Add user-facing appearance preference to the settings page, allowing
users to choose between Follow System, Dark, and Light themes. The
preference is persisted to localStorage and takes priority over
automatic detection in the existing theme pipeline.

* fix(web): update theme on cross-tab appearance change

The storage event handler only updated React state without calling
updateScheme(), leaving data-theme and useTheme subscribers stale.

* fix(web): move cross-tab appearance sync to global initializeTheme

The storage event listener was inside useAppearance(), which is only
mounted on the settings page. Other pages never received cross-tab
theme updates. Move the listener into initializeTheme() so all pages
respond to appearance changes from other tabs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants