Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions app/components/ThemeToggle/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
*
* ThemeToggle Component
*
*/
import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { createStructuredSelector } from 'reselect';
import styled from 'styled-components';
import { themeActionCreators } from '@app/containers/Theme/reducer';
import { selectTheme } from '@app/containers/Theme/selectors';

const ToggleButton = styled.button`
background: var(--color-accent-muted);
border: 1px solid var(--color-border);
border-radius: 50%;
width: 40px;
height: 40px;
padding: 0;
color: var(--color-text);
cursor: pointer;
font-size: 20px;
line-height: 1;
font-family: inherit;
transition: all var(--transition-fast);
display: flex;
align-items: center;
justify-content: center;
flex-shrink: 0;

&:hover {
background: var(--color-accent);
color: #fff;
border-color: var(--color-accent);
}

&:active {
transform: scale(0.98);
}
`;

export function ThemeToggle({ theme, onToggle }) {
return (
<ToggleButton
onClick={onToggle}
aria-label={theme === 'dark' ? 'Switch to light mode' : 'Switch to dark mode'}
data-testid="theme-toggle"
>
{theme === 'dark' ? '☀️' : '🌙'}
</ToggleButton>
);
}

ThemeToggle.propTypes = {
theme: PropTypes.string.isRequired,
onToggle: PropTypes.func.isRequired
};

const mapStateToProps = createStructuredSelector({
theme: selectTheme()
});

const mapDispatchToProps = (dispatch) => ({
onToggle: () => dispatch(themeActionCreators.toggleTheme())
});

export default connect(mapStateToProps, mapDispatchToProps)(ThemeToggle);
48 changes: 48 additions & 0 deletions app/containers/Theme/reducer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
*
* Theme reducer
*
*/
import { createActions } from 'reduxsauce';
import produce from 'immer';

export const THEME_PAYLOAD = {
THEME: 'theme'
};

export const initialState = {
theme: 'light' // 'light' or 'dark'
Comment on lines +13 to +14

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

initialState hardcodes 'light' — causes a flash of wrong theme on page load.

When a user with a saved 'dark' preference revisits, the Redux store initializes to 'light'. Until an action is dispatched to restore the saved theme (if that even happens), CSS variables will render in the wrong mode, causing a visible flash. A common fix is to seed initialState from localStorage in the browser, or apply the data-theme attribute in a blocking <script> in _document.js before React hydrates.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/containers/Theme/reducer.js` around lines 12 - 13, initialState currently
hardcodes theme: 'light' causing a flash; change initialState in reducer.js to
read the persisted value when available (use
window.localStorage.getItem('theme') or similar) and fall back to 'light' when
missing, i.e. create initialState.theme = (typeof window !== 'undefined' &&
localStorage.getItem('theme')) || 'light'; ensure the reducer/exported
initialState and any function names referencing initialState or theme still
work; alternatively, if you prefer a non-Redux fix, add a blocking script in
_document.js that reads localStorage and sets
document.documentElement.dataset.theme before React hydrates.

};

export const { Types: themeActionTypes, Creators: themeActionCreators } = createActions({
setTheme: [THEME_PAYLOAD.THEME],
toggleTheme: null
});

function persistTheme(theme) {
if (typeof window !== 'undefined') {
localStorage.setItem('theme', theme);
document.documentElement.setAttribute('data-theme', theme);
}
}

export function themeReducer(state = initialState, action) {
return produce(state, (draft) => {
switch (action.type) {
case themeActionTypes.SET_THEME:
draft.theme = action.theme;
persistTheme(action.theme);
Comment on lines +32 to +34

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

SET_THEME applies action.theme without validation — can corrupt localStorage.

If setTheme() is called with undefined or an invalid string, draft.theme is set to that value and persistTheme(undefined) writes the literal string "undefined" to localStorage. On the next page load, the persisted theme is unrecognizable and the store may behave unexpectedly.

🛡️ Proposed fix — guard against invalid values
 case themeActionTypes.SET_THEME:
+  if (action.theme !== 'light' && action.theme !== 'dark') break;
   draft.theme = action.theme;
   persistTheme(action.theme);
   break;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
case themeActionTypes.SET_THEME:
draft.theme = action.theme;
persistTheme(action.theme);
case themeActionTypes.SET_THEME:
if (action.theme !== 'light' && action.theme !== 'dark') break;
draft.theme = action.theme;
persistTheme(action.theme);
break;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/containers/Theme/reducer.js` around lines 32 - 34, The SET_THEME reducer
currently assigns draft.theme = action.theme and calls
persistTheme(action.theme) without validation, which can persist "undefined";
update the case handling in the reducer to validate action.theme (e.g., check
typeof action.theme === 'string' and/or membership in the allowed theme set)
before assigning draft.theme and calling persistTheme; if invalid, skip
persisting and fall back to a safe default or retain the existing draft.theme
(or call persistTheme with the fallback), and optionally extract the validation
into a helper like isValidTheme to keep the SET_THEME case concise.

break;
case themeActionTypes.TOGGLE_THEME: {
const newTheme = draft.theme === 'light' ? 'dark' : 'light';
draft.theme = newTheme;
persistTheme(newTheme);
break;
}
default:
break;
}
});
Comment on lines +22 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Side effects (localStorage + DOM mutation) inside a reducer violate Redux's pure-function contract.

persistTheme writes to localStorage and mutates document.documentElement directly from within the reducer. Redux reducers must be pure and side-effect-free. This breaks:

  • Time-travel debugging (replaying actions will re-apply DOM side effects).
  • Server-side rendering (even with the typeof window guard, the intent of purity is broken).
  • Unit testing (tests must mock global browser APIs to test simple action dispatches).

The persistence and DOM update should be moved to a middleware, a saga, or a useEffect that watches the Redux theme state.

♻️ Suggested approach — move side effects out of the reducer

Keep the reducer pure:

-function persistTheme(theme) {
-  if (typeof window !== 'undefined') {
-    localStorage.setItem('theme', theme);
-    document.documentElement.setAttribute('data-theme', theme);
-  }
-}

 export function themeReducer(state = initialState, action) {
   return produce(state, (draft) => {
     switch (action.type) {
       case themeActionTypes.SET_THEME:
         draft.theme = action.theme;
-        persistTheme(action.theme);
         break;
       case themeActionTypes.TOGGLE_THEME: {
         const newTheme = draft.theme === 'light' ? 'dark' : 'light';
         draft.theme = newTheme;
-        persistTheme(newTheme);
         break;
       }
       ...
     }
   });
 }

Then in a layout component or _app.js, react to changes:

// In a useEffect watching the theme selector:
useEffect(() => {
  localStorage.setItem('theme', theme);
  document.documentElement.setAttribute('data-theme', theme);
}, [theme]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/containers/Theme/reducer.js` around lines 21 - 44, The reducer contains
side effects: remove all calls to persistTheme (and the persistTheme helper)
from themeReducer and ensure the reducer (handling themeActionTypes.SET_THEME
and themeActionTypes.TOGGLE_THEME) only updates draft.theme and returns state;
then implement persistence and DOM mutation outside the reducer (for example: a
React useEffect in your layout/_app component or a Redux middleware/saga) that
subscribes to the theme selector and performs localStorage.setItem('theme',
theme) and document.documentElement.setAttribute('data-theme', theme) when the
state changes.

}

export default themeReducer;
8 changes: 8 additions & 0 deletions app/containers/Theme/selectors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { createSelector } from 'reselect';
import { initialState } from './reducer';

const selectThemeDomain = (state) => state.theme || initialState;

export const selectTheme = () => createSelector(selectThemeDomain, (substate) => substate.theme);

export const selectIsDarkMode = () => createSelector(selectThemeDomain, (substate) => substate.theme === 'dark');