Create a text validator/formatter mechanism by xster · Pull Request #9535 · flutter/flutter

xster · 2017-04-21T21:18:06Z

#9511

Add a text formatter interface used by EditingText. Provide some default implementations.

xster · 2017-04-21T21:18:42Z

Soliciting some high level feedback before I put in all the tests

Hixie · 2017-04-21T21:36:44Z

why only those two instead of all four?

nit: spaces around named argument declarations

it was arbitrary :) changed

cbracken · 2017-04-21T23:53:47Z

s/.$/ patterns./

cbracken · 2017-04-21T23:54:36Z

... defaults to the empty string.

cbracken · 2017-04-22T00:01:32Z

Maybe:

/// This formatter attempts to preserve the [TextEditingValue.selection].

That said, as-is, I'm not convinced this adds much. It feels like we're saying 'This formatter behaves how you'd expect it to.' Perhaps more directly state that this formatter will adjust the selection in the case that blacklisted characters were removed?

reworded a bit

cbracken · 2017-04-22T00:15:39Z

Is this correct?

~~During a copy-paste, I'd assume a replacementString where every contiguous string of newlines is collapsed to a single 0x20 space.~~
During a copy-paste, I'd assume a replacementString that replaces each newline with a 0x20 space.

During text entry, ideally I'd want \n to be replaced with the empty string, but in single-line text fields, I suspect we'll also set a keyboard that only supplies a Done key so using a space is probably a non-issue.

Not sure I fully caught what you meant. Can you add an example? Or did you want r'\n+'?

if the user pastes:

with a replacement string of ' ' (1 space), it outputting ' ' (3 spaces) feels more predictable (and custom behaviours are 1 line away for the developer).

Corrected my original statement. Should be a straight-up 1:1 replacement of \n with a space.

I think that's true no? If there's 20 \n in whichever placement, replaceAll will replace them all with 20 spaces right?

Exactly - yep.

Hixie · 2017-04-22T00:26:53Z

nit: the formatting here isn't consistent with the style we normally use

Hixie · 2017-04-22T00:27:06Z

nit: newline between the members

Hixie · 2017-04-22T00:27:46Z

can we guarantee that you can't compose something with a newline?

Can't but I think it's a bit too edge case-y to worry about. My intent is just handle cases where if someone blacklists latin characters and starts typing chinese, there should still be latin characters in the composition interim. If the user presses send/save/wtv right at that moment, I think we should 'commit' the composition first (which triggers the formatting) and then pass it out.

I think when you're in a single-line text field, we need to be able to guarantee that you cannot ever enter a newline.

I was pedantic with the wording. But pragmatically, I don't think it's possible to enter a newline while composing.

How about if the code passes in a newline in the original string? Or if someone writes a custom keyboard that does it?

what did you mean the code passes in? Did you mean if the user passes in a second formatter that breaks the first one?

Either way, added handling to format text under composition

I meant if someone writes Flutter code that sets the initial value of the editable text field to 'a\nb'.

Hixie · 2017-04-22T00:29:49Z

mutating the list is sketchy. if someone creates one then passes it in each time, it'll get progressively longer.
if someone passes in a const list, it'll crash.

Ya, good call, that was a disaster... Changed

xster · 2017-04-22T02:32:48Z

I expanded this PR a bit to avoid diffbase gymnastics.

Added a WhitelistingTextInputFormatter
Use a digits only whitelisting formatter in the gallery demo
Add a phone number formatter in the gallery demo

Now fixes #9511. Fixes #9340. And fixes a bug in the gallery on iOS that prevents me from testing #9200.

PTAL. Will add tests all around.

Hixie · 2017-04-24T15:42:26Z

US phone number

Hixie · 2017-04-24T17:21:35Z

put the { on the previous line and the } on the next line to match similar code elsewhere in the codebase

Hixie · 2017-04-24T17:21:55Z

nit: don't use => if you have a line break

Hixie · 2017-04-24T17:22:15Z

xster · 2017-04-25T01:11:35Z

Added tests

xster · 2017-04-28T19:50:39Z

Gentle ping

Hixie · 2017-04-28T20:43:18Z

LGTM but @cbracken should rereview since he marked it as X.

cbracken

lgtm

Since TextInputFormatter & co. aren't strictly dependent on anything in widgets, consider moving those to services. On the off chance anyone else decided to implement their own widgets alternative, they could still deal with the existing validators.

…ult implementations.

…ext demo.

xster · 2017-04-28T22:26:41Z

oh ya, that's a really good point. Since the TextSelection stuff is already in services. Done

xster requested review from abarth and cbracken April 21, 2017 21:18

googlebot added the cla: yes label Apr 21, 2017

Hixie reviewed Apr 21, 2017

View reviewed changes

xster force-pushed the text-formatter branch from 46dc0c8 to 8e37623 Compare April 21, 2017 23:05

cbracken requested changes Apr 22, 2017

View reviewed changes

Hixie reviewed Apr 22, 2017

View reviewed changes

Hixie reviewed Apr 24, 2017

View reviewed changes

xster force-pushed the text-formatter branch from 7a8219e to df90750 Compare April 24, 2017 20:31

cbracken approved these changes Apr 28, 2017

View reviewed changes

xster added 9 commits April 28, 2017 15:23

Add a text formatter interface used by EditingText. Provide some defa…

d9135b1

…ult implementations.

self nits

48edf71

Handle -1 selection offsets

c818a58

review notes

e28c526

simplify regular expression

92b0c1a

Add whitelisting formatters. Use a custom phone number formatter in t…

91f545c

…ext demo.

review notes

7908912

not being able to addAll(null) is pretty annoying

0b17777

review notes

82d6a71

xster added 4 commits April 28, 2017 15:23

partial tests

1a5bb3d

Add tests

3ae8541

didn’t end up needing mockito

3e56409

move to services

34f58dc

xster force-pushed the text-formatter branch from 1ce366c to 34f58dc Compare April 28, 2017 22:26

xster merged commit f65fea8 into flutter:master Apr 28, 2017

xster deleted the text-formatter branch April 28, 2017 22:33

github-actions Bot locked as resolved and limited conversation to collaborators Aug 13, 2021

Conversation

xster commented Apr 21, 2017

Uh oh!

xster commented Apr 21, 2017 • edited 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!

cbracken Apr 22, 2017 • 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!

xster Apr 22, 2017 • 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!

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!

xster commented Apr 22, 2017 • edited 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!

xster commented Apr 25, 2017

xster commented Apr 21, 2017 •

edited

Loading

cbracken Apr 22, 2017 •

edited

Loading

xster Apr 22, 2017 •

edited

Loading

xster commented Apr 22, 2017 •

edited

Loading