Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@justinmc
Copy link
Contributor

@justinmc justinmc commented Dec 21, 2018

Closes flutter/flutter#25454

The logic for converting the passed "inputType" to the native input type differed between Android and iOS. The iOS code was differing from what the flutter docs say, so I have changed that code to match the docs and Android.

The equivalent Android code is at https://github.com/flutter/engine/blob/master/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java#L78.

@justinmc justinmc self-assigned this Dec 21, 2018
@justinmc justinmc changed the title WIP TextInputType.number default fix TextInputType.number default fix Dec 21, 2018
@GaryQian
Copy link
Contributor

GaryQian commented Dec 21, 2018

Can we add tests to ensure the correct keyboard is returned? Not sure about how well the engine testing framework is set up to be able to test this though. Maybe a framework side test? Otherwise, the change itself LGTM.

@justinmc
Copy link
Contributor Author

@GaryQian I was wondering the same thing. I'll check for a way to assert which keyboard is shown in the driver tests, but I'm not sure if that's possible. It seems like there are no unit tests for these files so I can't go that route.

@GaryQian
Copy link
Contributor

GaryQian commented Dec 21, 2018

There is a larger line of work to add tooling to improve testability of the engine. We currently rely on the flutter framework tests to semi-indirectly test the engine, however this workflow is rather inefficient and adds an undesired level of separation between test and code.

For now, it should be possible to add a separate framework test for this in another PR.

@justinmc
Copy link
Contributor Author

Note to self about testing this:

I had an idea to test this by trying to enter text that isn't supported by the current keyboard, then asserting that the TextField doesn't contain those characters, but wasn't able to get the driver tests working on my mac. Will come back to it. https://github.com/justinmc/flutter/tree/keybaord-driver-test

@justinmc
Copy link
Contributor Author

I was holding off on merging this because I wanted to figure out a way to test this, but I don't think it's possible. Similar to my other keyboard related engine PR I wanted to test (#6989) and its related integration test issue, I'm going to merge and keep an eye out for the ability to test this in the future.

@justinmc justinmc merged commit 3f99878 into flutter:master Jan 11, 2019
@justinmc justinmc deleted the input-type-number branch January 11, 2019 18:24
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 11, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 11, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 11, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 12, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Jan 12, 2019
flutter/engine@b7f6bf0...35daf14

git log b7f6bf0..35daf14 --no-merges --oneline
35daf14 Roll src/third_party/skia d2fa7e33798c..c334df71b8f9 (45 commits) (flutter/engine#7448)
6179ac6 fix up analysis for Dart in Engine (flutter/engine#7404)
bec12d8 Reland Dart SDK rolls made since 2019/01/08 (flutter/engine#7446)
3f99878 Match the ios number input type behavior to what is said in the docs (flutter/engine#7281)
358a24c Make SetLocales more consistent with other RuntimeController methods (flutter/engine#7447)
0c11836 Use anti-aliasing when drawing text in the performance overlay (flutter/engine#7445)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
kangwang1988 pushed a commit to XianyuTech/flutter that referenced this pull request Feb 12, 2019
flutter/engine@b7f6bf0...35daf14

git log b7f6bf0..35daf14 --no-merges --oneline
35daf14 Roll src/third_party/skia d2fa7e33798c..c334df71b8f9 (45 commits) (flutter/engine#7448)
6179ac6 fix up analysis for Dart in Engine (flutter/engine#7404)
bec12d8 Reland Dart SDK rolls made since 2019/01/08 (flutter/engine#7446)
3f99878 Match the ios number input type behavior to what is said in the docs (flutter/engine#7281)
358a24c Make SetLocales more consistent with other RuntimeController methods (flutter/engine#7447)
0c11836 Use anti-aliasing when drawing text in the performance overlay (flutter/engine#7445)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants