[codex] Add native mobile composer and markdown#3101
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.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)
Comment |
github-actions
Bot
added
vouch:trusted
|
🚀 Expo continuous deployment is ready!
|
![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 and found 3 potential issues.
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Caret not restored after update
- Changed the condition from
savedRange.length > 0tosavedRange.location != NSNotFoundso collapsed carets (zero-length selections) are also restored after attributedText reassignment, matching the behavior in refreshDisplayedAttachments.
- Changed the condition from
- ✅ Fixed: Bold weight maps to semibold
- Changed the mapping for font weight 700/"700" from "semibold" to "bold" so markdown bold and heading runs render at the correct weight on iOS.
- ✅ Fixed: Theme prop forces full rebuild
- Added a string equality guard in setThemeJson (matching the tokensJson pattern), stored the raw themeJson string for comparison, and memoized the themeJson construction on the JS side to prevent redundant document rebuilds on re-renders.
Or push these changes by commenting:
@cursor push f7e5cbe9a3
Preview (f7e5cbe9a3)
diff --git a/apps/mobile/modules/t3-composer-editor/ios/T3ComposerEditorView.swift b/apps/mobile/modules/t3-composer-editor/ios/T3ComposerEditorView.swift
--- a/apps/mobile/modules/t3-composer-editor/ios/T3ComposerEditorView.swift
+++ b/apps/mobile/modules/t3-composer-editor/ios/T3ComposerEditorView.swift
@@ -263,6 +263,7 @@
private var tokensJson = "[]"
private var tokens: [ComposerTokenPayload] = []
private var requestedSelection: ComposerSelectionPayload?
+ private var themeJson = ""
private var theme = ComposerThemePayload(
text: "#262626",
placeholder: "#8e8e93",
@@ -377,6 +378,10 @@
}
func setThemeJson(_ themeJson: String) {
+ guard self.themeJson != themeJson else {
+ return
+ }
+ self.themeJson = themeJson
guard let nextTheme = decode(ComposerThemePayload.self, from: themeJson) else {
return
}
diff --git a/apps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm b/apps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
--- a/apps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
+++ b/apps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
@@ -427,7 +427,7 @@
const NSRange savedRange = _textView.selectedRange;
_suppressSelectionChange = YES;
_textView.attributedText = convertedAttrString;
- if (savedRange.length > 0 && NSMaxRange(savedRange) <= _textView.attributedText.length) {
+ if (savedRange.location != NSNotFound && NSMaxRange(savedRange) <= _textView.attributedText.length) {
_textView.selectedRange = savedRange;
}
_suppressSelectionChange = NO;
diff --git a/apps/mobile/modules/t3-markdown-text/src/util.ts b/apps/mobile/modules/t3-markdown-text/src/util.ts
--- a/apps/mobile/modules/t3-markdown-text/src/util.ts
+++ b/apps/mobile/modules/t3-markdown-text/src/util.ts
@@ -48,7 +48,7 @@
return "semibold";
case 700:
case "700":
- return "semibold";
+ return "bold";
case 800:
case "800":
return "bold";
diff --git a/apps/mobile/src/native/T3ComposerEditor.ios.tsx b/apps/mobile/src/native/T3ComposerEditor.ios.tsx
--- a/apps/mobile/src/native/T3ComposerEditor.ios.tsx
+++ b/apps/mobile/src/native/T3ComposerEditor.ios.tsx
@@ -125,17 +125,31 @@
})),
);
}, [props.value, skillLabels]);
- const themeJson = JSON.stringify({
- text: String(textColor),
- placeholder: String(placeholderColor),
- chipBackground: String(chipBackground),
- chipBorder: String(chipBorder),
- chipText: String(chipText),
- skillBackground: String(skillBackground),
- skillBorder: String(skillBorder),
- skillText: String(skillText),
- fileTint: String(fileTint),
- });
+ const themeJson = useMemo(
+ () =>
+ JSON.stringify({
+ text: String(textColor),
+ placeholder: String(placeholderColor),
+ chipBackground: String(chipBackground),
+ chipBorder: String(chipBorder),
+ chipText: String(chipText),
+ skillBackground: String(skillBackground),
+ skillBorder: String(skillBorder),
+ skillText: String(skillText),
+ fileTint: String(fileTint),
+ }),
+ [
+ textColor,
+ placeholderColor,
+ chipBackground,
+ chipBorder,
+ chipText,
+ skillBackground,
+ skillBorder,
+ skillText,
+ fileTint,
+ ],
+ );
const resolvedTextStyle = StyleSheet.flatten(textStyle) ?? {};
return (
<NativeViewYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 86731fe. Configure here.
| if (savedRange.length > 0 && NSMaxRange(savedRange) <= _textView.attributedText.length) { | ||
| _textView.selectedRange = savedRange; | ||
| } | ||
| _suppressSelectionChange = NO; |
There was a problem hiding this comment.
Caret not restored after update
Medium Severity
When markdown content is reassigned in drawRect, selection is only restored if the saved range has non-zero length, so a collapsed caret (insertion point) is dropped after streaming or other text updates. refreshDisplayedAttachments already restores both caret and range selections.
Reviewed by Cursor Bugbot for commit 86731fe. Configure here.
| return "semibold"; | ||
| case 700: | ||
| case "700": | ||
| return "semibold"; |
There was a problem hiding this comment.
Bold weight maps to semibold
Low Severity
fontWeightToNativeProp maps both "700" and numeric 700 to "semibold" instead of "bold", so markdown runs styled with fontWeight: "700" (headings and bold in NativeMarkdownSelectableText) render lighter than intended on iOS native runs.
Reviewed by Cursor Bugbot for commit 86731fe. Configure here.
| theme = nextTheme | ||
| applyTheme() | ||
| applyControlledDocument(force: true) | ||
| } |
There was a problem hiding this comment.
Theme prop forces full rebuild
Medium Severity
setThemeJson always calls applyControlledDocument(force: true) without comparing to the current theme, and the iOS wrapper builds a new themeJson string every render. Any native prop update with unchanged colors still rebuilds the full attributed document and resets selection handling, which can interrupt typing when the composer’s parent re-renders.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 86731fe. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| // Even if the uiTextView prop is set, we can still default to using | ||
| // normal selection (i.e. base RN text) if the text doesn't need to be | ||
| // selectable | ||
| if ((!props.selectable || !props.uiTextView) && !isAncestor) { |
There was a problem hiding this comment.
🟡 Medium src/MarkdownTextPrimitive.tsx:98
When uiTextView={true} is passed without explicitly setting selectable, the component returns RNText instead of the UITextView-based native component. This happens because the check !props.selectable at line 98 evaluates to true when selectable is undefined, even though textDefaults sets selectable: true by default. The uiTextView prop is effectively ignored in this case, which contradicts the intended behavior of using the native UITextView when that prop is enabled.
- if ((!props.selectable || !props.uiTextView) && !isAncestor) {
+ const isSelectable = props.selectable ?? textDefaults.selectable;
+ if ((!isSelectable || !props.uiTextView) && !isAncestor) {🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-markdown-text/src/MarkdownTextPrimitive.tsx around line 98:
When `uiTextView={true}` is passed without explicitly setting `selectable`, the component returns `RNText` instead of the UITextView-based native component. This happens because the check `!props.selectable` at line 98 evaluates to `true` when `selectable` is `undefined`, even though `textDefaults` sets `selectable: true` by default. The `uiTextView` prop is effectively ignored in this case, which contradicts the intended behavior of using the native UITextView when that prop is enabled.
Evidence trail:
apps/mobile/modules/t3-markdown-text/src/MarkdownTextPrimitive.tsx lines 12-15 (textDefaults with selectable: true), line 98 (condition `(!props.selectable || !props.uiTextView) && !isAncestor`), lines 74-84 (textDefaults spread only inside MarkdownTextPrimitiveChild), lines 95-97 (comment explaining intended behavior). Commit: REVIEWED_COMMIT.
| import { useThemeColor } from "../lib/useThemeColor"; | ||
|
|
||
| function AppNavigator() { | ||
| const pathname = usePathname(); |
There was a problem hiding this comment.
🟢 Low app/_layout.tsx:34
When the user navigates to /settings/auth after the app has already mounted on a different route, clerkRouteIsActive becomes true, but ClerkSettingsSheetDetentProvider ignores the prop change because its isExpanded state is initialized only via useState(initiallyExpanded). The sheet renders with [0.7] detent instead of the intended [0.92] because isExpanded remains false. Consider synchronizing the expanded state with the route using useEffect inside the provider, or moving the route detection and state into a component that can call expand() when entering the auth route.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/app/_layout.tsx around line 34:
When the user navigates to `/settings/auth` after the app has already mounted on a different route, `clerkRouteIsActive` becomes `true`, but `ClerkSettingsSheetDetentProvider` ignores the prop change because its `isExpanded` state is initialized only via `useState(initiallyExpanded)`. The sheet renders with `[0.7]` detent instead of the intended `[0.92]` because `isExpanded` remains `false`. Consider synchronizing the expanded state with the route using `useEffect` inside the provider, or moving the route detection and state into a component that can call `expand()` when entering the auth route.
Evidence trail:
apps/mobile/src/features/cloud/ClerkSettingsSheetDetent.tsx line 26: `useState(initiallyExpanded)` — only used on mount. apps/mobile/src/app/_layout.tsx lines 34-41: `clerkRouteIsActive` passed as `initiallyExpanded`. apps/mobile/src/app/_layout.tsx line 81: `sheetAllowedDetents: isExpanded ? [0.92] : [0.7]`. apps/mobile/src/app/settings/auth.tsx: no call to `expand()`. apps/mobile/src/app/settings/waitlist.tsx line 48: only `waitlist` calls `expand()`, not `auth`.
| case 700: | ||
| case "700": | ||
| return "semibold"; |
There was a problem hiding this comment.
🟡 Medium src/util.ts:49
fontWeight: 700 and fontWeight: "700" are mapped to "semibold", but React Native treats 700 as equivalent to "bold". This creates an inconsistency where fontWeight: "bold" renders as bold while fontWeight: 700 renders as semibold. Consider mapping 700 and "700" to "bold" to match React Native's behavior.
- case 700:
- case "700":
- return "semibold";
+ case 700:
+ case "700":
+ return "bold";🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-markdown-text/src/util.ts around lines 49-51:
`fontWeight: 700` and `fontWeight: "700"` are mapped to `"semibold"`, but React Native treats `700` as equivalent to `"bold"`. This creates an inconsistency where `fontWeight: "bold"` renders as bold while `fontWeight: 700` renders as semibold. Consider mapping `700` and `"700"` to `"bold"` to match React Native's behavior.
Evidence trail:
apps/mobile/modules/t3-markdown-text/src/util.ts lines 20-61 (at REVIEWED_COMMIT): fontWeightToNativeProp function showing 700/"700" → "semibold" (line 49-51) while "bold" → "bold" (line 25). React Native Font Weight Cheatsheet (https://gist.github.com/knowbody/c5cdf26073b874eae86ba96e7cf3a540) confirms 700 = Bold. React Native docs (https://reactnative.dev/docs/text-style-props) list '700' and 'bold' as font weight values.
|
|
||
| const INLINE_HTML_TAG_PATTERN = /<\/?(?:kbd|mark|sub|sup|u)(?:\s[^>]*)?>/gi; | ||
|
|
||
| function decodeHtmlEntitiesOnce(value: string): string { |
There was a problem hiding this comment.
🟢 Low src/nativeMarkdownText.ts:73
decodeHtmlEntitiesOnce throws RangeError on input like � or � because String.fromCodePoint rejects values above 0x10FFFF. The regex accepts any digit/hex length without range validation, so malformed entities crash the parser. Consider adding bounds checks before calling fromCodePoint.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-markdown-text/src/nativeMarkdownText.ts around line 73:
`decodeHtmlEntitiesOnce` throws `RangeError` on input like `�` or `�` because `String.fromCodePoint` rejects values above `0x10FFFF`. The regex accepts any digit/hex length without range validation, so malformed entities crash the parser. Consider adding bounds checks before calling `fromCodePoint`.
Evidence trail:
apps/mobile/modules/t3-markdown-text/src/nativeMarkdownText.ts lines 73-82 at REVIEWED_COMMIT: regex `#(\d+)` and `#x([0-9a-f]+)` accept unbounded numeric values; `String.fromCodePoint()` called at lines 78 and 81 without range validation; ECMAScript spec: String.fromCodePoint throws RangeError for values > 0x10FFFF.
| } | ||
|
|
||
| const tokens = [...new Set([...builtInTokens, ...Object.keys(customIcons)])].sort(); | ||
| const generatedSource = `import type { ImageSourcePropType } from "react-native";\n\nexport const MARKDOWN_FILE_ICON_SOURCES = {\n${tokens |
There was a problem hiding this comment.
🟢 Low scripts/sync-pierre-file-icons.mjs:130
The template on line 131 generates unquoted object keys like ${token}: require(...). When builtInTokens contains values with hyphens (e.g., c-sharp from a regex match like [^"]+), the output becomes c-sharp: require(...) which TypeScript parses as c - sharp: require(...) — a syntax error. Consider wrapping the token in quotes: "${token}": require(...).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/modules/t3-markdown-text/scripts/sync-pierre-file-icons.mjs around line 130:
The template on line 131 generates unquoted object keys like `${token}: require(...)`. When `builtInTokens` contains values with hyphens (e.g., `c-sharp` from a regex match like `[^"]+`), the output becomes `c-sharp: require(...)` which TypeScript parses as `c - sharp: require(...)` — a syntax error. Consider wrapping the token in quotes: `"${token}": require(...)`.
Evidence trail:
apps/mobile/modules/t3-markdown-text/scripts/sync-pierre-file-icons.mjs lines 114-116 (regex `[^"+]` captures tokens), lines 130-132 (template uses unquoted `${token}:`), apps/mobile/modules/t3-markdown-text/src/markdownFileIcons.generated.ts (current output with all-simple-identifier keys). Dependency: `@pierre/trees@1.0.0-beta.4` in apps/mobile/package.json line 110.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
1 participant
Summary
Move the post-architecture mobile product work into a focused, independently reviewable PR.
vscode-iconsreferences outsideapps/web/THIRD_PARTY_NOTICES.mdStack
mainThe client connection/runtime rearchitecture is intentionally excluded and remains in #2978, stacked on top of this PR.
Validation
vp checkvp run typecheckvp run lint:mobilevp test --exclude apps/server/src/assets/AssetAccess.test.ts— 449 files and 3,554 tests passedA full
vp testrun only hits the existing macOS/varversus/private/varassertions inapps/server/src/assets/AssetAccess.test.ts.Note
High Risk
Large new native surface (Expo module + Fabric codegen, CocoaPods) on the critical composer and message-rendering path; iOS-only behavior increases platform divergence risk.
Overview
Adds two iOS-only native modules and wires them through mobile chat and settings.
t3-composer-editoris a UIKitUITextView–backed composer that renders skill and file paths as atomic attachment chips, keeps source text/selection in sync with JS, and handles image paste to temp files.@t3tools/mobile-markdown-textis a Fabric attributed-text stack (Bluesky-derived, heavily customized) withSelectableMarkdownText: GFM parsing, Shiki-highlighted code blocks, file/skill chips, Pierre file icons (sync:pierre-icons), and native selection.Theme tokens in
global.cssandclerk-theme.jsongain inline-skill and glass-surface colors; screens move off hard-coded colors touseThemeColor.Clerk: new
/settings/authroute withAuthView/UserProfileView,ClerkSettingsSheetDetentProviderso the settings sheet expands for sign-in, and account/waitlist flows navigate there instead of alerts or a modal.@clerk/expobumps to^3.4.1.Non-iOS markdown/composer paths stub or fall back; Android is excluded from the new pods/codegen.
Reviewed by Cursor Bugbot for commit 3a4a825. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add native iOS composer editor and selectable markdown text renderer to the mobile app
T3ComposerEditor) with inline token chips for skills and file mentions, image paste support, and theme-driven styling via a new Expo module inapps/mobile/modules/t3-composer-editor.T3MarkdownText) with Fabric-native attributed text runs, selectable text, syntax-highlighted code blocks, file icons, and external link favicons viaapps/mobile/modules/t3-markdown-text.TextInputwithComposerEditorinThreadComposerandNewTaskDraftScreen, wiring skill awareness, selection control, and image paste callbacks.collectComposerInlineTokensintopackages/sharedand updates the web composer to delegate to the shared implementation./settings/authroute for in-app Clerk sign-in/account UI, replacing the previous alert-based flow and controlled by a newClerkSettingsSheetDetentProvider.T3MarkdownTextandT3ComposerEditormodules are iOS-only; non-iOS platforms fall back to stub/plain-text implementations.Macroscope summarized 3a4a825.