Uh oh!
There was an error while loading. Please reload this page.
feat(mobile): Long press to copy message text - #2060
Conversation
Rebuild PR #2013 on top of current origin/main without the duplicated commit lineage. Co-authored-by: codex <codex@users.noreply.github.com>
- Add syntax-highlighted diff rendering and cached review state - Support pasted/attached images in review comments - Improve mobile review sheet handling for large or partial diffs Co-authored-by: codex <codex@users.noreply.github.com>
- Switch review screen to a virtualized list layout - Queue diff highlighting and default to the JS engine - Add word-diff helpers and tests
Co-authored-by: Julius Marminge <julius@macmini.local>
- Use variant-specific Expo schemes for development, preview, and production - Preserve the production `t3code` scheme while fixing remote-connect routing
Long press on user or assistant message bubbles opens a form sheet with selectable text and a "Copy all" button. Extracts a reusable createExternalStore utility and refactors both message copy and review comment stores to use it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
| const onCopyAll = useCallback(() => { | ||
| if (messageText) { | ||
| void Clipboard.setStringAsync(messageText); | ||
| void Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success); | ||
| setCopied(true); | ||
| dismissTimerRef.current = setTimeout(() => { | ||
| dismissTimerRef.current = null; | ||
| router.back(); | ||
| }, 600); | ||
| } | ||
| }, [messageText, router]); |
There was a problem hiding this comment.
🟡 Mediumthreads/MessageCopySheet.tsx:45
In onCopyAll, if the button is pressed multiple times before React re-renders with disabled={copied}, the code overwrites dismissTimerRef.current without clearing the previous timer. This leaks the previous timer ID, and if the component unmounts before all timers fire, only the last timer gets cleaned up. The orphaned timer(s) will still call router.back() potentially after the user has navigated elsewhere.
const onCopyAll = useCallback(() => {
if (messageText) {
void Clipboard.setStringAsync(messageText);
void Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success);
setCopied(true);
+ if (dismissTimerRef.current) {+ clearTimeout(dismissTimerRef.current);+ }
dismissTimerRef.current = setTimeout(() => {
dismissTimerRef.current = null;
router.back();
}, 600);
}
}, [messageText, router]);🤖 Copy this AI Prompt to have your agent fix this:
In file apps/mobile/src/features/threads/MessageCopySheet.tsx around lines 45-55:
In `onCopyAll`, if the button is pressed multiple times before React re-renders with `disabled={copied}`, the code overwrites `dismissTimerRef.current` without clearing the previous timer. This leaks the previous timer ID, and if the component unmounts before all timers fire, only the last timer gets cleaned up. The orphaned timer(s) will still call `router.back()` potentially after the user has navigated elsewhere.
Evidence trail:
apps/mobile/src/features/threads/MessageCopySheet.tsx lines 44-54 (onCopyAll function creates timer without clearing existing one), lines 37-42 (cleanup effect only clears dismissTimerRef.current which is only the last timer), lines 49-52 (timer assignment overwrites ref without clearTimeout), line 99 (disabled={copied} relies on React re-render)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6bead6f. Configure here.
| key={attachment.id} | ||
| activeOpacity={0.7} | ||
| onPress={() => props.onPressImage(uri, headers)} | ||
| <Pressable onLongPress={() => props.onLongPressMessage(message.text)}> |
There was a problem hiding this comment.
Long-pressing image-only messages opens empty copy sheet
Medium Severity
The onLongPress handler unconditionally passes message.text to the copy sheet, even when the text is empty (e.g., image-only messages). Since OrchestrationMessage.text is Schema.String (allows ""), setMessageCopyText("") stores an empty string. In MessageCopySheet, the guard if (!messageText) treats "" as falsy and returns null, so the user sees a confusing empty form sheet (just a grabber on a blank background) that they must manually dismiss. The onLongPressMessage handler needs to skip opening the sheet when the text is empty.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6bead6f. Configure here.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces a new user-facing feature (long press to copy message text) with new components, routes, and state management. Additionally, there are two unresolved medium-severity bugs identified: a timer leak on rapid button presses and an empty sheet appearing for image-only messages. You can customize Macroscope's approvability policy. Learn more. |
b7da92c to
4d137a1Compare
copy-feature-clean.mp4
Summary
createExternalStoreutility — refactored bothmessageCopySelectionandreviewCommentSelectionto use it, eliminating ~60 lines of duplicate boilerplate🤖 Generated with Claude Code