[iOS] [iPad] fix avoid bottom inset in split view mode

[RichardFevrier]

Fix flutter/flutter#109845

Pre-launch Checklist

• [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
• [x] I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• [x] I listed at least one issue that this PR fixes in the description above.
• [ ] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
• [ ] I updated/added relevant documentation (doc comments with ///).
• [x] I signed the CLA.
• [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flutter-dashboard[bot]]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[cyanglaz]

Looks like the bug wasn't what I thought in https://github.com/flutter/flutter/issues/109845#issuecomment-1222742951.

@LongCatIsLooong @justinmc Are you the right people to review this?

@RichardFevrier, tests need to be added in order for this PR to land.

[RichardFevrier]

I don't know why we used keyboardWillBeHidden in the first place instead of keyboardWillChangeFrame. Seems like this should all still work though, but me or someone else should probably try this out locally and confirm that nothing seems broken.

Worse than that @justinmc, it calls twice the animation via startKeyBoardAnimation and can probably be related with flutter/flutter/issues/109435

Also something that I wanted to say the week before without closing the issue is: I do not know why the CI failed at testEnsureBottomInsetIsZeroWhenKeyboardDismissed since this PR can only improve that...

Can we also make sure this change doesn't regress flutter/flutter#99951

I can try that @cyanglaz on my iPad but is there not a Unit test to check it automatically?

And last point, I am okay to make a Unit Test to avoid regressions on that point, but is it possible to write Unit test for split view apps with your framework?

Thanks.

[luckysmg]

Maybe just delete the code below can fix this?

// Delete this code in keyboardWillBeHidden
  id isLocal = info[UIKeyboardIsLocalUserInfoKey];
  if (isLocal && ![isLocal boolValue]) {
    return;
  }

[luckysmg]

Updated: keyboardWillChangeFrame will include show/hide/changeFrame so maybe keyboardHidden seems to be redundant.

So this look seems better, and we just need to make sure that the bool isDismissing = CGRectEqualToRect(keyboardFrame, CGRectZero) || keyboardFrame.origin.y >= screenRect.size.height; or
bool isShowing = CGRectIntersectsRect(keyboardFrame, screenRect); is accurate to judge whether the keyboard is open or close currently😄.

[jmagman]

@RichardFevrier were you able to figure out how to write the split view mode integration tests? Also it looks like @luckysmg had some suggestions. @cyanglaz was this ready for re-review or are you waiting for the unit tests?

[cyanglaz]

I can try that @cyanglaz on my iPad but is there not a Unit test to check it automatically?

We do have unit-tests cover it but not integration test. It would be nice to at least do a manual test to make sure it is not regressed. If it is, then we will need to add better tests for https://github.com/flutter/flutter/issues/99951

I think we can land this PR when 1. Unit-tests (or even better integration tests) are added, 2. When we did a manual test that https://github.com/flutter/flutter/issues/99951 is not regressed.

[chinmaygarde]

It seems like this is still WIP. Adding the labels. If OTOH no progress is being made, perhaps we can close this?

[flutter-dashboard[bot]]

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.