mobile: render Markdown in the editable list-item row - #876
Merged
Conversation
Mobile's editable row showed source, so `**Milk**` read as **Milk** on the card and in the read-only editor but as `**Milk**` in the row you type in. It now swaps like the webapp's: rendered until it holds the caret, source for exactly as long as it does. The one question mobile had to answer for itself was the keyboard. Under "focused == editing", moving between rows unmounts one TextInput and mounts another, and React Native only keeps the software keyboard up when focus moves between two *mounted* inputs — so the naive swap would dismiss and reopen the keyboard on the most common interaction a list note has. The field is therefore never unmounted, only taken out of flow at opacity 0, which is what the webapp already does for its textarea. The rest carries over from #824 and is now written once in the spec: - Tap-to-edit places the caret where the user pointed. RN has no `caretPositionFromPoint`, so `mobile/src/utils/inlineCaret.ts` maps a tap to a rendered offset from the line boxes `onTextLayout` reports, then hands it to the shared `inlineSourceOffset`. The caret is forced through the controlled `selection` prop and released once the input reports it. - A row only swaps when rendering changes something, so a plain list keeps the always-live input it has always had. - An editable row's links are inert; a read-only row's stay live. - Both forms are the same height: one style object, with an explicit lineHeight and paddingLeft so a Text and a TextInput cannot disagree about the font's metrics or Android's EditText padding. - A drag freezes the row's form for its duration, so the lifted cell keeps the size the reorderable list measured. Closes #867
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization 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:
WalkthroughMobile editable list-item rows now display rendered Markdown when unfocused and source Markdown while editing. The Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
hanzei
marked this pull request as ready for review
August 10, 2026 06:24
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
mobile/src/components/ListItem.tsx (1)
330-414: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider labelling the hidden input with the rendered words.
While
showRenderedis true, the renderedTextis removed from the accessibility tree (Lines 405-406), and the still-mountedTextInputkeeps the raw source as its value. A screen-reader user therefore hearsbuy **milk**for a row that displaysbuy milk. This is the same mismatch the checkbox label avoids at Lines 308-309 withinlineMarkdownToText.The input must stay accessible because it is the row's real control, so an
accessibilityLabelderived from the rendered text keeps the announcement aligned with the screen while editing still operates on the source.♿ Possible adjustment
<TextInput ref={setInputRef} autoFocus={autoFocus} style={[styles.itemText, textTone]} value={text} + // While the rendered form is on top, announce the words the + // user sees rather than the source the field holds. + accessibilityLabel={showRendered ? inlineMarkdownToText(text) : undefined} selection={forcedSelection ?? undefined}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mobile/src/components/ListItem.tsx` around lines 330 - 414, Update the editable TextInput in the showRendered path to provide an accessibilityLabel derived from the rendered text, using the existing inlineMarkdownToText-style conversion used by the checkbox label. Keep the input accessible and preserve its raw source value and editing behavior while ensuring screen readers announce the displayed words.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/specs/markdown-rendering.md`:
- Around line 101-126: Update the §5 “Where it is implemented” table to include
rows for “Mobile editable-row swap (§1.2)” linking to
mobile/src/components/ListItem.tsx and “Mobile tap point → rendered offset”
linking to mobile/src/utils/inlineCaret.ts, while preserving the existing webapp
entries.
---
Nitpick comments:
In `@mobile/src/components/ListItem.tsx`:
- Around line 330-414: Update the editable TextInput in the showRendered path to
provide an accessibilityLabel derived from the rendered text, using the existing
inlineMarkdownToText-style conversion used by the checkbox label. Keep the input
accessible and preserve its raw source value and editing behavior while ensuring
screen readers announce the displayed words.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c4ea501-0ce4-4bd9-a436-59da89c37c6b
📒 Files selected for processing (6)
docs/specs/markdown-rendering.mdmobile/__tests__/ListItem.test.tsxmobile/__tests__/inlineCaret.test.tsmobile/src/components/ListItem.tsxmobile/src/utils/inlineCaret.tsmobile/src/utils/inlineMarkdown.ts
Adds the two files this change introduced to the "where it is implemented" table (§5), which listed the webapp's editable-row swap and click-to-caret map but not their mobile counterparts.
Tapping a rendered row moved focus to the note title instead of the row. While the rendered form is showing, the row's TextInput is out of flow, transparent and inside a `pointerEvents: 'none'` wrapper, and `focus()` was called on it in that state — iOS refuses `becomeFirstResponder` for a view with user interaction disabled, and Android's `requestFocus` falls through to the next focusable field in the window, which is the title. The tap now ends the rendered form and an effect keyed on that swap does the focusing, so the field is back in flow and interactive by the time it is asked for focus. Keying the effect on the swap rather than running it unconditionally also means a row frozen mid-drag waits for the drop. Jest never caught this because `focus()` is a no-op there; the regression test asserts the ordering instead — the rendered form must be gone, and the row's own field must be the one focused.
Not for merge — revert this commit before the PR leaves draft. Tapping a rendered row moves focus to the note title instead of the row, and it survived the fix in fba5213, so this logs the path rather than guessing at a third cause. `console.info` is what the app persists (src/utils/logger.ts), so the trace shows up in Settings -> Diagnostics with no debugger attached. Logged, in order: the tap (whether the text had been measured, the mapped offset, whether the row was rendered/dragging), the focus attempt (whether the ref holds a node), `isFocused()` immediately after and 300ms later, the row's own focus/blur, and the title input's focus. Lengths and offsets only, never item text — mobile/CLAUDE.md.
Not for merge — revert with the instrumentation commit before the PR leaves draft. The first trace showed focus landing on the tapped row (isFocused true, onFocus delivered) and being taken away 3ms later, with the title as the fallback rather than the thief. The remaining suspect is the row's own reflow: the rendered form measured 2 lines, and the source it swaps to is longer, so the cell changes height and the list's itemLayoutAnimation (Reanimated LinearTransition) runs on a cell containing the focused input. This logs the row height on every layout pass so the change is visible in the trace next to the blur.
Tapping a rendered row focused the right field and then lost it to the
note title — intermittently, which is what named the cause.
The row's input sits in a wrapper View that carries a style and
`pointerEvents: 'none'` while the rendered form is showing, and neither
once it goes back in flow. A View with no rendering-relevant props is a
flattening candidate on Android, so that commit can remove the wrapper's
native view and re-parent the EditText inside it — and an EditText that is
removed and re-added loses focus, which Android then hands to the first
focusable field in the window. That is the same fallback the reorderable
list's force-remount already works around (NoteEditorScreen, commitDrag).
It races the focus() that follows the same commit, which is why the same
tap worked one time in four. `collapsable={false}` keeps the wrapper a
real native view in both states, so there is nothing to re-parent.
Evidence, from the device traces on #867: the row logged no layout change
across the swap (ruling out a reflow / itemLayoutAnimation cause), focus
was requested successfully every time, and only the 300ms follow-up
disagreed — native dropping focus after JS had taken it.
Moving between two list items took two taps: the first blurred the focused row and dismissed the keyboard, the second reached the row that was tapped. A ScrollView captures the touch responder — blurring the focused input, dismissing the keyboard and swallowing the tap — when its keyboardShouldPersistTaps is left at the default and the tap lands on something that is not a TextInput (ScrollView.scrollResponderHandleStartShouldSetResponderCapture). The ScrollViewContainer wrapping the editor already passes "handled", but the item list is a FlatList underneath and brings a ScrollView of its own, which was still on the default. It went unnoticed because a row's tap target used to be its TextInput, which the check exempts. A rendered row's target is a Text, so every row-to-row move paid a tap for it. This is the ticket's headline acceptance criterion (#867): moving between rows must not flicker the keyboard.
hanzei
added a commit
that referenced
this pull request
Aug 18, 2026
…et (#932) #870 fixed focus jumping to the note title when a focused list item was dragged to a new slot: the reorderable list force-remounts any row whose slot changed, and the editor re-arms `autoFocus` on the previously focused item so the remounted row re-opens the keyboard on itself. #876 landed after it and gave the editable row a rendered form. A row mounts with `isEditing = false`, so any row whose text renders differently from its source — emphasis, code, a link, a bare URL — mounts *rendered*, with its input out of flow, transparent and inside a `pointerEvents: 'none'` wrapper. Neither platform focuses a field in that state (the same thing tap-to-edit already works around), so the armed `autoFocus` lands on Android's fallback: the note title. The pre-#870 symptom was back for every row with any Markdown in it. Seed `isEditing` from `autoFocus`, so a row that mounts asking for the caret mounts in source form and can actually take it. This covers the new item `handleAddItem` adds as well as the drag restore. Claude-Session: https://claude.ai/code/session_01XGq4L5KmepDPud9UNf6Qtq Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #867.
Mobile's editable row showed source, so
**Milk**read as Milk on the card and in the read-only editor but as**Milk**in the row you actually type in. It now swaps like the webapp's (#868): rendered until it holds the caret, source for exactly as long as it does.The keyboard question (#824 q8), which is why mobile was split out
Under focused == editing, tapping row A then row B unmounts
TextInputA and mountsTextInputB — with an unmount in between. React Native keeps the software keyboard up when focus moves directly between two mounted inputs and generally does not across an unmount, so the naive swap dismisses and reopens the keyboard, and jumps the scroll position with it, on the most common interaction a list note has.Of the three candidates in the ticket, this takes #2, never unmount, which is what the webapp already does for its textarea:
TextInputis always mounted. When the row shows its rendered form the input is moved out of flow (position: absolute,opacity: 0,pointerEvents: 'none') and the renderedTexttakes its place in the column, so the row is as tall as whichever form is showing.Because the field is never unmounted, everything in
NoteEditorScreenthat reaches for a row imperatively — Enter-to-split, backspace-merge, "add item" focus, all throughitemInputRefs— keeps working on a row that happens to be showing rendered text, unchanged.Two Android focus bugs found on device
Both were invisible to Jest, which has no native view tree and mocks
focus()to a no-op. Tapping a rendered row focused the note title instead of the row. Two independent causes, found by instrumenting the path and reading the traces rather than by guessing:fba5213). The tap calledfocus()before the swap committed, so the input was still transparent and insidepointerEvents: 'none'. iOS refusesbecomeFirstResponderfor a view with user interaction disabled; Android'srequestFocusfalls through to the next focusable field in the window. The tap now ends the rendered form and an effect keyed on that swap does the focusing.6ed0ffd). The input's wrapper carries a style andpointerEvents: 'none'while rendered, and neither once back in flow — making it a view-flattening candidate on exactly the commit the focus follows. Flattening re-parents theEditText, and anEditTextremoved and re-added loses focus, which Android hands to the first focusable field. The two raced, which is why the same tap worked roughly one time in four.collapsable={false}pins the wrapper as a real native view in both states.The device traces also proved the row does not change height across the swap, ruling out the reorderable list's
itemLayoutAnimationas a cause.The rest, carried over from #824
caretPositionFromPoint, so the newmobile/src/utils/inlineCaret.tsreconstructs the rendered offset from the line boxesonTextLayoutreports and hands it to the sharedinlineSourceOffset— no second offset map. The caret is forced through the controlledselectionprop and released once the input reports it landed. The interpolation across a line is exact in a monospaced face and a character or two out inside a long proportional line.inlineRendersAsSource), so a list with no Markdown in it keeps the always-live input it has always had.~~strike~~collision included.lineHeightandpaddingLeftso aTextand aTextInputcannot disagree about the font's metrics or the theme'sEditTextpadding on Android.react-native-reorderable-listreports the cell active.inlineMarkdownNodesnow lexes with source tracking on (like the webapp'srenderInlineItem), which is what gives the spans the tap mapping walks.Spec
docs/specs/markdown-rendering.md§1 and §1.2 said the editable-row swap was webapp-only and pointed at #867; §1.2 is now written once for both clients, with the two places they genuinely differ called out inline. The §6 "deliberately not covered" entry is gone, §5 lists the two new mobile files, and §7 records where the swap and the caret mapping are tested.Tests
mobile/__tests__/ListItem.test.tsx— the state machine: which form is showing, that the input survives a swap, that a plain row never swaps, that an editable row's links are inert while a read-only row's are not, that a drag freezes the form, and the tap-to-edit cases including that the swap is committed before focus is requested (the regression test for bug 1 above).mobile/__tests__/inlineCaret.test.ts— the pure half of the mapping, against synthetic line boxes.task checkpasses: 1362 mobile tests, 111 suites, plus the docs, migration and translation gates.Verified, and not
lineStarts. Cosmetic next to the focus bug; not chased yet.No API changes, so nothing breaking and no migration.