Conversation
delegate to separate class * rename/rearrange variables and fns for clarity * additional misc nonfunctional maintainability improvements
| for gestureRecognizer in self.gestureRecognizers ?? [] { | ||
| // receive touch events even if they are part of one of these recognized gestures | ||
| // this supports checkboxes and other interactive markdown elements in the text area (crudely) | ||
| if gestureRecognizer.name == "UITextInteractionNameSingleTap" | ||
| || gestureRecognizer.name == "UITextInteractionNameTapAndAHalf" | ||
| || gestureRecognizer.name == "UITextInteractionNameLinkTap" | ||
| { | ||
| gestureRecognizer.cancelsTouchesInView = false | ||
| } | ||
| } | ||
|
|
||
| for gesture in gestureRecognizers ?? [] { | ||
| let gestureName = gesture.name?.lowercased() | ||
|
|
||
| if gestureName?.contains("interactiverefinement") ?? false { | ||
| gesture.addTarget(self, action: #selector(longPressGestureStateChanged(_:))) | ||
|
|
||
| // send interactive refinements to our handler | ||
| // this is the intended way to support a floating cursor | ||
| if gestureRecognizer.name == "UITextInteractionNameSingleTap" { | ||
| gestureRecognizer.addTarget( | ||
| self, action: #selector(handleInteractiveRefinement(_:))) | ||
| } | ||
| } |
There was a problem hiding this comment.
The new version of this looks clearer to me. Investigation revealed that self.addInteraction(textInteraction) adds all gesture recognizers in textInteraction.gesturesForFailureRequirements to self.gestureRecognizers, allowing me to iterate them in one loop. I also call out the names of the recognizers specifically.
Todo: looks like I forgot to fill in the correct name for the interactive refinement recognizer
There was a problem hiding this comment.
Todo update: done ✅
| @objc func handlePan(_ sender: UIPanGestureRecognizer? = nil) { | ||
| mtkView.handleTrackpadScroll(sender) | ||
| } |
There was a problem hiding this comment.
While intended for the trackpad, this is a pan handler, so this name makes more sense to me. Vaguely more google-able. I would rename the mtkView function too, but actually there's already a handlePan there. This one's used for markdown and that one for drawing. Expect more renames like this after I clean that up.
| @objc private func handleInteractiveRefinement(_ recognizer: UIGestureRecognizer) { | ||
| switch recognizer.state { | ||
| case .possible: | ||
| break | ||
| case .began: | ||
| isLongPressCursorDrag = true | ||
| case .cancelled, .ended: | ||
| isLongPressCursorDrag = false | ||
|
|
||
| interactiveRefinementInProgress = true | ||
| case .changed: | ||
| break | ||
| case .ended, .cancelled, .failed: | ||
| interactiveRefinementInProgress = false | ||
|
|
||
| inputDelegate?.selectionWillChange(self) | ||
| mtkView.drawImmediately() | ||
| inputDelegate?.selectionDidChange(self) | ||
| default: | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
Enumerate all enum variants (can you tell I am a Rust engineer?). Also rename fn and mutated property to be vaguely more google-able.
There was a problem hiding this comment.
you didn't get rid of the default tho...
There was a problem hiding this comment.
Swift issues a warning if I do because the set of variants could change
| pan.allowedTouchTypes = [ | ||
| NSNumber(value: UITouch.TouchType.direct.rawValue), | ||
| NSNumber(value: UITouch.TouchType.indirect.rawValue), | ||
| // NSNumber(value: UITouch.TouchType.pencil.rawValue), |
There was a problem hiding this comment.
mentioning the disallowed touch type made this make more sense to me
| NSNumber(value: UITouch.TouchType.indirectPointer.rawValue), | ||
| ] | ||
| pan.allowedScrollTypesMask = .all | ||
| if UIPencilInteraction.prefersPencilOnlyDrawing { // todo: update when prefersPencilOnlyDrawing changes |
There was a problem hiding this comment.
possible bug discovered: updating the pencil only drawing preference does not update the min touches for a pan
| /// Returns whether the event should be forwarded up the inheritance hierarchy | ||
| func handleKeyEvent(_ presses: Set<UIPress>, with event: UIPressesEvent?, pressBegan: Bool) | ||
| -> Bool |
There was a problem hiding this comment.
This function was updating a class property that callers would check to see what to do next. Now it returns a bool indicating what to do next and the property has been removed.
| selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" | ||
| selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" | ||
| selectedDebuggerIdentifier = "" | ||
| selectedLauncherIdentifier = "Xcode.IDEFoundation.Launcher.PosixSpawn" |
There was a problem hiding this comment.
I don't see the corresponding issue for this
|
remaining QA will be async |
This PR makes cleanups to iOSMTK.swift, mostly to the text and drawing input wrappers but also to the metal view itself.
I wasn't careful to be sure that everyone would like every cleanup. As a significant motivation, this cleanup has helped me get hands-on experience with Swift. I also intend to maintain this logic going forward and wasn't shy about subjecting it to personal preferences.
Changes:
iOSMTKTextInputWrapper->MdView) and move related definitions togetherQA steps:
select, copy, then paste some strokes (touch)select, copy, then paste some strokes (pencil)select, copy, then paste some strokes (universal control)pan and zoom at the same time (universal control)Existing issues: