Feat: add goal completion toast#75
Conversation
|
@MansimranKaurBedi0 is attempting to deploy a commit to the PRIYANSHU DOSHI's projects Team on Vercel. A member of the Team first needs to authorize it. |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Good idea and the feature works conceptually — Toast component, notifiedGoals tracking, and the completion check logic are all reasonable. A few things need fixing.
Blockers
1. onClose in useEffect deps causes the timer to reset on every render
In Toast.tsx:
useEffect(() => {
const timer = setTimeout(() => { onClose(); }, 3000);
return () => clearTimeout(timer);
}, [onClose]);In GoalTracker.tsx, onClose is passed as an inline arrow function:
<Toast message={toastMessage} onClose={() => setToastMessage("")} />Every time GoalTracker re-renders, onClose is a new function reference. The effect sees a changed dep, cancels the old timer, and starts a new 3-second countdown — so the toast may never close.
Fix: run the effect only once with empty deps, and use a ref for onClose:
const onCloseRef = useRef(onClose);
useEffect(() => {
const timer = setTimeout(() => onCloseRef.current(), 3000);
return () => clearTimeout(timer);
}, []);2. Indentation in Toast.tsx
File uses 4-space indent. Rest of codebase uses 2-space. Please align.
Minor
- Missing EOF newline in
Toast.tsxandGoalTracker.tsx. page.tsxchange (removing a blank line between import and component) is unrelated — please revert it..catch(() => { })has a stray space inside braces — use.catch(() => {}).animate-bounceon the toast is distracting. Consider removing it or usinganimate-pulse/ no animation.
Fix the two blockers and this is ready.
|
Implemented all requested changes: ✅ Fixed toast timer issue Lint and type-check are passing. Kindly review again. Thanks! |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
The GoalTracker diff has regressions that block this PR:
1. Stripped className from buttons and inputs
All styling has been removed from buttons, inputs, and labels inside GoalTracker.tsx. The create form fields and action buttons appear as unstyled bare HTML elements. This breaks the existing UI.
2. Missing label elements
The <label> elements were removed from the create form, hurting accessibility.
Please restore the original GoalTracker styling (CSS vars, border, bg-[var(--control)], etc.) and only add your toast functionality on top — don't modify the existing markup structure or styles.
|
Hi @MansimranKaurBedi0 — this PR has a merge conflict with git fetch upstream
git rebase upstream/main
# resolve conflicts, then:
git push --force-with-leaseOnce rebased, we'll review and merge. |
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
- Runtime bug — goal.label does not exist. The field is goal.title in the current codebase. The toast will always show undefined. Fix: Goal complete: ${goal.title}! 2. Infinite loop risk — [goals, notifiedGoals] in useEffect deps means every setNotifiedGoals call re-fires the effect. Use a ref for notifiedGoals tracking instead of state. 3. Multiple simultaneous completions — setToastMessage overwrites the previous; only the last goal shows. Use a queue or array of active toasts. 4. Missing EOF newlines on GoalTracker.tsx and Toast.tsx.
Priyanshu-byte-coder
left a comment
There was a problem hiding this comment.
Two issues:
-
Missing EOF newline — add a trailing newline to GoalTracker.tsx.
-
Raw Tailwind colors —
text-red-400,text-red-300,text-red-500are used. Replace all withtext-[var(--destructive)](with appropriate opacity if needed). Colors must go through CSS vars for theme compatibility.
|
This PR has merge conflicts with |
|
This PR has conflicts with the current |
|
Hey @MansimranKaurBedi0! Saw your work on GSSoC 2026. We are building TermUI, a TypeScript terminal UI framework with React-style hooks and JSX, rendered entirely in the terminal. We have 55 unassigned GSSoC issues open, including new widgets, hooks, and CLI tooling. Your TypeScript background transfers directly. Karanjot, TermUI maintainer |
|
This PR has merge conflicts with the current main branch. Please rebase on the latest main to resolve them — your contribution is labeled for GSSoC scoring. |
|
Hi! This PR has merge conflicts with the git fetch upstream
git rebase upstream/main
# resolve any conflicts
git push --force-with-leaseIf you're no longer working on this, let us know and we can close it. Thanks! |
|
This PR has merge conflicts with the main branch. Please rebase your branch on latest main and resolve the conflicts so it can be reviewed and merged. |
|
This PR has been open for a long time and has merge conflicts with the current If you're still interested in contributing this change, please:
If no update is made within 2 weeks, this PR will be closed as stale. |
Summary
Implemented in-app goal completion toast notifications for the weekly goal tracker.
Closes #65
Type of Change
Changes Made
Toastcomponent for in-app notificationsHow to Test
Steps for the reviewer to verify this works:
npm installandnpm run devGoalTrackercurrent >= targetScreenshots (if UI change)
Added UI screenshot of toast notification after goal completion.
Checklist
npm run lintpasses locallynpm run type-check)