Repository navigation
Conversation
…ilure When completeTask mutation fails, the onError handler now calls toggleTask to flip the checkbox back to its original state, preventing the UI from showing an incorrect completed/incomplete status after a server error.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Daily Product Improvement
📝 TLDR
Revert the task checkbox optimistic toggle when the server mutation fails so the UI never shows incorrect task state.
📝 Description
When a user clicks a task checkbox on the Dashboard,
handleToggleimmediately callstoggleTaskto flip the visual state before the server confirms. This is the correct optimistic update pattern. However, theonErrorhandler on thecompleteTaskmutation only showed a toast — it did not calltoggleTaskagain to flip the checkbox back. If the network was flaky or the server rejected the request, the checkbox would remain in the wrong position until the next page refresh.This Change
toggleTask(variables.taskId)to theonErrorcallback so the checkbox is reverted on failureSummary
When the
completeTaskmutation fails on the Dashboard Tasks card, the checkbox now reverts to its original position in addition to showing the error toast. Previously the optimistic toggle was never rolled back on failure.Recommendations Reviewed
Area: Dashboard – VendorsCard
Issue:
vendorsisundefinedwhile loading, sovendorCount === 0is true and the "No vendors added yet" empty state flashes before real data arrivesRecommendation: Guard with
isLoadingbefore showing empty stateType: UX/UI
Priority: Medium
Effort: Small
Status: Already covered by open PR #256
Area: Dashboard – TasksCard
Issue: Optimistic task toggle not reverted when
completeTaskmutation fails — checkbox stays in wrong stateRecommendation: Call
toggleTask(variables.taskId)inonErrorto revertType: Bug fix
Priority: Medium
Effort: Small
Status: ✅ Implemented in this PR
Area: Guest List – GuestsView
Issue: "No households yet" shown when active filters return zero results, which is misleading
Recommendation: Check
households.length > 0to distinguish filtered-empty vs truly-empty stateType: UX/UI
Priority: High
Effort: Small
Status: Already covered by open PR #246
Area: Guest List – EventsTabs
Issue:
<Button>is a direct child of<ul>(invalid HTML) and tab strip has no ARIA tab semanticsRecommendation: Wrap in
<li>, addrole="tablist",role="tab",aria-selectedType: UX/UI / Accessibility
Priority: Medium
Effort: Small
Status: Already covered by open PR #244
Area: Guest List – GuestSearchFilter
Issue: Switching event tab resets text search and tag/country filters along with RSVP filter — overly aggressive
Recommendation: Only reset RSVP filter on tab change; preserve text search and other filters
Type: Product flow
Priority: Medium
Effort: Small
Status: No open PR
Area: Dashboard – DashboardTopbar
Issue: Date starts as
''and is set after mount viauseEffect, causing a visible layout shift on every page loadRecommendation: Initialize date directly (no
useEffect), addsuppressHydrationWarningon the spanType: UX/UI
Priority: Low
Effort: Small
Status: No open PR
Area: Guest List – InviteLinkPanel
Issue: Panel always renders even when there are no guests yet, making the self-invite link confusing for brand-new weddings
Recommendation: Conditionally hide the panel until at least one event or guest exists
Type: Product flow
Priority: Low
Effort: Small
Status: No open PR
Area: Dashboard – PlanningOverview
Issue: RSVP summary totals (attending/pending/declined) are computed twice with nearly identical logic in
MiniStatsandRsvpCardRecommendation: Extract into a single
computeRsvpSummaryfunction, compute once, pass as propsType: Frontend cleanup
Priority: Low
Effort: Small
Status: No open PR
Area: Guest List – GuestsView
Issue: Component is ~1,000 lines managing drawer state, mutations, sort state, filter state, and communication log queries
Recommendation: Extract drawer state + mutations into
useGuestDetailDrawerhook; extract delete confirmation intoDeleteHouseholdDialogType: Frontend cleanup
Priority: Medium
Effort: Large
Status: No open PR
Area: Dashboard – dashboard/page.tsx
Issue: Root content wrapper uses
<div>instead of<main>, breaking landmark structure for screen readersRecommendation: Replace with
<main>Type: Accessibility
Priority: Medium
Effort: Small
Status: Already covered by open PR #232
Selected Improvement
Fix task checkbox optimistic update revert on mutation failure
File:
src/components/dashboard/planning-overview.tsxAdded
toggleTask(variables.taskId)to theonErrorcallback of thecompleteTaskmutation so the checkbox reverts to its original state when the server request fails.Why This Was Selected
This is a correctness bug with direct user impact: after a network hiccup or server error, the task checkbox stays in the wrong position indefinitely. Users could believe a task was completed when it wasn't (or vice versa), leading to planning confusion. The fix is a two-line change confined to a single callback and poses zero risk to the success path.
Changes Made
TasksCard, updatedcompleteTask.useMutationonErrorcallback to receivevariablesand calltoggleTask(variables.taskId)before the error toast, reverting the optimistic toggle.Files Changed
src/components/dashboard/planning-overview.tsxVerification
npx tsc --noEmit— no errors related to this change (one pre-existing unrelated deprecation warning)onErrorcallback changed; success path,handleToggle, and rendering are untouchedvariables.taskIdmatches thetaskIdsent tocompleteTask.mutate, which is the same ID passed to the initialtoggleTaskcallFuture Recommendations
Filter reset on event tab change
Recommendation: Only reset RSVP filter on tab change; preserve text search, tag, and country filters
Priority: Medium | Effort: Small | Reason not included: Not a bug; behavioral change needs product review | GitHub issue: No
DashboardTopbar date layout shift
Recommendation: Remove
useEffect, compute date inline, addsuppressHydrationWarningPriority: Low | Effort: Small | Reason not included: Polish-only, lower impact than today's bug fix | GitHub issue: No
InviteLinkPanel always visible before guests exist
Recommendation: Hide panel until at least one event or guest is present
Priority: Low | Effort: Small | Reason not included: Minor UX nicety, warrants product input on exact trigger condition | GitHub issue: No
Duplicate RSVP summary computation
Recommendation: Extract
computeRsvpSummary, compute once atPlanningOverviewlevelPriority: Low | Effort: Small | Reason not included: Code cleanup only, no user-visible impact | GitHub issue: No
GuestsView component decomposition
Recommendation: Extract
useGuestDetailDrawerhook andDeleteHouseholdDialogfrom the 1,000-line componentPriority: Medium | Effort: Large | Reason not included: Large refactor, requires careful testing of drawer state, optimistic updates, and delete flow | GitHub issue: Yes — spans multiple interaction flows and needs dedicated review
GitHub Issues Created or Proposed
GuestsView component decomposition — proposed as a GitHub issue. The component has grown to ~1,000 lines with drawer state, mutation logic, sort state, filter state, and communication log queries all co-located. Extracting a
useGuestDetailDrawerhook and aDeleteHouseholdDialogwould significantly improve maintainability and testability, but the scope warrants a separate PR and careful QA of the drawer open/close/dirty-state lifecycle.Generated by Claude Code