[iOS] Hide keyboard on hot restart#167013
Conversation
github-actions
bot
added
platform-ios
engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm
Show resolved
Hide resolved
loic-sharma
force-pushed
the
ios_hide_keyboard
branch
from
2f225f8 to
ca25f4c
Compare
loic-sharma
force-pushed
the
ios_hide_keyboard
branch
from
41706e9 to
5861980
Compare
loic-sharma
force-pushed
the
ios_hide_keyboard
branch
from
5861980 to
01063d3
Compare
| // This file is modified during the test and needs to be restored at the end. | ||
| final String oldContents = mainFile.readAsStringSync(); | ||
| const String forceKeyboardOn = 'const bool forceKeyboard = true;'; | ||
| const String forceKeyboardOff = 'const bool forceKeyboard = false;'; |
There was a problem hiding this comment.
is it just a raw string to be displayed? why does it look like a piece of code?
There was a problem hiding this comment.
I added a comment to clarify:
// When the test starts, the app forces the keyboard to be visible.
// The test turns off this behavior by mutating the app's source code from
// `forceKeyboardOn` to `forceKeyboardOff`.
// See: //dev/integration_tests/keyboard_hot_restart/lib/main.dartLet me know if you have follow-up questions!
There was a problem hiding this comment.
wow changing the source code from the test, i've never seen this before. perhaps we can pass in an argument into the test instead.
There was a problem hiding this comment.
A test argument wouldn't work as we need to change the app's behavior at the middle of the test: first it should make the keyboard visible, but after the hot restart it shouldn't. Let me know if you'd like to do a call to talk through this further :)
There was a problem hiding this comment.
This seems very brittle. Is this an existing practice?
There was a problem hiding this comment.
We have verify few tests that hot reload or hot restart an app. This appears to be an area of significant tech debt, but below are prior art I could find.
Hot mode test verifies that editing the framework's source code invalidates libraries:
flutter/dev/devicelab/lib/tasks/hot_mode_tests.dart
Lines 128 to 131 in b9abb56
Web dev mode test verifies that web hot reload works as expected:
flutter/dev/devicelab/lib/tasks/web_dev_mode_tests.dart
Lines 90 to 99 in b9abb56
| @@ -0,0 +1,5 @@ | |||
| # keyboard_hot_restart | |||
|
|
|||
| An app used to verify that the keyboard is not visible after a hot restart. | |||
There was a problem hiding this comment.
bonus: maybe also check system context menu is dismissed?
There was a problem hiding this comment.
The presence of the SystemContextMenu widget is probably enough to indicate it's showing or not, if you can check for that. If not, maybe you could print something from the TextField's contextMenuBuilder.
There was a problem hiding this comment.
Ahhh I didn't realize I could check for the context menu on the framework side, thanks for the pointers!
There was a problem hiding this comment.
Justin and I talked offline. Unfortunately, the presence of the SystemContextMenu widget and TextField's contextMenuBuilder are not enough to detect whether the native context menu is visible. We don't have a good way to verify this native context menu from this "outside" test.
engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm
Outdated
Show resolved
Hide resolved
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM but deferring to Huan for iOS expertise. Also I agree you should check for the presence of the system context menu in the test if you can.
Thanks for fixing this! It has annoyed me before. And thanks for writing a devicelab test for this.
| @@ -0,0 +1,5 @@ | |||
| # keyboard_hot_restart | |||
|
|
|||
| An app used to verify that the keyboard is not visible after a hot restart. | |||
There was a problem hiding this comment.
The presence of the SystemContextMenu widget is probably enough to indicate it's showing or not, if you can check for that. If not, maybe you could print something from the TextField's contextMenuBuilder.
| } | ||
| [owner_controller_.platformViewsController reset]; | ||
| [owner_controller_.restorationPlugin reset]; | ||
| [owner_controller_.textInputPlugin reset]; |
There was a problem hiding this comment.
This is all it took to fix it?
There was a problem hiding this comment.
Yup. The fix took 30 seconds, the test took 3 days 😅
| } | ||
| [owner_controller_.platformViewsController reset]; | ||
| [owner_controller_.restorationPlugin reset]; | ||
| [owner_controller_.textInputPlugin reset]; |
There was a problem hiding this comment.
just curious - the reset method of text input plugin was defined but never called before?
There was a problem hiding this comment.
No reset is new, but it calls the existing hideTextInput method which is private and responds to the framework's TextInput.hide message.
hellohuanlin
left a comment
hellohuanlin
left a comment
There was a problem hiding this comment.
Thank you for chatting offline about modifying the source file reasons.
loic-sharma
added
the
autosubmit
flutter-dashboard
bot
removed
the
autosubmit
3 participants
This hides the keyboard and text input context menu if you hot restart your iOS app.
Before.MP4
After.MP4
Part of #10713
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.