Support for decimal and signed numeric keyboard #15846

sbaranov · 2018-03-23T00:39:18Z

Fixes #14916.
Requires flutter/engine#4853.

Engine roll includes:

Make flutter_test on Mac exit on error like Linux and Windows. (Make flutter_test on Mac exit on error like Linux and Windows. engine#4873)
Allow FirstFrameListeners to remove themselves from the FlutterView's list (Allow FirstFrameListeners to remove themselves from the FlutterView's list engine#4871)
Allow FirstFrameListeners to remove themselves from the FlutterView's…
Fix vulkan include path (Fix vulkan include path engine#4870)
Support for decimal and signed numeric keyboard (Support for decimal and signed numeric keyboard engine#4853)
CONTRIBUTING.md: explain the need for building host with non-host (CONTRIBUTING.md: explain the need for building host with non-host engine#4863
Roll Dart to 2f68e82526d59b679a522e25db224d8c7e23d280 (- roll Dart to 2f68e82526d59b679a522e25db224d8c7e23d280 engine#4864)

Hixie · 2018-03-23T17:55:12Z

packages/flutter/lib/src/services/text_input.dart

+    this.decimal: false,
+  }) : type = 2;
+
+  final int type;


needs documentation

or you could make it private.

if you leave it public, it should probably be called "index" to match enums.

Hixie · 2018-03-23T17:56:26Z

packages/flutter/lib/src/services/text_input.dart

+  final int type;
+
+  /// The number is signed, allowing a positive or negative sign at the start.
+  /// This flag is only used for numeric input type.


put a blank line between the sentences, so that only the first is considered part of the "brief" description

Hixie · 2018-03-23T17:57:02Z

packages/flutter/lib/src/services/text_input.dart

+  final bool signed;
+
+  /// The number is decimal, allowing a decimal point to provide fractional.
+  /// This flag is only used for numeric input type.


here and above, maybe instead of "numeric" use "[number]"?
also, maybe link to [new TextInputType.numberWithOptions] to say how to set this.

also maybe mention that it defaults to null if the type isn't number?

Hixie · 2018-03-23T17:58:00Z

packages/flutter/lib/src/services/text_input.dart

-  number,
+  /// Requests a default keyboard with ready access to the number keys.
+  /// Additional settings, such as decimal point and/or positive/negative
+  /// signs, can be requested by using the [numberWithOptions] constructor.


I believe you need [new TextInputType.numberWithOptions] to make that xref work.

Just to confirm, using "new" here even though the actual usage is with "const"?

Hixie · 2018-03-23T17:59:22Z

packages/flutter/lib/src/services/text_input.dart

+    'text', 'multiline', 'number', 'phone', 'datetime', 'emailAddress', 'url',
+  ];
+
+  /// Enum value name, this is what enum.toString() would normally return.


// not /// since this isn't documentation we would want to make public, it's a comment.

FYI, style guide says "Use /// for public-quality private documentation".

Hixie · 2018-03-23T18:00:03Z

packages/flutter/lib/src/services/text_input.dart

+  }
+
+  @override
+  String toString() => '$runtimeType('


avoid => with multiline expressions in general (we make some exceptions in build methods where the result is more readable)

Hixie · 2018-03-23T18:00:41Z

packages/flutter/lib/src/services/text_input.dart

+
+  @override
+  bool operator ==(dynamic other) {
+    if (identical(this, other))


you probably don't need this identical check, this is a very cheap set of values to compare

(i don't have a good intuition about whether it's worth including the identical checks or not. In general we have tended to start by not including them, then add them when it becomes obvious that the normal path is starting to be a bit heavy.)

Hixie · 2018-03-23T18:03:16Z

packages/flutter/lib/src/services/text_input.dart

+
+  @override
+  int get hashCode => hashValues(
+      type.hashCode,


just pass in the value, not the value's hashCode. Did you find this pattern somewhere?

Blindly copied from where it used complex object.

Hixie · 2018-03-23T18:03:34Z

packages/flutter/lib/src/services/text_input.dart

+  int get hashCode => hashValues(
+      type.hashCode,
+      signed.hashCode,
+      decimal.hashCode


indent by 2 rather than 4.

This is a case where => looks ok, though we usually would either do it all on one line, or use a block with return.

Hixie · 2018-03-23T18:04:34Z

packages/flutter/lib/src/services/text_input.dart

+
+  @override
+  String toString() => '$runtimeType('
+      'type: \u2524$_name\u251C, '


that's surprisingly fancy, why not just inline the name?

Copied from another class down below, assumed it's some kind of convention on the team. Will happily change to plain format.

Hixie · 2018-03-23T18:04:47Z

packages/flutter/test/services/text_input_test.dart

-        obscureText: true,
-        autocorrect: false,
-        actionLabel: 'xyzzy'
+          inputType: TextInputType.text,


indent by 2

Hixie · 2018-03-23T18:05:21Z

packages/flutter/test/services/text_input_test.dart

+          inputType: TextInputType.text,
+          obscureText: true,
+          autocorrect: false,
+          actionLabel: 'xyzzy'


fwiw: if you're using dartfmt, it usually does a better job when you give trailing commas

Hixie · 2018-03-23T18:08:04Z

packages/flutter/test/widgets/editable_text_test.dart

@@ -161,7 +160,7 @@ void main() {
    controller.text = 'test';
    await tester.idle();
    expect(tester.testTextInput.editingState['text'], equals('test'));
-    expect(tester.testTextInput.setClientArgs['inputType'], equals('TextInputType.text'));
+    expect(tester.testTextInput.setClientArgs['inputType']['type'], equals('TextInputType.text'));
    expect(tester.testTextInput.setClientArgs['inputAction'], equals('TextInputAction.done'));
  });



add tests for:

== and hashCode (not the specific code, just that it's the same when == is the same, and different otherwise)

toString

that signed and decimal can both be set, that they default to false for number and null for non-number

Added in text_field.test.dart.

Hixie · 2018-03-23T18:08:15Z

…oard

cbracken · 2018-03-27T23:27:25Z

* Support for decimal and signed numeric keyboard * Comments * Rebase. * Roll engine to dd6f46c

sbaranov requested a review from Hixie March 23, 2018 00:39

googlebot added the cla: yes label Mar 23, 2018

sbaranov mentioned this pull request Mar 23, 2018

Support for decimal and signed numeric keyboard flutter/engine#4853

Merged

Hixie reviewed Mar 23, 2018

View reviewed changes

jason-simmons mentioned this pull request Mar 27, 2018

SystemInput plugin crash on android #15985

Closed

sbaranov added 5 commits March 27, 2018 13:33

Support for decimal and signed numeric keyboard

c18017b

Comments

d3284ad

Rebase.

83ab2d5

Merge branch 'master' of github.com:flutter/flutter into numeric_keyb…

fa8ce76

…oard

Roll engine to dd6f46c

fa232c9

cbracken mentioned this pull request Mar 27, 2018

Roll engine to ea17fff90b7d79332813c68ca5c7f4c22a479fb6 #15993

Closed

cbracken approved these changes Mar 27, 2018

View reviewed changes

sbaranov merged commit c5288c7 into flutter:master Mar 28, 2018

sbaranov deleted the numeric_keyboard branch March 28, 2018 15:33

DaveShuckerow pushed a commit to DaveShuckerow/flutter that referenced this pull request May 14, 2018

Support for decimal and signed numeric keyboard (flutter#15846)

66150e3

* Support for decimal and signed numeric keyboard * Comments * Rebase. * Roll engine to dd6f46c

github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for decimal and signed numeric keyboard #15846

Support for decimal and signed numeric keyboard #15846

sbaranov commented Mar 23, 2018 •

edited by cbracken

Loading

Hixie Mar 23, 2018

Hixie Mar 23, 2018

Hixie Mar 23, 2018

Hixie Mar 23, 2018

Hixie Mar 23, 2018

Hixie Mar 23, 2018

Hixie Mar 23, 2018

sbaranov Mar 23, 2018

Hixie Mar 23, 2018

sbaranov Mar 23, 2018 •

edited

Loading

Hixie Mar 23, 2018

Hixie Mar 23, 2018

Hixie Mar 23, 2018

Hixie Mar 23, 2018

sbaranov Mar 23, 2018

Hixie Mar 23, 2018

Hixie Mar 23, 2018

sbaranov Mar 23, 2018

Hixie Mar 23, 2018

Hixie Mar 23, 2018

Hixie Mar 23, 2018

sbaranov Mar 23, 2018

Hixie commented Mar 23, 2018

cbracken commented Mar 27, 2018

Support for decimal and signed numeric keyboard #15846

Support for decimal and signed numeric keyboard #15846

Conversation

sbaranov commented Mar 23, 2018 • edited by cbracken Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbaranov Mar 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hixie commented Mar 23, 2018

cbracken commented Mar 27, 2018

sbaranov commented Mar 23, 2018 •

edited by cbracken

Loading

sbaranov Mar 23, 2018 •

edited

Loading