Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate WhitelistingTextInputFormatter and BlacklistingTextInputFormatter #59120

Merged
merged 4 commits into from Jun 19, 2020

Conversation

Hixie
Copy link
Contributor

@Hixie Hixie commented Jun 10, 2020

They are replaced by the single class FilteringTextInputFormatter.

Also, Remove deprecated_member_use_from_same_package. Since I ignore this lint rather than adding a bunch of //ignores, I also go and clean up other places we ignored this lint.

fyi @Piinks for the deprecation stuff
fyi @xster since you added these classes

@fluttergithubbot fluttergithubbot added a: tests "flutter test", flutter_test, or one of our tests f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. team Infra upgrades, team productivity, code health, technical debt. See also team: labels. labels Jun 10, 2020
@Hixie
Copy link
Contributor Author

Hixie commented Jun 10, 2020

Might be easier to review by reviewing the two commits separately.

Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

This is awesome! The analyzer has some thoughts though.

@@ -13,3 +13,9 @@ export 'date_picker_deprecated.dart';
export 'date_picker_dialog.dart' show showDatePicker;
export 'date_range_picker_dialog.dart' show showDateRangePicker;
export 'input_date_picker.dart' show InputDatePickerFormField;

// TODO(ianh): Not exporting everything is unusual and we should
Copy link
Contributor

Choose a reason for hiding this comment

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

@darrenaustin may already have a plan for this.

/// Old name for [FilteringTextInputFormatter.deny].
@Deprecated(
'Use FilteringTextInputFormatter.deny instead. '
'This feature was deprecated after 1.19.0-4.0.pre'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'This feature was deprecated after 1.19.0-4.0.pre'
'This feature was deprecated after 1.19.0-4.0.pre.'

I think the deprecations here are just missing a period to make the analyzer happy. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d'oh

@Piinks
Copy link
Contributor

Piinks commented Jun 10, 2020

It looks like customer tests may have to be updated as well.

@Hixie
Copy link
Contributor Author

Hixie commented Jun 10, 2020

I need to talk to the gallery folks, yeah.

@Hixie Hixie force-pushed the filters branch 6 times, most recently from 681e600 to 9591ed4 Compare June 12, 2020 20:01
@Hixie
Copy link
Contributor Author

Hixie commented Jun 12, 2020

Update: PR should have all the tests fixed now, and I spoke to the Gallery folks, they'll update after this lands (I made the specific deprecations that were failing on the Gallery not be failures for now; I'll deprecate them later once there's no breaking changes to worry about).

@Hixie
Copy link
Contributor Author

Hixie commented Jun 13, 2020

@Piinks PTAL

@Hixie
Copy link
Contributor Author

Hixie commented Jun 16, 2020

Fixes #59099

@Hixie
Copy link
Contributor Author

Hixie commented Jun 17, 2020

Merge conflicts fixed. Should be ready to land assuming no problems are found in review.

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

/// in separate passes.
///
/// See also [String.splitMapJoin], which is used to implement this
/// behavior in both cases.
Copy link
Member

Choose a reason for hiding this comment

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

Nice comprehensive doc comment!

static final BlacklistingTextInputFormatter singleLineFormatter
= BlacklistingTextInputFormatter(RegExp(r'\n'));
}

/// Old name for [FilteringTextInputFormatter.allow].
// TODO(ianh): Deprecate these once the samples are migrated.
Copy link
Member

Choose a reason for hiding this comment

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

Which samples is this referring to?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a link?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gallery in samples. they've said they'll fix it when i land this.

Copy link
Member

Choose a reason for hiding this comment

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

@goderbauer
Copy link
Member

Looks like cirrus is unhappy, though.

…matter

They are replaced by the single class FilteringTextInputFormatter.
Since I ignored this lint rather than adding a bunch of //ignores in the earlier commit, I also go and clean up other places we ignored this lint.
@christopherfujino
Copy link
Member

Reverting this TBR by @Hixie as it caused LUCI post-submit to fail with:

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket.appspot.com/8877105797085540816/+/steps/run_test.dart_for_framework_tests_shard/0/stdout

The following StateError was thrown running a test:
Bad state: No element

When the exception was thrown, this was the stack:
#0      Iterable.single (dart:core/iterable.dart:558:25)
#1      WidgetController._getElementPoint (package:flutter_test/src/controller.dart:646:47)
#2      WidgetController.getCenter (package:flutter_test/src/controller.dart:618:12)
#3      WidgetController.tap (package:flutter_test/src/controller.dart:256:18)
#4      main.<anonymous closure> (file:///b/s/w/ir/k/flutter/packages/flutter/test/material/text_field_test.dart:3285:18)
#15     FakeAsync.flushMicrotasks (package:fake_async/fake_async.dart:192:32)
#16     AutomatedTestWidgetsFlutterBinding.runTest.<anonymous closure> (package:flutter_test/src/binding.dart:1135:17)
#17     AutomatedTestWidgetsFlutterBinding.runTest.<anonymous closure> (package:flutter_test/src/binding.dart:1123:35)
(elided 25 frames from dart:async and package:stack_trace)

The test description was:
  Pasted values are formatted

christopherfujino added a commit that referenced this pull request Jun 19, 2020
christopherfujino added a commit that referenced this pull request Jun 19, 2020
christopherfujino added a commit that referenced this pull request Jun 19, 2020
christopherfujino added a commit that referenced this pull request Jun 19, 2020
…tInputFormatter (#59120)" (#59876)

This relands #59120, which was reverted in #59870.
zljj0818 pushed a commit to zljj0818/flutter that referenced this pull request Jun 22, 2020
zljj0818 pushed a commit to zljj0818/flutter that referenced this pull request Jun 22, 2020
zljj0818 pushed a commit to zljj0818/flutter that referenced this pull request Jun 22, 2020
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@braulio94
Copy link
Contributor

Since this is deprecated, what should we use instead?

@braulio94
Copy link
Contributor

Since this is deprecated, what should we use instead?

Got it, eg:

FilteringTextInputFormatter.digitsOnly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: tests "flutter test", flutter_test, or one of our tests f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. team Infra upgrades, team productivity, code health, technical debt. See also team: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants