Can you rename to flt_firstResponder in case of naming conflict? (ObjC doesn't support namespace)

Good idea! Went through and made the change.

Nit maybe kOnInteractiveKeyboardPointerMoveMethod = @"TextInput.onPointerMoveForInteractiveKeyboard", to be more specific.

Just updated, I'll make sure to try and add Interactive Keyboard more in the future, definitely would help with clarity!

use CGRect rather than double for UI stuff.

With regards to the pointerY this only contains a single double, so I am not sure that this would classify as UI.

sorry i meant CGFloat

Gotcha, just updated the code!

nits:

Will do! Just slipped by me.

Using _keyboardViewContainer.subviews.count == 0 doesn't seems to be safe and future proof. For example, what if someone added a new subview for _keyboardViewContainer and didn't know this logic exist? Could you use a different indicator?

Definitely a concern, the same thing occurs in the tests so there definitely is a need to update this approach. I went through and included a UIView that represents the screenshot. Then we can see if it the superview matches instead.

nits: could you refactor these lines into a method to match what you did in if
Maybe something like:
`[self setKeyboardContainerHeight:pointerY]

Done! Always helpful to simplify code into sections to increase understandability.

@cyanglaz i could be wrong, but iirc the standard practice is just assign when exposing the property in test header (similar to the activeView properly above). weak is confusing since it's actually a strong property.

Oh. I didn't realize it is only exposing the property. I'm not aware of the standard practice tho but since activeView also used assign, I will take your word for it :)

@cyanglaz and I just discussed offline - it should be readonly here, since your unit test does not need the setter.

@property(nonatomic, readonly) UIView* keyboardViewContainer;

Gotcha! That works for me, just went through and made the changes.

We are moving away from using [UIScreen mainScreen]. I don't think we should block this PR because of this, but we will need to follow up to clean up later.
FYI @mossmana

Is the pointerY based on the screen, or the flutter app?

Since it comes from the Framework, I assume it is based on the flutter app? So In scenarios (add to app, or app extension) when the flutter app does not fill the full screen, the calculation below that determines if the pointer goes overtop of the keyboard should be based on the flutter app's height instead of the screen height, correct?

Maybe instead of the screenHeight, we should use the height of the FlutterViewController's view?

We are moving away from using [UIScreen mainScreen]. I don't think we should block this PR because of this, but we will need to follow up to clean up later. FYI @mossmana

@Matt2D While usually I would agree with not blocking the PR, in this particular case, this is a new introduction of [UIScreen mainScreen]. I would prefer that we don't add new instances. You can refer to flutter/flutter#125496 for how we are handling the deprecation in other places.

@cyanglaz and I were discussing this the other day, and it's actually a bit tricky here - we would need to refactor this code to make it re-usable.

I think I can take this part over. Can make a PR today.

@cyanglaz in this case, since this is being used to determine the pointer position on the screen, this is relative to the screen rather than the flutter view.

FYI this is the PR promised: #44303

After that gets landed, you can do something like this here:

UIScreen *screen = _viewController.flutterScreenIfViewLoaded;
if (!screen) {
  return;
}
CGFloat screenHeight = screen.bounds.size.height;
// ...

Thank you for the code snippet, I think the new api is much cleaner! Updated the code using the mentioned solution.

ditto about [UIScreen mainScreen]

Updated with the fix provided by the new PR Huan did.

Should it be FlutterViewController instead?

In add to app scenario, the rootViewController might not be FlutterViewController, which might be covered by FlutterViewController. So the keyboard might be shown "behind" the FlutterViewController

Since this is positioning a keyboard relative to the full screen -- regardless of whether the FlutterView is full screen, or a subview covering only part of the screen in an add-to-app scenario -- I think we want the root view controller don't we?

Ah yeah that's true. We would need to make sure to position the keyboard screenshot on the very top. Set ZIndex to maxInt maybe?

Definitely worth adding!

Did you mean if (_keyboardView.superview == nil)

Yep you are right! Thanks for catching this.

minor optional nits:

The logic of this method can also be used for the scenario where the pointer is not overtop the keyboard.
The frameRect's Y can be updated to be the max of pointerY and keyboard height.

  CGRect frameRect = _keyboardRect;
  if (screenHeight - keyboardHeight <= pointerY) {
      frameRect.origin.y = pointerY;
  }
  _keyboardViewContainer.frame = frameRect;

Then in the handlePointerMove, the logic can be simplified as

if (_keyboardView.superview != nil) {
   [self setKeyboardContainerHeight:pointerY];
} else {
   [self takeScreenshot];
   [self hideKeyboardWithoutAnimation];
}

It turns out that for the change we still need to check the height for takeScreenshot since we don't want to take the screenshot too early:

- (void)handlePointerMove:(CGFloat)pointerY {
  //View must be loaded at this point.
  UIScreen* screen = _viewController.flutterScreenIfViewLoaded;
  double screenHeight = screen.bounds.size.height;
  double keyboardHeight = _keyboardRect.size.height;

  if (_keyboardView.superview != nil) {
    // If screenshot is present.
    [self setKeyboardContainerHeight:pointerY];
  } else {
    // If no screenshot has been taken.
    if (screenHeight - keyboardHeight <= pointerY) {
      //If pointer is above keyboard.
      [self takeKeyboardScreenshotAndDisplay];
      [self hideKeyboardWithoutAnimation];
    }
  }
}

- (void)setKeyboardContainerHeight:(CGFloat)pointerY {
  //View must be loaded at this point.
  UIScreen* screen = _viewController.flutterScreenIfViewLoaded;
  double screenHeight = screen.bounds.size.height;
  double keyboardHeight = _keyboardRect.size.height;

  CGRect frameRect = _keyboardRect;
  if (screenHeight - keyboardHeight <= pointerY) {
      frameRect.origin.y = pointerY;
  }
  _keyboardViewContainer.frame = frameRect;
}

Ah I missed that part. So the caller part can probably be

if (_keyboardView.superview != nil) {
   [self setKeyboardContainerHeight:pointerY];
} else if (screenHeight - keyboardHeight <= pointerY) {
   [self takeScreenshot];
   [self hideKeyboardWithoutAnimation];
}

Discussed offline - the duplicate logic of screenHeight - keyboardHeight <= pointerY can get error prune.

Another idea is to pass in an additional BOOL to the helper function, but that also makes the code unnecessarily complicate.

I think it makes sense to keep it as it is.

optional nits: use a readonly property instead.

Avoid underscore in obj-c identifiers. We're doing something similar in another outstanding PR on the macOS side and using flutter as the prefix here:
https://github.com/flutter/engine/pull/43101/files#diff-edbbe0be10d99338c2496cad85a27776c79ec8b260825ee4d309b1e4d194d5b2R10

So we'd end up with flutterFirstResponder here.

@cyanglaz @hellohuanlin preferences? We should probably be consistent across embedders.

flutterFirstResponder SGTM

Will do! Thank you for the suggestion

Maybe something like:

(And maybe move the comment inside the conditional block)

Makes sense, thank you!

Consider renaming to something more specific for readability -- e.g. take KeyboardSnapshot

Definitely, I also added add to screen to be even better for readability.

Consider just inlining this into the call below, or move this up above and use in both the event above and the check below.

Definitely!

Similar comment here.

Done

Maybe rather than just "up" something like "back to original position"?

Good idea! Done