Skip to content

Back button error fix #892

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rahulharpal1603 merged 1 commit into from
Dec 30, 2025
Merged

Back button error fix #892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 19 additions & 5 deletions frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, { useState, useEffect } from 'react';
import { useDispatch } from 'react-redux';
import { useDispatch, useSelector } from 'react-redux';
import {
setAvatar,
setName,
Expand All @@ -18,6 +18,7 @@ import { Input } from '@/components/ui/input';
import { Label } from '@/components/ui/label';
import { avatars } from '@/constants/avatars';
import { AppFeatures } from '@/components/OnboardingSteps/AppFeatures';
import { RootState } from '@/app/store';

interface AvatarNameSelectionStepProps {
stepIndex: number;
Expand All @@ -32,11 +33,20 @@ export const AvatarSelectionStep: React.FC<AvatarNameSelectionStepProps> = ({
}) => {
const dispatch = useDispatch();

const [name, setLocalName] = useState('');
const [selectedAvatar, setLocalAvatar] = useState('');
const [name, setLocalName] = useState(localStorage.getItem('name') || '');
const [selectedAvatar, setLocalAvatar] = useState(
localStorage.getItem('avatar') || '',
);
const isEditing = useSelector(
(state: RootState) => state.onboarding.isEditing,
);

useEffect(() => {
if (localStorage.getItem('name') && localStorage.getItem('avatar')) {
if (
localStorage.getItem('name') &&
localStorage.getItem('avatar') &&
!isEditing
) {
dispatch(markCompleted(stepIndex));
}
}, []);
Comment on lines 44 to 52
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 30, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing useEffect dependency and incomplete editing cycle.

Two concerns with this useEffect:

  1. Missing dependency: The effect uses isEditing but doesn't include it in the dependency array. If the component re-mounts while isEditing is true, the effect won't reflect the current editing state correctly.

  2. Incomplete editing cycle: When users click "Next" after editing (via handleNextClick), isEditing is never reset to false. This means once editing mode is activated, it stays active indefinitely, potentially affecting subsequent navigation.

🔎 Suggested fixes

Option 1: Add isEditing to dependencies

  useEffect(() => {
    if (
      localStorage.getItem('name') &&
      localStorage.getItem('avatar') &&
      !isEditing
    ) {
      dispatch(markCompleted(stepIndex));
    }
- }, []);
+ }, [isEditing, dispatch, stepIndex]);

Option 2: Reset isEditing when completing the step

  const handleNextClick = () => {
    dispatch(setName(name));
    dispatch(setAvatar(selectedAvatar));
    localStorage.setItem('name', name);
    localStorage.setItem('avatar', selectedAvatar);
+   dispatch(setIsEditing(false));
    dispatch(markCompleted(stepIndex));
  };

Consider whether the editing mode should persist across steps or reset when moving forward.

📝 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
useEffect(() => {
if (localStorage.getItem('name') && localStorage.getItem('avatar')) {
if (
localStorage.getItem('name') &&
localStorage.getItem('avatar') &&
!isEditing
) {
dispatch(markCompleted(stepIndex));
}
}, []);
useEffect(() => {
if (
localStorage.getItem('name') &&
localStorage.getItem('avatar') &&
!isEditing
) {
dispatch(markCompleted(stepIndex));
}
}, [isEditing, dispatch, stepIndex]);
🤖 Prompt for AI Agents 
In frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx around lines
44-52, the useEffect reads isEditing but omits it from the dependency array and
the editing flag is never reset when the user completes the step; update the
useEffect dependency array to include isEditing (and any other used values like
stepIndex/dispatch if referenced) so the effect re-runs when editing state
changes, and modify the handleNextClick flow to reset isEditing to false (or
dispatch an action that clears editing) when marking the step completed so
editing mode does not persist after moving forward.

Expand All @@ -57,7 +67,11 @@ export const AvatarSelectionStep: React.FC<AvatarNameSelectionStepProps> = ({
dispatch(markCompleted(stepIndex));
};

if (localStorage.getItem('name') && localStorage.getItem('avatar')) {
if (
localStorage.getItem('name') &&
localStorage.getItem('avatar') &&
!isEditing
) {
return null;
}

Expand Down
14 changes: 9 additions & 5 deletions frontend/src/components/OnboardingSteps/FolderSetupStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import {
} from '@/components/ui/card';
import { Button } from '@/components/ui/button';
import { FolderOpen, X, Folder } from 'lucide-react';
import { useDispatch } from 'react-redux';
import { AppDispatch } from '@/app/store';
import { useDispatch, useSelector } from 'react-redux';
import { AppDispatch, RootState } from '@/app/store';
import { markCompleted, previousStep } from '@/features/onboardingSlice';
import { AppFeatures } from '@/components/OnboardingSteps/AppFeatures';
import { useFolder } from '@/hooks/useFolder';
import { useEffect, useState } from 'react';

import { setIsEditing } from '@/features/onboardingSlice';
interface FolderSetupStepProps {
stepIndex: number;
totalSteps: number;
Expand All @@ -31,9 +31,12 @@ export function FolderSetupStep({

// Local state for folders
const [folder, setFolder] = useState<string>('');
const isEditing = useSelector(
(state: RootState) => state.onboarding.isEditing,
);

useEffect(() => {
if (localStorage.getItem('folderChosen') === 'true') {
if (localStorage.getItem('folderChosen') === 'true' && !isEditing) {
dispatch(markCompleted(stepIndex));
}
}, []);
Comment on lines 38 to 42
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 30, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Incomplete editing mode cycle - same issue as AvatarSelectionStep.

This component has the same two issues found in AvatarSelectionStep.tsx:

  1. Missing useEffect dependency: isEditing is used but not included in the dependency array (line 39).
  2. No reset mechanism: handleNext (lines 59-63) never resets isEditing to false, while handleBack (lines 65-68) sets it to true.

This creates a one-way state change where editing mode, once activated, never deactivates. Consider the same fixes suggested for AvatarSelectionStep.tsx:

  • Add isEditing to the useEffect dependency array, OR
  • Dispatch setIsEditing(false) in handleNext before marking the step complete

Also applies to: 59-68

🤖 Prompt for AI Agents 
In frontend/src/components/OnboardingSteps/FolderSetupStep.tsx around lines
38-42 and 59-68, the useEffect reads localStorage and uses isEditing but does
not include isEditing in the dependency array (line 39), and handleNext never
resets isEditing while handleBack sets it true, causing a one-way editing state;
add isEditing to the useEffect dependency array so the effect re-runs when
editing mode changes, and in handleNext (before dispatching markCompleted)
dispatch setIsEditing(false) to exit editing mode (keep handleBack setting
true).

Expand All @@ -60,10 +63,11 @@ export function FolderSetupStep({
};

const handleBack = () => {
dispatch(setIsEditing(true));
dispatch(previousStep());
};

if (localStorage.getItem('folderChosen') === 'true') {
if (localStorage.getItem('folderChosen') === 'true' && !isEditing) {
return null;
}
const progressPercent = Math.round(
Expand Down
12 changes: 10 additions & 2 deletions frontend/src/components/OnboardingSteps/ThemeSelectionStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ import { Sun, Moon, Monitor } from 'lucide-react';

import { AppFeatures } from '@/components/OnboardingSteps/AppFeatures';
import { useTheme } from '@/contexts/ThemeContext';

import { setIsEditing } from '@/features/onboardingSlice';
import { useSelector } from 'react-redux';
import { RootState } from '@/app/store';
interface ThemeSelectionStepProps {
stepIndex: number;
totalSteps: number;
Expand All @@ -30,10 +34,13 @@ export const ThemeSelectionStep: React.FC<ThemeSelectionStepProps> = ({
currentStepDisplayIndex,
}) => {
const { setTheme, theme } = useTheme();
const isEditing = useSelector(
(state: RootState) => state.onboarding.isEditing,
);
const dispatch = useDispatch<AppDispatch>();

useEffect(() => {
if (localStorage.getItem('themeChosen')) {
if (localStorage.getItem('themeChosen') === 'true' && !isEditing) {
dispatch(markCompleted(stepIndex));
}
}, []);
Comment on lines 42 to 46
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 30, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Incomplete editing mode cycle - consistent across all step components.

This is the third step component with the same two issues:

  1. Missing useEffect dependency: isEditing is used in the condition (line 43) but not in the dependency array.
  2. No reset mechanism: handleBack sets isEditing to true (line 57), but handleNext (lines 51-54) never resets it to false.

Impact: Once a user clicks "Back" from any step, isEditing remains true throughout the rest of the onboarding flow. This might be intentional to force users to review all steps, but without documentation or a clear reset point, it creates an incomplete state machine.

Recommendation: Establish a clear editing mode lifecycle. Consider one of these approaches:

  • Reset isEditing to false when any step's "Next" button is clicked
  • Reset isEditing to false when the final onboarding step is completed
  • Document if the persistent true state is intentional behavior

Also applies to: 51-59

🤖 Prompt for AI Agents 
frontend/src/components/OnboardingSteps/ThemeSelectionStep.tsx around lines
42-46 and 51-59: the useEffect reads localStorage and checks isEditing but does
not include isEditing in its dependency array, and handleBack sets isEditing to
true while handleNext never resets it; update the useEffect dependency array to
include isEditing (so the completion check runs when editing state changes) and
modify handleNext to set isEditing(false) before/after dispatching the next-step
actions (so editing mode is cleared when advancing); ensure these changes
satisfy lint rules and mirror the same fix in the other step components.

Expand All @@ -47,9 +54,10 @@ export const ThemeSelectionStep: React.FC<ThemeSelectionStepProps> = ({
};

const handleBack = () => {
dispatch(setIsEditing(true));
dispatch(previousStep());
};
if (localStorage.getItem('themeChosen')) {
if (localStorage.getItem('themeChosen') === 'true' && !isEditing) {
return null;
}

Expand Down
7 changes: 6 additions & 1 deletion frontend/src/features/onboardingSlice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ interface OnboardingState {
stepStatus: boolean[];
avatar: string | null;
name: string;
isEditing: boolean;
}

const initialState: OnboardingState = {
Expand All @@ -17,6 +18,7 @@ const initialState: OnboardingState = {
stepStatus: STEP_NAMES.map(() => false),
avatar: localStorage.getItem('avatar'),
name: localStorage.getItem('name') || '',
isEditing: false,
};
const onboardingSlice = createSlice({
name: 'onboarding',
Expand All @@ -28,6 +30,9 @@ const onboardingSlice = createSlice({
setName(state, action: PayloadAction<string>) {
state.name = action.payload;
},
setIsEditing(state, action: PayloadAction<boolean>) {
state.isEditing = action.payload;
},
markCompleted(state, action: PayloadAction<number>) {
const stepIndex = action.payload;
if (stepIndex >= 0 && stepIndex < state.stepStatus.length) {
Expand All @@ -51,7 +56,7 @@ const onboardingSlice = createSlice({
},
});

export const { setAvatar, setName, markCompleted, previousStep } =
export const { setAvatar, setName, setIsEditing, markCompleted, previousStep } =
onboardingSlice.actions;

export default onboardingSlice.reducer;
Loading