feat(web): add toggleable confirmation before unpinning a thread - #7313
feat(web): add toggleable confirmation before unpinning a thread#7313UtkarshUsername wants to merge 12 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
One finding: the new unpin setting is reset by "Restore defaults" but never reported as changed, so the restore affordance stays disabled and silently omits it. Details inline.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit aee3bb5. Configure here.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a new user-facing feature (toggleable confirmation dialog before unpinning threads) with new settings, UI controls, and dialog workflows. While well-scoped and defaulting to off, new feature capabilities warrant human review. You can add or adjust custom eligibility rules. Learn more. |
377efb2 to
57ba837
Compare
Accidental unpin from the thread context menu moves the thread out of the pinned section with no way to undo. Show a confirmation dialog that names the thread and defaults to Cancel, matching the existing archive and delete patterns. Applied to both the Sidebar context menu and the chat header action menu.
Matches the delete confirmation pattern by explaining the consequence: 'This will move the thread out of your pinned section.'
The confirmation was only in the context menu switch/case, leaving the pin icon click path uncovered. Move it into attemptUnpin so both the right-click context menu and the pin icon button are guarded.
The chat header action menu called unpinThread directly without the confirmation dialog added to attemptUnpin. Apply the same guard here so both unpin paths are covered.
- Return early when confirmThreadUnpin is enabled but thread/API lookup fails (prevents bypassing the confirmation on stale context menus) - Add confirmThreadUnpin to changedSettingLabels so Restore defaults button enables and the confirmation dialog lists it when dirty - Add settings.confirmThreadUnpin to memo dependency array
36e2c21 to
fd5028e
Compare
There was a problem hiding this comment.
Reviewed the new "Unpin confirmation" setting and its two call sites. The settings row itself composes the shared SettingsRow / Switch / SettingResetButton primitives correctly and the restore-defaults wiring is now complete. Two consistency issues remain, both in apps/web/src.
Posted via Macroscope — UI Consistency
- Skip only the dialog when local api is unavailable; fall back to a generic title when the thread lookup misses, so the confirmation setting can never swallow the unpin action (matches confirmAndDeleteThread) - Move unpin-confirmation catalog entry above archive-confirmation so settings search results match rendered row order
Dismissing prior approval to re-evaluate af7a94c
1 participant
Closes #6929
What Changed
Added a toggle to ask for confirmation before the "Unpin thread" action fires. The dialog shows the thread name (Unpin thread "Thread Title"?), a description and defaults to Cancel. The user must explicitly confirm to proceed. The confirmation toggle is turned off by default.
Why
The unpin action previously took effect immediately, making it easy to misclick while navigating the thread context menu. Pinned threads are usually curated deliberately, so an accidental unpin means losing a thread from a carefully organized list with no undo. A confirmation step matches the existing pattern used for archive and delete, and prevents accidental disruption.
UI Changes
unpin.mp4
Checklist
Note
Low Risk
Client-only UX setting with a false default; no auth, data, or unpin API changes.
Overview
Adds optional confirmation before unpinning a thread, matching archive/delete. New
confirmThreadUnpinclient setting defaults to off, so current one-click unpin stays unless the user enables it.When on, sidebar unpin and the thread action menu show a dialog with the thread title before calling
unpinThread. General settings gets a searchable toggle with reset.Reviewed by Cursor Bugbot for commit a8788b9. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add toggleable confirmation before unpinning a thread
confirmThreadUnpinboolean toClientSettingsSchema(defaultfalse) and its patch schema in settings.tsattemptUnpincallback in Sidebar.tsx and the 'unpin' handler in useThreadActionMenu.ts show a confirmation dialog (including thread title) before callingunpinThreadconfirmThreadUnpinistrue; canceling aborts the unpin. The setting defaults tofalse, preserving existing behavior unless explicitly enabledMacroscope summarized a8788b9.