Skip to content

Conversation

@LongCatIsLooong
Copy link
Contributor

Explains when not to use a FilteringTextInputFormatter.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@LongCatIsLooong LongCatIsLooong requested a review from justinmc July 18, 2022 16:53
@flutter-dashboard flutter-dashboard bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Jul 18, 2022
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

/// preserve the existing [TextEditingValue.selection] to values it would now
/// fall at with the removed characters.
/// A [FilteringTextInputFormatter] also tries to preserve the existing
/// [TextEditingValue.selection] and [TextEditingValue.composing], and adjusts
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe add a "region" after "[TextEditingValue.composing]" to make this read better?

/// them accordingly if text within either of these ranges is replaced.
///
/// This formatter is typically used to match potentially recurring [Pattern]s
/// in the new [TextEditingValue], and it never completely rejects the new
Copy link
Member

Choose a reason for hiding this comment

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

maybe break this sentence up a little. It's super long with the two "and"s.

@LongCatIsLooong
Copy link
Contributor Author

@goderbauer rephrased some of the new docs could you take a look again?

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.

Still LGTM

/// [TextEditingValue] and falls back to the current [TextEditingValue] when the
/// given [filterPattern] fails to match. Consider using a different
/// [TextInputFormatter] such as:
/// `TextInputFormatter.withFunction((oldValue, newValue) => regExp.hasMatch(newValue.text) ? newValue : oldValue)`.
Copy link
Member

Choose a reason for hiding this comment

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

could you wrap this in a ```dart section to get nicer code formatting in docs?

@LongCatIsLooong LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 27, 2022
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍

/// fall at with the removed characters.
/// A [FilteringTextInputFormatter] also tries to preserve the existing
/// [TextEditingValue.selection] and the [TextEditingValue.composing] region,
/// adjusting them accordingly if text within either of these ranges is replaced.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Is it also possible that that text before these ranges is replaced and they need to be updated? Not sure but just being pedantic.

/// for accepting/rejecting new input based on a predicate on the full string.
/// As an example, [FilteringTextInputFormatter] typically shouldn't be used
/// with [RegExp]s that contain positional matchers (`^` or `$`) since these
/// patterns are usually meant for matching the whole string.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is much more clear to me and I feel like I know what to do in both situations thanks to the examples 👍

@LongCatIsLooong LongCatIsLooong removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 27, 2022
@LongCatIsLooong LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 27, 2022
@auto-submit
Copy link
Contributor

auto-submit bot commented Jul 27, 2022

  • The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 27, 2022
@LongCatIsLooong LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 27, 2022
@auto-submit auto-submit bot merged commit 41940c9 into flutter:master Jul 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jul 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 28, 2022
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
@LongCatIsLooong LongCatIsLooong deleted the filtering-input-formatter-docs branch September 13, 2022 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TextInputFormatter] FilteringTextInputFormatter might have some handling mistakes

3 participants