fix(desktop): show spinner on install update button while pending#3561
Conversation
Disabled state alone didn't feel responsive — click registered without obvious acknowledgement. Adds a spinning icon alongside the existing "Installing..." label so users see the action was received.
📝 WalkthroughWalkthroughTwo desktop UI components are enhanced to display visual feedback during update installation. Both the UpdateRequiredPage and UpdateToast components now conditionally render an animated spinning icon when the install mutation is pending, providing clearer user indication of the installation process. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR improves the install update UX in both Key changes:
Confidence Score: 4/5Safe to merge after adding the missing gap-2 class on the toast Install button for consistent spacing The logic is correct and well-implemented — disabled state, pending text, and spinning icon all work together as expected. The only issue is a minor style inconsistency (missing gap-2 in UpdateToast) that causes the spinner and label to render without spacing. apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx — missing gap-2 on the Install button
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx | Adds spinning HiArrowPath icon and "Installing…" label when installMutation is pending; missing gap-2 class causes icon/text to render without spacing |
| apps/desktop/src/renderer/components/UpdateRequiredPage/UpdateRequiredPage.tsx | Adds spinning HiArrowPath icon and "Installing…" label when installMutation is pending; correctly includes gap-2 for icon/text spacing |
Sequence Diagram
sequenceDiagram
participant U as User
participant B as Install Button
participant M as installMutation
participant E as Electron IPC
U->>B: Click Install / Install & Restart
B->>M: mutate()
M-->>B: isPending = true
B->>B: disabled=true, show spinner + "Installing..."
M->>E: autoUpdate.install()
E-->>M: resolve / reject
note over E: On success, app restarts
M-->>B: isPending = false (if no restart)
B->>B: re-enable, show normal state
Comments Outside Diff (1)
-
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx, line 91-100 (link)Missing
gap-2on Install buttonWhen
installMutation.isPendingistrue, the spinningHiArrowPathicon and the "Installing…" label are rendered as sibling flex children inside<Button>, but unlike the equivalent button inUpdateRequiredPage.tsx(line 93), this button has noclassName="gap-2". Without a gap, the icon and text will be jammed together with no visual separation.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx Line: 91-100 Comment: **Missing `gap-2` on Install button** When `installMutation.isPending` is `true`, the spinning `HiArrowPath` icon and the "Installing…" label are rendered as sibling flex children inside `<Button>`, but unlike the equivalent button in `UpdateRequiredPage.tsx` (line 93), this button has no `className="gap-2"`. Without a gap, the icon and text will be jammed together with no visual separation. How can I resolve this? If you propose a fix, please make it concise.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx
Line: 91-100
Comment:
**Missing `gap-2` on Install button**
When `installMutation.isPending` is `true`, the spinning `HiArrowPath` icon and the "Installing…" label are rendered as sibling flex children inside `<Button>`, but unlike the equivalent button in `UpdateRequiredPage.tsx` (line 93), this button has no `className="gap-2"`. Without a gap, the icon and text will be jammed together with no visual separation.
```suggestion
<Button
size="sm"
onClick={handleInstall}
disabled={installMutation.isPending}
className="gap-2"
>
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(desktop): show spinner on install up..." | Re-trigger Greptile
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx (1)
91-100: Add the same icon/text spacing used by the required-page button.This toast button now renders an icon next to text, but unlike
UpdateRequiredPage, it does not addgap-2, so the spinner can sit too close to the label depending on the sharedButtonstyles.Proposed spacing tweak
<Button size="sm" onClick={handleInstall} disabled={installMutation.isPending} + className="gap-2" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx` around lines 91 - 100, In UpdateToast's install Button (the Button rendering inside UpdateToast that uses handleInstall and installMutation), add the same horizontal spacing used in UpdateRequiredPage by applying the gap-2 utility when an icon is present; e.g. pass a className (or merge with existing className) that includes "gap-2" conditionally when installMutation.isPending is true so the spinner and label have consistent spacing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx`:
- Around line 91-100: In UpdateToast's install Button (the Button rendering
inside UpdateToast that uses handleInstall and installMutation), add the same
horizontal spacing used in UpdateRequiredPage by applying the gap-2 utility when
an icon is present; e.g. pass a className (or merge with existing className)
that includes "gap-2" conditionally when installMutation.isPending is true so
the spinner and label have consistent spacing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97355f34-26e3-4e1d-a988-5595414678df
📒 Files selected for processing (2)
apps/desktop/src/renderer/components/UpdateRequiredPage/UpdateRequiredPage.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
UpdateToastandUpdateRequiredPagenow show a spinningHiArrowPathicon while the mutation is pending, alongside the existing disabled (dimmed) state and "Installing..." labelTest plan
Summary by cubic
Show a spinning icon on the update Install buttons while the install mutation is pending to acknowledge the click and improve feedback. Applies to the Install button in the update toast and the Install & Restart button on the Update Required page, alongside the existing disabled state and "Installing..." label.
Written for commit 035437b. Summary will update on new commits.
Summary by CodeRabbit