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

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

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

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

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

// 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

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

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

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

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

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

indent by 2

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

packages/flutter/test/services/text_input_test.dart

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

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

packages/flutter/test/services/text_input_test.dart Outdated

Copy link

Contributor

Hixie Mar 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent by 2

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

Support for decimal and signed numeric keyboard #15846

Support for decimal and signed numeric keyboard #15846

Uh oh!

Conversation

sbaranov commented Mar 23, 2018 • edited by cbracken Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbaranov Mar 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hixie commented Mar 23, 2018

Uh oh!

cbracken commented Mar 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sbaranov commented Mar 23, 2018 •

edited by cbracken

Loading

sbaranov Mar 23, 2018 •

edited

Loading