Improves timing when notifying input delegate of selection changes (Thanks Alexander Blach (@blach)!) (#129)

* Renames willBeginEditingFromNonEditableTextInteraction to isPerformingNonEditableTextInteraction

* Removes sendSelectionChangedToTextSelectionView

* Notifies of selection changes in layoutSubviews()

* Disables "Based on dependency analysis"

* Disables SwiftLint warning

Author: simonbs

Sources/Runestone/TextView/TextInput/TextInputView.swift

@@ -31,11 +31,12 @@ final class TextInputView: UIView, UITextInput {
             }
         }
         set {
-            // We should not use this setter. It's intended for UIKit to use. It'll invoke the setter in various scenarios, for example when navigating the text using the keyboard.
+            // We should not use this setter. It's intended for UIKit to use.
+            // It'll invoke the setter in various scenarios, for example when navigating the text using the keyboard.
             let newRange = (newValue as? IndexedRange)?.range
             if newRange != _selectedRange {
+                shouldNotifyInputDelegateAboutSelectionChangeInLayoutSubviews = true
                 _selectedRange = newRange
-                inputDelegate?.selectionDidChange(self)
                 delegate?.textInputViewDidChangeSelection(self)
             }
         }
@@ -257,13 +258,11 @@ final class TextInputView: UIView, UITextInput {
     var indentStrategy: IndentStrategy = .tab(length: 2) {
         didSet {
             if indentStrategy != oldValue {
-                inputDelegate?.selectionWillChange(self)
                 indentController.indentStrategy = indentStrategy
                 layoutManager.tabWidth = indentController.tabWidth
                 layoutManager.setNeedsLayout()
                 setNeedsLayout()
                 layoutIfNeeded()
-                inputDelegate?.selectionDidChange(self)
             }
         }
     }
@@ -313,7 +312,6 @@ final class TextInputView: UIView, UITextInput {
                 layoutManager.invalidateLines()
                 layoutManager.setNeedsLayout()
                 layoutManager.layoutIfNeeded()
-                sendSelectionChangedToTextSelectionView()
             }
         }
     }
@@ -327,7 +325,6 @@ final class TextInputView: UIView, UITextInput {
                 layoutManager.invalidateLines()
                 layoutManager.setNeedsLayout()
                 layoutManager.layoutIfNeeded()
-                sendSelectionChangedToTextSelectionView()
             }
         }
     }
@@ -454,9 +451,7 @@ final class TextInputView: UIView, UITextInput {
         }
         set {
             if newValue != _selectedRange {
-                inputDelegate?.selectionWillChange(self)
                 _selectedRange = newValue
-                inputDelegate?.selectionDidChange(self)
                 delegate?.textInputViewDidChangeSelection(self)
             }
         }
@@ -546,6 +541,8 @@ final class TextInputView: UIView, UITextInput {
     }
     private var hasPendingFullLayout = false
     private let editMenuController = EditMenuController()
+    // swiftlint:disable:next identifier_name
+    private var shouldNotifyInputDelegateAboutSelectionChangeInLayoutSubviews = false
 
     // MARK: - Lifecycle
     init(theme: Theme) {
@@ -606,6 +603,13 @@ final class TextInputView: UIView, UITextInput {
         layoutManager.layoutIfNeeded()
         layoutManager.layoutLineSelectionIfNeeded()
         layoutPageGuideIfNeeded()
+        // We notify the input delegate about selection changes in layoutSubviews so we have a chance of disabling notifying the input delegate during an editing operation.
+        // We will sometimes disable notifying the input delegate when the user enters Korean text.
+        // This workaround is inspired by a dialog with Alexander Blach (@lextar), developer of Textastic.
+        if shouldNotifyInputDelegateAboutSelectionChangeInLayoutSubviews {
+            inputDelegate?.selectionWillChange(self)
+            inputDelegate?.selectionDidChange(self)
+        }
     }
 
     override func copy(_ sender: Any?) {
@@ -617,18 +621,14 @@ final class TextInputView: UIView, UITextInput {
     override func paste(_ sender: Any?) {
         if let selectedTextRange = selectedTextRange, let string = UIPasteboard.general.string {
             let preparedText = prepareTextForInsertion(string)
-            inputDelegate?.selectionWillChange(self)
             replace(selectedTextRange, withText: preparedText)
-            inputDelegate?.selectionDidChange(self)
         }
     }
 
     override func cut(_ sender: Any?) {
         if let selectedTextRange = selectedTextRange, let text = text(in: selectedTextRange) {
             UIPasteboard.general.string = text
-            inputDelegate?.selectionWillChange(self)
             replace(selectedTextRange, withText: "")
-            inputDelegate?.selectionDidChange(self)
         }
     }
 
@@ -872,11 +872,9 @@ private extension TextInputView {
     }
 
     private func performFullLayout() {
-        inputDelegate?.selectionWillChange(self)
         layoutManager.invalidateLines()
         layoutManager.setNeedsLayout()
         layoutManager.layoutIfNeeded()
-        inputDelegate?.selectionDidChange(self)
     }
 }
 
@@ -981,6 +979,8 @@ extension TextInputView {
         guard shouldChangeText(in: deleteRange, replacementText: "") else {
             return
         }
+        // Disable notifying delegate in layout subviews to prevent issues when entering Korean text. This workaround is inspired by a dialog with Alexander Black (@lextar), developer of Textastic.
+        shouldNotifyInputDelegateAboutSelectionChangeInLayoutSubviews = false
         // Just before calling deleteBackward(), UIKit will set the selected range to a range of length 1, if the selected range has a length of 0.
         // In that case we want to undo to a selected range of length 0, so we construct our range here and pass it all the way to the undo operation.
         let selectedRangeAfterUndo: NSRange
@@ -994,8 +994,10 @@ extension TextInputView {
             timedUndoManager.endUndoGrouping()
             timedUndoManager.beginUndoGrouping()
         }
+        // We've set shouldNotifyInputDelegateAboutSelectionChange to false so we must notify the input delegate ourselves.
+        inputDelegate?.selectionWillChange(self)
         replaceText(in: deleteRange, with: "", selectedRangeAfterUndo: selectedRangeAfterUndo)
-        sendSelectionChangedToTextSelectionView()
+        inputDelegate?.selectionDidChange(self)
         if isDeletingMultipleCharacters {
             timedUndoManager.endUndoGrouping()
         }
@@ -1139,8 +1141,8 @@ extension TextInputView {
         timedUndoManager.registerUndo(withTarget: self) { textInputView in
             textInputView.inputDelegate?.selectionWillChange(textInputView)
             textInputView.replaceText(in: range, with: text)
-            textInputView.inputDelegate?.selectionDidChange(textInputView)
             textInputView.selectedRange = oldSelectedRange
+            textInputView.inputDelegate?.selectionDidChange(textInputView)
         }
     }
 
@@ -1171,26 +1173,6 @@ extension TextInputView {
         return NSRange(location: cappedLocation, length: cappedLength)
     }
 
-    func sendSelectionChangedToTextSelectionView() {
-        // Fores the position of the caret to be updated. Normally we can do this by notifying the input delegate when changing the selected range like:
-        //
-        // inputDelegate?.selectionWillChange(self)
-        // selectedRange = newSelectedRange
-        // inputDelegate?.selectionDidChange(self)
-        //
-        // If we don't notify the input delegate when the setter on selectedTextRange is called, then the location of the caret will not be updated.
-        // However, if we do notify the delegate when the setter is called, Korean input will no longer work as described in https://github.com/simonbs/Runestone/issues/11
-        // So the workaround is to not notify the delegate but tell the text selection view directly that the selection has changed.
-        if let textSelectionView = textSelectionView {
-            let sel = NSSelectorFromString("selectionChanged")
-            if textSelectionView.responds(to: sel) {
-                textSelectionView.perform(sel)
-            } else {
-                print("\(textSelectionView) does not respond to 'selectionChanged'")
-            }
-        }
-    }
-
     private func moveCaret(to linePosition: LinePosition) {
         // By restoring the selected range using the old line position we can better preserve the old selected language.
         let line = lineManager.line(atRow: linePosition.row)
@@ -1294,14 +1276,14 @@ extension TextInputView {
             return
         }
         markedRange = markedText.isEmpty ? nil : NSRange(location: range.location, length: markedText.utf16.count)
-        inputDelegate?.selectionWillChange(self)
         replaceText(in: range, with: markedText)
-        inputDelegate?.selectionDidChange(self)
         delegate?.textInputViewDidUpdateMarkedRange(self)
     }
 
     func unmarkText() {
+        inputDelegate?.selectionWillChange(self)
         markedRange = nil
+        inputDelegate?.selectionDidChange(self)
         delegate?.textInputViewDidUpdateMarkedRange(self)
     }
 }
@@ -1501,9 +1483,7 @@ extension TextInputView: LayoutManagerDelegate {
 // MARK: - IndentControllerDelegate
 extension TextInputView: IndentControllerDelegate {
     func indentController(_ controller: IndentController, shouldInsert text: String, in range: NSRange) {
-        inputDelegate?.selectionWillChange(self)
         replaceText(in: range, with: text)
-        inputDelegate?.selectionDidChange(self)
     }
 
     func indentController(_ controller: IndentController, shouldSelect range: NSRange) {

Sources/Runestone/TextView/TextView.swift

@@ -584,7 +584,7 @@ open class TextView: UIScrollView {
     private let tapGestureRecognizer = QuickTapGestureRecognizer()
     private var _inputAccessoryView: UIView?
     private let _inputAssistantItem = UITextInputAssistantItem()
-    private var willBeginEditingFromNonEditableTextInteraction = false
+    private var isPerformingNonEditableTextInteraction = false
     private var delegateAllowsEditingToBegin: Bool {
         guard isEditable else {
             return false
@@ -671,8 +671,6 @@ open class TextView: UIScrollView {
     @discardableResult
     override open func becomeFirstResponder() -> Bool {
         if !isEditing && delegateAllowsEditingToBegin {
-            // Reset willBeginEditingFromNonEditableTextInteraction to support calling becomeFirstResponder() programmatically.
-            willBeginEditingFromNonEditableTextInteraction = false
             _ = textInputView.resignFirstResponder()
             _ = textInputView.becomeFirstResponder()
             return true
@@ -735,30 +733,30 @@ open class TextView: UIScrollView {
     /// Inserts text at the location of the caret or, if no selection or caret is present, at the end of the text.
     /// - Parameter text: A string to insert.
     open func insertText(_ text: String) {
+        textInputView.inputDelegate?.selectionWillChange(textInputView)
         textInputView.insertText(text)
-        // Called in TextView since we only want to force the text selection view to update when editing text programmatically.
-        textInputView.sendSelectionChangedToTextSelectionView()
+        textInputView.inputDelegate?.selectionDidChange(textInputView)
     }
 
     /// Replaces the text that is in the specified range.
     /// - Parameters:
     ///   - range: A range of text in the document.
     ///   - text: A string to replace the text in range.
     open func replace(_ range: UITextRange, withText text: String) {
+        textInputView.inputDelegate?.selectionWillChange(textInputView)
         textInputView.replace(range, withText: text)
-        // Called in TextView since we only want to force the text selection view to update when editing text programmatically.
-        textInputView.sendSelectionChangedToTextSelectionView()
+        textInputView.inputDelegate?.selectionDidChange(textInputView)
     }
 
     /// Replaces the text that is in the specified range.
     /// - Parameters:
     ///   - range: A range of text in the document.
     ///   - text: A string to replace the text in range.
     public func replace(_ range: NSRange, withText text: String) {
+        textInputView.inputDelegate?.selectionWillChange(textInputView)
         let indexedRange = IndexedRange(range)
         textInputView.replace(indexedRange, withText: text)
-        // Called in TextView since we only want to force the text selection view to update when editing text programmatically.
-        textInputView.sendSelectionChangedToTextSelectionView()
+        textInputView.inputDelegate?.selectionDidChange(textInputView)
     }
 
     /// Replaces the text in the specified matches.
@@ -1079,7 +1077,6 @@ private extension TextView {
             return
         }
         if gestureRecognizer.state == .ended {
-            willBeginEditingFromNonEditableTextInteraction = false
             let point = gestureRecognizer.location(in: textInputView)
             let oldSelectedRange = textInputView.selectedRange
             textInputView.moveCaret(to: point)
@@ -1240,24 +1237,24 @@ extension TextView: TextInputViewDelegate {
         guard isEditable else {
             return
         }
-        isEditing = !willBeginEditingFromNonEditableTextInteraction
+        isEditing = !isPerformingNonEditableTextInteraction
         // If a developer is programmatically calling becomeFirstresponder() then we might not have a selected range.
         // We set the selectedRange instead of the selectedTextRange to avoid invoking any delegates.
-        if textInputView.selectedRange == nil && !willBeginEditingFromNonEditableTextInteraction {
+        if textInputView.selectedRange == nil && !isPerformingNonEditableTextInteraction {
             textInputView.selectedRange = NSRange(location: 0, length: 0)
         }
         // Ensure selection is laid out without animation.
         UIView.performWithoutAnimation {
             textInputView.layoutIfNeeded()
         }
         // The editable interaction must be installed early in the -becomeFirstResponder() call
-        if !willBeginEditingFromNonEditableTextInteraction {
+        if !isPerformingNonEditableTextInteraction {
             installEditableInteraction()
         }
     }
 
     func textInputViewDidBeginEditing(_ view: TextInputView) {
-        if !willBeginEditingFromNonEditableTextInteraction {
+        if !isPerformingNonEditableTextInteraction {
             editorDelegate?.textViewDidBeginEditing(self)
         }
     }
@@ -1425,7 +1422,13 @@ extension TextView: UITextInteractionDelegate {
             // When long-pressing our instance of UITextInput, the UITextInteraction will make the text input first responder.
             // In this case the user wants to select text in the text view but not start editing, so we set a flag that tells us
             // that we should not install editable text interaction in this case.
-            willBeginEditingFromNonEditableTextInteraction = true
+            isPerformingNonEditableTextInteraction = true
+        }
+    }
+
+    public func interactionDidEnd(_ interaction: UITextInteraction) {
+        if interaction.textInteractionMode == .nonEditable {
+            isPerformingNonEditableTextInteraction = false
         }
     }
 }