input: fix caret jumping to end of text on iOS keyboard switch - #3799
input: fix caret jumping to end of text on iOS keyboard switch#3799koteitan wants to merge 2 commits into
Conversation
The post composer tracks an explicit cursor position (newCursorIndex) that takes priority over the saved selection when the view updates. It was updated on every text change but never cleared or updated when the user moved the caret without editing, so it went stale, still pointing at the position of the last text edit (typically the end of the text). Pressing the iOS keyboard-switch (globe) button changes the keyboard height, which triggers a SwiftUI re-render, and updateUIView re-applied the stale index, jumping the caret to the end of the text. Track user-driven caret moves in textViewDidChangeSelection so the tracked position never goes stale, and suppress tracking while updateUIView applies programmatic text/selection changes, so their delegate callbacks are not recorded as user-driven moves. Changelog-Fixed: Fixed cursor jumping to the end of the text when pressing the iOS keyboard-switching button in the post composer Closes: damus-io#3545 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: koteitan <ukaibutton@mac.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesText Selection Tracking
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
damus/Shared/Components/Text/TextViewWrapper.swift (1)
53-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
///documentation for the selection-tracking contract.The new coordinator property and delegate method, plus the modified
updateUIViewbehavior, are documented only with inline//comments. Add declaration-level Swift doc comments describing when programmatic callbacks are suppressed and when caret positions are tracked.As per coding guidelines, Swift code must ensure docstring coverage for any code added or modified.
Also applies to: 116-118, 160-168
🤖 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 `@damus/Shared/Components/Text/TextViewWrapper.swift` around lines 53 - 56, The selection-tracking contract lacks declaration-level Swift documentation. Add /// doc comments to the coordinator’s programmatic-change state property, the relevant delegate method, and the modified updateUIView behavior, documenting that programmatic callbacks are suppressed while only user-driven caret moves update the tracked position; retain the existing inline comments and behavior.Source: Coding guidelines
damusTests/PostViewTests.swift (1)
322-356: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise
updateUIViewin the suppression test.This test manually sets
isApplyingProgrammaticChange, so it verifies only the delegate guard. It would still pass ifupdateUIViewstopped setting or resetting that flag. Add coverage that invokesupdateUIViewand confirms programmatic selection callbacks are suppressed and the flag is restored afterward.🤖 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 `@damusTests/PostViewTests.swift` around lines 322 - 356, Extend testSelectionChangeUpdatesTrackedCursorPosition to invoke updateUIView while applying a programmatic selection change, using the existing coordinator, textView, and binding setup. Verify the selection callback does not update trackedCursorPosition during updateUIView, then assert isApplyingProgrammaticChange is restored to false afterward.
🤖 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.
Nitpick comments:
In `@damus/Shared/Components/Text/TextViewWrapper.swift`:
- Around line 53-56: The selection-tracking contract lacks declaration-level
Swift documentation. Add /// doc comments to the coordinator’s
programmatic-change state property, the relevant delegate method, and the
modified updateUIView behavior, documenting that programmatic callbacks are
suppressed while only user-driven caret moves update the tracked position;
retain the existing inline comments and behavior.
In `@damusTests/PostViewTests.swift`:
- Around line 322-356: Extend testSelectionChangeUpdatesTrackedCursorPosition to
invoke updateUIView while applying a programmatic selection change, using the
existing coordinator, textView, and binding setup. Verify the selection callback
does not update trackedCursorPosition during updateUIView, then assert
isApplyingProgrammaticChange is restored to false afterward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ca9d4d4b-f4fe-4403-ac53-a4e1822ac7eb
📒 Files selected for processing (2)
damus/Shared/Components/Text/TextViewWrapper.swiftdamusTests/PostViewTests.swift
Convert the inline comments on the selection tracking added by the previous commit into declaration-level doc comments, per the docstring coverage guideline. Comment-only change; no code change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: koteitan <ukaibutton@mac.com>
|
This has been an annoyance for a while now. it would be great seeing it fixed. |
2 participants
Summary
Fixes the post composer caret jumping to the end of the text when the iOS keyboard-switching (globe) button is pressed while the caret is placed mid-text (#3545).
Root cause:
PostViewkeeps an explicitly tracked cursor position (newCursorIndex) whichTextViewWrapper.updateUIViewapplies with priority over the saved selection (a mechanism introduced in 4941b50 to fix #3461). The tracked position is updated on every text change, but never when the user moves the caret without editing (e.g. by tapping), and it is never cleared after being applied. After typing, it goes stale, still pointing at the position of the last edit — typically the end of the text. Switching keyboards changes the keyboard height, which triggers a SwiftUI re-render, andupdateUIViewre-applies the stale index, jumping the caret to the end.Fix: implement
textViewDidChangeSelectionin the coordinator so user-driven caret moves keep the tracked position in sync, and suppress that tracking whileupdateUIViewapplies programmatic text/selection changes — their delegate callbacks must not be recorded as user moves, and view state must not be mutated during a SwiftUI view update. Range selections (length > 0) are left untracked since a single index cannot represent them; behavior there is unchanged.Checklist
Standard PR Checklist
Changelog-Fixed)Closes:orFixes:tags in the commit messages wherever applicableTest report
Device: iPhone 11 Pro
iOS: 26.5.2
Damus (before / bug reproduction): official TestFlight build 1.18 (1332), commit 85e38ad4
Damus (after / fix verification): Debug build of this change sideloaded via AltStore, shown as version 1.18 (6) — built from the fork test branch (koteitan/damus @ 31be5c4 = this PR's change plus sideload-packaging-only commits)
Setup: English (QWERTY) and Japanese (Romaji QWERTY) keyboards enabled; logged in with an existing account.
Test 1 — main effect (with video)
Video comparing both builds: https://v.nostr.build/sdiqpZlYTsx9Cfup.mp4
Steps:
Results:
Test 2 — range selections are not tracked (the
selectedRange.length == 0guard)Steps: double-tap a word to select it, then drag the selection handles to grow and shrink the selection.
Results: the selection stays stable while dragging and the edit menu works normally. (If range selections were tracked, each handle drag would mutate the tracked state and the resulting re-render would collapse the in-progress selection.)
Test 3 — programmatic changes are not tracked (the
isApplyingProgrammaticChangeguard)Steps: with the caret mid-text, type
@plus a few letters, insert a mention from the autocomplete list, then press the globe button.Results: after insertion the caret sits right after the mention, and it stays there across the keyboard switch. (When
updateUIViewre-assigns the attributed text, UIKit transiently resets the caret to the end and firestextViewDidChangeSelection; if that callback were recorded, the next keyboard switch would jump the caret to the end.)Overall:
Unit test added:
PostViewTests.testSelectionChangeUpdatesTrackedCursorPosition, covering the same three paths at the unit level: caret-move tracking, exclusion of range selections, and suppression of programmatic selection changes.Other notes
🤖 Generated with Claude Code
https://claude.ai/code/session_0118vy9nfQ3SYqh8ARtMRRyb
Summary by CodeRabbit