Feat/text selection code copy#358
Conversation
- Replace TouchableOpacity bubble wrapper with View so native text selection gestures reach inner text nodes - Swap @ronradtke/react-native-markdown-display for react-native-enriched-markdown (native Fabric renderer) to enable cross-paragraph text selection - Split markdown rendering: code fences extracted and rendered as a custom CodeBlock component with language label and Copy button; prose segments rendered by EnrichedMarkdownText with selectable=true - Remove Copy and Regenerate from long-press action sheet; restore full action sheet (Copy all, Regenerate, Edit, Generate Image) via the ••• meta row button - Fix phi model detection regex to use word-boundary + digit pattern so dolphin models are no longer incorrectly blocked - Add terminal icon in chat header to open existing DebugLogsScreen with touch and structure logs for diagnosing gesture issues Co-authored-by: Dishit Karia <hanmadishit74@gmail.com>"
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
There was a problem hiding this comment.
Code Review
This pull request replaces the markdown rendering library with react-native-enriched-markdown, adds a debug logs screen, and introduces logging for message content interactions. Feedback focuses on a regression where the action menu became inaccessible after replacing TouchableOpacity with a View. Additionally, the reviewer advised against manually splitting markdown segments with regex due to potential issues with text selection and streaming UX, suggested using a non-deprecated Clipboard API, and recommended avoiding .trim() on markdown content to preserve necessary whitespace.
| <View | ||
| testID={isUser ? 'user-message' : 'assistant-message'} | ||
| style={[ | ||
| styles.container, | ||
| isUser ? styles.userContainer : styles.assistantContainer, | ||
| ]} | ||
| activeOpacity={0.8} | ||
| onLongPress={handleLongPress} | ||
| delayLongPress={300} | ||
| > |
There was a problem hiding this comment.
The replacement of TouchableOpacity with a View and the removal of the handleLongPress logic (previously at line 233) makes the ActionMenuSheet inaccessible. Users can no longer open the menu to access essential actions like Copy, Edit, Retry, or Generate Image. While this change likely aims to enable native text selection, a replacement trigger for the action menu (such as a dedicated 'more' button or a different gesture) should be implemented to avoid this regression.
| const [copied, setCopied] = useState(false); | ||
|
|
||
| const handleCopy = useCallback(() => { | ||
| Clipboard.setString(code); |
| function splitSegments(text: string): Segment[] { | ||
| const segments: Segment[] = []; | ||
| const fence = /^```([^\n]*)\n([\s\S]*?)^```/gm; | ||
| let lastIndex = 0; | ||
| let match: RegExpExecArray | null; | ||
|
|
||
| while ((match = fence.exec(text)) !== null) { | ||
| if (match.index > lastIndex) { | ||
| const md = text.slice(lastIndex, match.index).trim(); | ||
| if (md) segments.push({ type: 'markdown', content: md }); | ||
| } | ||
| segments.push({ type: 'code', language: match[1].trim(), code: match[2] }); | ||
| lastIndex = match.index + match[0].length; | ||
| } | ||
|
|
||
| if (lastIndex < text.length) { | ||
| const md = text.slice(lastIndex).trim(); | ||
| if (md) segments.push({ type: 'markdown', content: md }); | ||
| } | ||
|
|
||
| return segments; | ||
| } |
There was a problem hiding this comment.
Manually splitting markdown with regex is fragile and introduces several issues:
- Selection Continuity: Because markdown segments and code blocks are rendered as separate top-level children in a
View, users cannot perform a single continuous text selection that spans across a paragraph and a code block. - Streaming UX: During streaming, the closing backticks are missing, so the regex won't match. The code block will be rendered by the markdown library until it's finished, at which point it will suddenly 'pop' into this custom
CodeBlockcomponent, causing a flickering effect and layout shift. - Markdown Integrity: It can break structures like nested lists or blockquotes if a code block appears inside them.
Since react-native-enriched-markdown appears to support custom styles (see line 204), it is better to use the library's built-in customization hooks (e.g., renderRules for the fence or code_block types) to inject the Copy button directly into the markdown rendering pipeline.
| const md = text.slice(lastIndex, match.index).trim(); | ||
| if (md) segments.push({ type: 'markdown', content: md }); |
There was a problem hiding this comment.
Using .trim() on markdown segments removes leading and trailing whitespace (including newlines) that may be critical for correct markdown parsing and visual spacing between paragraphs and code blocks.
| const md = text.slice(lastIndex, match.index).trim(); | |
| if (md) segments.push({ type: 'markdown', content: md }); | |
| const md = text.slice(lastIndex, match.index); | |
| if (md) segments.push({ type: 'markdown', content: md }); |



Summary
TouchableOpacitywrapper on chat message bubbles withView— theTouchableOpacitywas intercepting all touch gestures, so the native OS text selection UI never triggered. Long press would open the action sheet instead of showing selection handles@ronradtke/react-native-markdown-displayforreact-native-enriched-markdown— the old lib rendered each paragraph as aView, which is a hard OS boundary for text selection. Cross-paragraph selection was impossible regardless of the wrapper fix. The new lib is a Fabric-native renderer (New Architecture) that parses markdown in C and renders as native text, enabling cross-paragraph selection out of the boxCodeBlockcomponent with a language label and per-block Copy button — the new lib's native renderer cannot host custom views inside code blocks so we split rendering at the JS layerImpact