Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances user feedback by adding loading indicators and refining form data handling in the quiz and portfolio optimization flows.
- Introduce
isLoadingstate with a spinner on the quiz submission button. - Add loading states (
issLoadingandisRiskLoading) and spinners in the portfolio optimization component. - Update formData keys to snake_case and adjust JSX formatting for consistency.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/app/(dashboard)/dashboard/portfolio/quiz/page.tsx | Added isLoading state, loader spinner, and disabled state for quiz submission button |
| src/app/(dashboard)/dashboard/portfolio/_components/port-opt-form.tsx | Introduced issLoading/isRiskLoading states, loader spinner for quiz redirect, and updated formData fields to snake_case |
Comments suppressed due to low confidence (1)
src/app/(dashboard)/dashboard/portfolio/_components/port-opt-form.tsx:114
- The state variable
issLoadingand its settersetissLoadingappear to have a typo and inconsistent casing. Rename them toisLoadingandsetIsLoadingfor clarity and to match standard camelCase convention.
const [issLoading, setissLoading] = useState(false);
| setIsLoading(true); | ||
| if (!user) return; | ||
| const val = totalScore; | ||
| if (val === null || isNaN(val)) return; |
There was a problem hiding this comment.
Calling setIsLoading(true) before the guard clauses (if (!user) return; and if (val === null || isNaN(val)) return;) can leave isLoading stuck in true if those early returns run. Move the loading flag after those checks or ensure it’s cleared in those branches.
| setIsLoading(true); | |
| if (!user) return; | |
| const val = totalScore; | |
| if (val === null || isNaN(val)) return; | |
| if (!user) return; | |
| const val = totalScore; | |
| if (val === null || isNaN(val)) return; | |
| setIsLoading(true); |
| const qs = new URLSearchParams({ | ||
| tickers: JSON.stringify(selectedTickers), | ||
| years, | ||
| inv: investmentAmount, | ||
| tgt: targetAmount, | ||
| }).toString(); | ||
| router.push(`/dashboard/portfolio/quiz?${qs}`); |
There was a problem hiding this comment.
You call setissLoading(true) on click but never reset it. If router.push fails or takes time, the button may remain disabled indefinitely. Consider resetting the loading state in a finally block or after navigation completes.
| const qs = new URLSearchParams({ | |
| tickers: JSON.stringify(selectedTickers), | |
| years, | |
| inv: investmentAmount, | |
| tgt: targetAmount, | |
| }).toString(); | |
| router.push(`/dashboard/portfolio/quiz?${qs}`); | |
| try { | |
| const qs = new URLSearchParams({ | |
| tickers: JSON.stringify(selectedTickers), | |
| years, | |
| inv: investmentAmount, | |
| tgt: targetAmount, | |
| }).toString(); | |
| await router.push(`/dashboard/portfolio/quiz?${qs}`); | |
| } catch (error) { | |
| console.error("Navigation failed:", error); | |
| // Optionally, show an error message to the user here | |
| } finally { | |
| setissLoading(false); | |
| } |
No description provided.