Add dark mode toggle button
Needs RevisionPublic
Actions

Authored by Chenani-MohamedAmine on Tue, Mar 3, 00:38.

Details

Reviewers

ieli

Maniphest Tasks

T2246: Add dark mode toggle button

Summary

This adds a sun/moon button in AppNavbar.vue that toggles the dark
class on <html> and persists the choice in localStorage.

New files:

composables/useDarkMode.js: toggle logic, localStorage persistence, system preference fallback
components/__tests__/useDarkMode.test.js: 5 tests covering toggle, persistence, and class manipulation

Ref T2246

Test Plan

Visit / in light mode — moon icon visible in navbar
Click moon icon — page switches to dark mode, icon becomes sun
Refresh page — dark mode persists (read from localStorage)
Click sun icon — switches back to light mode
Clear localStorage — defaults to system preference
All 5 useDarkMode tests pass (npm run test:unit)

Diff Detail

Repository

rSP ServPulse

Lint

Lint Passed

Unit

No Test Coverage

Branch

feature/T2246-dark-mode-toggle

Build Status

Buildable 6394
Build 6678: arc lint + arc unit

Event Timeline

Chenani-MohamedAmine requested review of this revision.Tue, Mar 3, 00:38

Chenani-MohamedAmine created this revision.

Harbormaster completed remote builds in B6394: Diff 10346.Tue, Mar 3, 00:38

The implementation uses a boolean dark class on the <html> element. For better flexibility, consider using a data-theme attribute instead of a boolean class. This allows each theme to define its own dark/light variants. Additionally, the toggle button should be hidden for mono themes (themes with only one palette). The useDarkMode composable should expose whether the theme is "toggleable", and the navbar should conditionally render the button based on this. Finally, theme configuration should be available in the admin side - the Settings tab in AdminDashboard.vue would be the appropriate place to add theme selection.

frontend/src/components/__tests__/useDarkMode.test.js
91	Stray `Z` character that causes ReferenceError: Z is not defined. This breaks the entire test suite (0 tests run). This is a major issue caused by a lack of attention to detail - a simple typo that should have been caught before committing. Please be more mindful of the code you submit for review.
frontend/src/composables/useDarkMode.js
5	`isDark` is a module-level `ref(false)`. This shared mutable state persists across component remounts. If the component using `useDarkMode` unmounts and remounts, the theme state may behave unexpectedly.
34–36	`initTheme()` is called inside `onMounted`, which only runs on the client. On server-side rendering, the initial HTML won't have the dark class, but the client will add it after mounting, causing a hydration mismatch. Consider calling `initTheme()` at module load time instead.

This revision now requires changes to proceed.Tue, Mar 3, 07:37

ieli added a subscriber: dereckson.Tue, Mar 3, 12:27

This comment was removed by ieli.

Chenani-MohamedAmine updated this revision to Diff 10355.Tue, Mar 3, 15:01

This comment was removed by Chenani-MohamedAmine.

Harbormaster completed remote builds in B6403: Diff 10355.Tue, Mar 3, 15:01

In D3989#61933, @Chenani-MohamedAmine wrote:

"Ref T2246"

I believe it's in the description that this belongs to.

Chenani-MohamedAmine edited the summary of this revision. (Show Details)Tue, Mar 3, 23:55

Chenani-MohamedAmine added a task: T2246: Add dark mode toggle button.

Fixed the remaining issues (tailwind config, useDarkMode with data-theme, stray Z in test)

Harbormaster completed remote builds in B6407: Diff 10360.Wed, Mar 4, 00:21

dereckson edited the summary of this revision. (Show Details)Wed, Mar 4, 06:44

In D3989#61878, @ieli wrote:

The implementation uses a boolean dark class on the <html> element. For better flexibility, consider using a data-theme attribute instead of a boolean class. This allows each theme to define its own dark/light variants. Additionally, the toggle button should be hidden for mono themes (themes with only one palette). The useDarkMode composable should expose whether the theme is "toggleable", and the navbar should conditionally render the button based on this. Finally, theme configuration should be available in the admin side - the Settings tab in AdminDashboard.vue would be the appropriate place to add theme selection.

Theme configuration missing from admin settings. Re-read carefuly the comment quoted.

This revision now requires changes to proceed.Wed, Mar 4, 12:12

Added theme configuration in admin Settings tab (mode selector + toggleable checkbox) as requested.

Harbormaster completed remote builds in B6425: Diff 10380.Wed, Mar 4, 16:14

In D3989#62108, @Chenani-MohamedAmine wrote:

Added theme configuration in admin Settings tab (mode selector + toggleable checkbox) as requested.

Not quite. The implementation is incorrect. I never asked for:

☑ Allow users to toggle theme
The theme toggle button will be hidden from the navbar.

This checkbox approach is wrong. Themes should be defined as either:

Pair themes (e.g., Standard, Solarized, etc...) - have both light AND dark variants. Users can toggle between them.
Single themes (e.g., Sepia, etc...) - have only ONE variant. No toggle should appear because there's nothing to toggle between.

The admin selects a theme, not a "varient". The toggle visibility is automatically determined by the theme's variant support - NOT by a manual checkbox. Users get the toggle when the theme supports multiple variants.

Please re-read the original requirement and redesign accordingly.

This revision now requires changes to proceed.Wed, Mar 4, 17:17