commit 1150a144bc3c680991f19a5ae18d1f9793d6a226
parent 89acde1b90a8ec52f3475e4414619ce867cba0d3
Author: Daniel D’Aquino <daniel@daquino.me>
Date: Mon, 2 Oct 2023 20:24:12 +0000
composer: Make text editor more robust
This change makes the text editor/composer more robust and simple and solves Github issue #1558
It builds on the changes made for #1211, and on Jericho's (Jericho Hasselbush <jericho@sal-et-lucem.com>) discovery during his work on #1544
It uses setContentCompressionResistance, disabled text box scrolling, and dynamic height adjustments (based on more accurate layout calculations) to allow several improvements:
- It ensures lines get wrapped and not overflown
- It uses system native "scroll cursor into view" when typing, eliminating the need to a ghost caret
- It ensures we do not have a scroll view within a scroll view (which is confusing)
- It ensures that we set the height of the text box to its ideal value using a native layout calculation (Removes some issues with copying and pasting larger text)
- It resolves other small issues, such as #1558
Issue #1558 repro
-----------------
Result: VERIFIED
Device: iPhone 14 Pro Simulator
iOS: 17.0
Damus: `476f52562a70c2615ad084640dd1a0ba5c4c12e3`
Issue #1558 steps:
1. Type "hello world, hello @da"
2. Select "Damus" in contact list
3. Try moving cursor to the end of "world". Cursor should have gone there, but it immediately goes back to the end of "@damus " instead.
Testing for #1558
-----------------
Result: PASS
Device: iPhone 14 Pro Simulator
iOS: 17.0
Damus: This commit
Steps:
1. Type "hello world, hello @da"
2. Select "Damus" in contact list
3. Try moving cursor to the end of "world". Cursor goes there.
General functionality testing
-----------------------------
Result: CONDITIONAL PASS.
Summary: Behaviour is improved from #1211 patch, and #1558 is fixed. There are a few remaining issues, but they do not look like regressions from these changes. More details below.
Device: iPhone 14 Pro Simulator
iOS: 17.0
Damus: This commit
Coverage:
1. Basic typing works. PASS
2. Basic user tagging works. PASS
3. Typing long text line wraps the line. PASS
4. Adding newlines to the end of the text works and text is visible (i.e. Text box is expanding with text). PASS
5. Adding lots of newlines causes the text box and inner PostView content to expand, and those contents can be scrolled. PASS
6. Typing text when cursor is out of view (both up and down) causes PostView to scroll the cursor into view. PASS
7. Tagging user on a line positioned at the middle of the screen causes view to scroll cursor into view. PASS
8. Tagging user on a very long line positioned causes view to scroll cursor into view. PASS
9. Pasting very long text (5 paragraphs of Lorem Ipsum) expands the text box as necessary, wraps all long lines, scrolls cursor at the end into view. PASS
10. Scrolling through very long text shows that there is only one scroll view active (PostView's). PASS
11. Typing text that expands text box does not cause jitters. PASS
12. Typing mentions do not cause jitter. PASS
13. Adding newline from the end of a mid paragraph unfortunately still causes cursor to jump to the end of the text. This is an existing bug (https://github.com/damus-io/damus/issues/1521). EXISTING ISSUE.
14. Tagging a user at the end of a line when there are other lines below it may cause the cursor to jump a few characters forward. It is unclear whether this is a regression because prior to this change the cursor would get stuck at the end of the mention. But since this is a very specific edge case that might not be a regression, it might be a good idea to address this on a separate ticket. CONDITIONAL PASS
15. Could not run PostView unit tests due to various build errors on the test target.
Closes: https://github.com/damus-io/damus/issues/1558
Changelog-Fixed: Fix situations where the note composer cursor gets stuck in one place after tagging a user
Changelog-Fixed: Fix some note composer issues, such as when copying/pasting larger text, and make the post composer more robust.
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
Signed-off-by: William Casarin <jb55@jb55.com>
Diffstat:
2 files changed, 24 insertions(+), 76 deletions(-)
diff --git a/damus/Views/PostView.swift b/damus/Views/PostView.swift
@@ -56,7 +56,6 @@ struct PostView: View {
@State var filtered_pubkeys: Set<Pubkey> = []
@State var focusWordAttributes: (String?, NSRange?) = (nil, nil)
@State var newCursorIndex: Int?
- @State var caretRect: CGRect = CGRectNull
@State var textHeight: CGFloat? = nil
@State var mediaToUpload: MediaUpload? = nil
@@ -220,13 +219,6 @@ struct PostView: View {
self.newCursorIndex = nil
}, updateCursorPosition: { newCursorIndex in
self.newCursorIndex = newCursorIndex
- }, onCaretRectChange: { uiView in
- // When the caret position changes, we change the `caretRect` in our state, so that our ghost caret will follow our caret
- if let selectedStartRange = uiView.selectedTextRange?.start {
- DispatchQueue.main.async {
- caretRect = uiView.caretRect(for: selectedStartRange)
- }
- }
})
.environmentObject(tagModel)
.focused($focus)
@@ -316,9 +308,6 @@ struct PostView: View {
func Editor(deviceSize: GeometryProxy) -> some View {
HStack(alignment: .top, spacing: 0) {
- if(caretRect != CGRectNull) {
- GhostCaret
- }
VStack(alignment: .leading, spacing: 0) {
HStack(alignment: .top) {
ProfilePicView(pubkey: damus_state.pubkey, size: PFP_SIZE, highlight: .none, profiles: damus_state.profiles, disable_animation: damus_state.settings.disable_animation)
@@ -340,25 +329,6 @@ struct PostView: View {
}
}
- // The GhostCaret is a vertical projection of the editor's caret that should sit beside the editor.
- // The purpose of this view is create a reference point that we can scroll our ScrollView into
- // This is necessary as a bridge to communicate between:
- // - The UIKit-based UITextView (which has the caret position)
- // - and the SwiftUI-based ScrollView/ScrollReader (where scrolling commands can only be done via the SwiftUI "ID" parameter
- var GhostCaret: some View {
- Rectangle()
- .foregroundStyle(DEBUG_SHOW_GHOST_CARET_VIEW ? .cyan : .init(red: 0, green: 0, blue: 0, opacity: 0))
- .frame(
- width: DEBUG_SHOW_GHOST_CARET_VIEW ? caretRect.width : 0,
- height: caretRect.height)
- // Use padding to vertically align our ghost caret with our actual text caret.
- // Note: Programmatic scrolling cannot be done with the `.position` modifier.
- // Experiments revealed that the scroller ignores the position modifier.
- .padding(.top, caretRect.origin.y)
- .id(GHOST_CARET_VIEW_ID)
- .disabled(true)
- }
-
func fill_target_content(target: PostTarget) {
self.post = initialString()
self.tagModel.diff = post.string.count
@@ -396,13 +366,6 @@ struct PostView: View {
.onAppear {
scroll_to_event(scroller: scroller, id: "post", delay: 1.0, animate: true, anchor: .top)
}
- // Note: The scroll commands below are specific because there seems to be quirk with ScrollReader where sending it to the exact same position twice resets its scroll position.
- .onChange(of: caretRect.origin.y, perform: { newValue in
- scroller.scrollTo(GHOST_CARET_VIEW_ID)
- })
- .onChange(of: searchingIsNil, perform: { newValue in
- scroller.scrollTo(GHOST_CARET_VIEW_ID)
- })
}
// This if-block observes @ for tagging
diff --git a/damus/Views/TextViewWrapper.swift b/damus/Views/TextViewWrapper.swift
@@ -7,10 +7,6 @@
import SwiftUI
-// Defines how much extra bottom spacing will be applied after the text.
-// This will avoid jitters when applying new lines, by ensuring it has enough space until the height is updated on the next view update cycle
-let TEXT_BOX_BOTTOM_MARGIN_OFFSET: CGFloat = 30.0
-
struct TextViewWrapper: UIViewRepresentable {
@Binding var attributedText: NSMutableAttributedString
@EnvironmentObject var tagModel: TagModel
@@ -19,19 +15,16 @@ struct TextViewWrapper: UIViewRepresentable {
let cursorIndex: Int?
var getFocusWordForMention: ((String?, NSRange?) -> Void)? = nil
let updateCursorPosition: ((Int) -> Void)
- let onCaretRectChange: ((UITextView) -> Void)
func makeUIView(context: Context) -> UITextView {
let textView = UITextView()
textView.delegate = context.coordinator
- // Scroll has to be enabled. When this is disabled, the text input will overflow horizontally, even when its frame's width is limited.
- textView.isScrollEnabled = true
- // However, a scrolling text box inside of its parent scrollview does not provide a very good experience. We should have the textbox expand vertically
- // To simulate that the text box can expand vertically, we will listen to text changes and dynamically change the text box height in response.
- // Add an observer so that we can adapt the height of the text input whenever the text changes.
- textView.addObserver(context.coordinator, forKeyPath: "contentSize", options: .new, context: nil)
- textView.showsVerticalScrollIndicator = false
+ // Disable scrolling (this view will expand vertically as needed to fit text)
+ textView.isScrollEnabled = false
+ // Set low content compression resistance to make this view wrap lines of text, and avoid text overflowing to the right
+ textView.setContentCompressionResistancePriority(.defaultLow, for: .horizontal)
+ textView.setContentCompressionResistancePriority(.required, for: .vertical)
TextViewWrapper.setTextProperties(textView)
return textView
@@ -52,9 +45,26 @@ struct TextViewWrapper: UIViewRepresentable {
setCursorPosition(textView: uiView)
let range = uiView.selectedRange
+ // Set the text height that will fit all the text
+ // This is needed because the UIKit auto-layout prefers to overflow the text to the right than to expand the text box vertically, even with low horizontal compression resistance
+ self.setIdealHeight(uiView: uiView)
+
uiView.selectedRange = NSRange(location: range.location + tagModel.diff, length: range.length)
tagModel.diff = 0
}
+
+ /// Based on our desired layout, calculate the ideal size of the text box, then set the height to the ideal size
+ private func setIdealHeight(uiView: UITextView) {
+ DispatchQueue.main.async { // Queue on main thread, because modifying view state directly during re-render causes undefined behavior
+ let idealSize = uiView.sizeThatFits(CGSize(
+ width: uiView.frame.width, // We want to stay within the horizontal bounds given to us
+ height: .infinity // We can expand vertically without any resistance
+ ))
+ if self.textHeight != idealSize.height { // Only update height when it changes, to avoid infinite re-render calls
+ self.textHeight = idealSize.height
+ }
+ }
+ }
private func setCursorPosition(textView: UITextView) {
guard let index = cursorIndex, let newPosition = textView.position(from: textView.beginningOfDocument, offset: index) else {
@@ -64,38 +74,27 @@ struct TextViewWrapper: UIViewRepresentable {
}
func makeCoordinator() -> Coordinator {
- Coordinator(attributedText: $attributedText, getFocusWordForMention: getFocusWordForMention, updateCursorPosition: updateCursorPosition, onCaretRectChange: onCaretRectChange, textHeight: $textHeight)
+ Coordinator(attributedText: $attributedText, getFocusWordForMention: getFocusWordForMention, updateCursorPosition: updateCursorPosition)
}
class Coordinator: NSObject, UITextViewDelegate {
@Binding var attributedText: NSMutableAttributedString
var getFocusWordForMention: ((String?, NSRange?) -> Void)? = nil
let updateCursorPosition: ((Int) -> Void)
- let onCaretRectChange: ((UITextView) -> Void)
- @Binding var textHeight: CGFloat?
init(attributedText: Binding<NSMutableAttributedString>,
getFocusWordForMention: ((String?, NSRange?) -> Void)?,
- updateCursorPosition: @escaping ((Int) -> Void),
- onCaretRectChange: @escaping ((UITextView) -> Void),
- textHeight: Binding<CGFloat?>
+ updateCursorPosition: @escaping ((Int) -> Void)
) {
_attributedText = attributedText
self.getFocusWordForMention = getFocusWordForMention
self.updateCursorPosition = updateCursorPosition
- self.onCaretRectChange = onCaretRectChange
- _textHeight = textHeight
}
func textViewDidChange(_ textView: UITextView) {
attributedText = NSMutableAttributedString(attributedString: textView.attributedText)
processFocusedWordForMention(textView: textView)
}
-
- func textViewDidChangeSelection(_ textView: UITextView) {
- textView.scrollRangeToVisible(textView.selectedRange)
- onCaretRectChange(textView)
- }
private func processFocusedWordForMention(textView: UITextView) {
var val: (String?, NSRange?) = (nil, nil)
@@ -183,20 +182,6 @@ struct TextViewWrapper: UIViewRepresentable {
}
}
- override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
- if keyPath == "contentSize", let textView = object as? UITextView {
- DispatchQueue.main.async {
- // Update text view height when text content size changes to fit all text content
- // This is necessary to avoid having a scrolling text box combined with its parent scrolling view
- self.updateTextViewHeight(textView: textView)
- }
- }
- }
-
- func updateTextViewHeight(textView: UITextView) {
- self.textHeight = textView.contentSize.height + TEXT_BOX_BOTTOM_MARGIN_OFFSET
- }
-
}
}
