iOS smart quote/dash configuration #44923

justinmc · 2019-11-14T18:39:17Z

Description

Flutter does not allow configuration of UITextSmartQuotesType and UITextSmartDashesType. This is a problem because it can result in weird punctuation characters being entered into password fields, among other things.

This PR will expose two new boolean parameters on all text fields: enableSmartQuotes and enableSmartDashes. These will cause their corresponding type to be set in iOS, and in Android they will be ignored.

They will both default to true, except on obscured text fields, where they will default to false.

Related Issues

Closes #26684.
Depends on engine PR flutter/engine#13863.

Tests

I test that the engine receives these parameters properly and that they default to the correct value in EditableText.

…changes

fluttergithubbot · 2019-11-14T18:39:20Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

GaryQian · 2019-11-15T19:12:42Z

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

  final bool autocorrect;

+  /// {@template flutter.services.textInput.enableSmartDashes}
+  /// Whether to allow the platform to automatically format dashes.


It may be useful to explicitly explain what this is doing other than this very short. How is it being formatted?

GaryQian · 2019-11-15T19:14:37Z

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

+  /// {@template flutter.services.textInput.enableSmartDashes}
+  /// Whether to allow the platform to automatically format dashes.
+  ///
+  /// This flag only affects iOS. It sets UITextSmartDashesType in the engine.


I'm not clear on what we generally do in cases where API is platform specific. I feel like additions like this lead to confusing API, where many properties may end up being useless when a developer is using it.

You should double check on the policy here. Preferably, we can do something less iOS specific, but IDK If that is possible here.

@Hixie Thoughts?

If it's even remotely reasonable to come up with a way to make it platform-agnostic (bearing in mind that over time we're going to support many platforms beyond iOS and Android), then we should attempt to make the code platform-agnostic, even if it only currently works on one platform.

If something really is just platform-specific, then that's fine.

LongCatIsLooong · 2019-11-20T19:15:37Z

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

+  /// Whether to allow the platform to automatically format dashes.
+  ///
+  /// This flag only affects iOS. It sets UITextSmartDashesType in the engine.
+  /// When true, it passes UITextSmartDashesTypeYes, and when false, it passes


Might be worth documenting the iOS SDK version constraint on both flags.

LongCatIsLooong · 2019-11-20T19:26:37Z

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

+  ///  * [enableSmartQuotes]
+  ///  * <https://developer.apple.com/documentation/uikit/uitextinputtraits>
+  /// {@endtemplate}
+  final bool enableSmartDashes;


The engine implementation seems to toggle between default and no. Is there a way for users to set the trait to UITextSmartQuotesType.yes?

Good point, there is no way right now... I was under the impression that "yes" and "default" were the same, but could that ever change, or is there any other reason to explicitly allow "yes"? https://developer.apple.com/documentation/uikit/uitextsmartdashestype?language=objc

There seems to be a very slight difference between yes and default, but "default" and "no" should cover most use cases:

This option selectively enables the smart dash behavior based on the keyboard type. For example, smart dashes are disabled for email address and password keyboards.

Seems like having an enum rather than a bool would make this possible to extend later, even if we only support two values today.

LongCatIsLooong · 2019-11-20T19:27:17Z

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

+  /// character will be automatically replaced by a left or right double quote
+  /// depending on its position in a word.
+  ///
+  /// Defaults to true, unless obscureText is true, when it defaults to false.


nit:

Suggested change

/// Defaults to true, unless obscureText is true, when it defaults to false.

/// Defaults to true, unless [obscureText] is true, when it defaults to false.

LongCatIsLooong · 2019-11-20T19:30:30Z

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

+  ///
+  /// This flag only affects iOS. It sets UITextSmartQuotesType in the engine.
+  /// When true, it passes UITextSmartQuotesTypeYes, and when false, it passes
+  /// UITextSmartQuotesTypeNo.


nit:

/// This flag only affects iOS. It sets [`UITextSmartQuotesType`](https://developer.apple.com/documentation/uikit/uitextsmartquotestype) in the engine. /// When true, it passes [`UITextSmartQuotesTypeYes`](https://developer.apple.com/documentation/uikit/uitextsmartquotestype/yes), and when false, it passes /// [`UITextSmartQuotesTypeNo`](https://developer.apple.com/documentation/uikit/uitextsmartquotestype/no).

~~I wonder if dart docs understands hyper links. Would be nice if we can link these uikit references to their api docs.~~

Also doesn't it pass UITextSmartQuotesTypeDefault when true, according to the engine PR?

Linked and updated to "default" (however, see my question above about "yes" vs. "default").

LongCatIsLooong · 2019-11-20T19:37:20Z

packages/flutter/lib/src/material/text_field.dart

    properties.add(DiagnosticsProperty<bool>('autofocus', autofocus, defaultValue: false));
    properties.add(DiagnosticsProperty<bool>('obscureText', obscureText, defaultValue: false));
    properties.add(DiagnosticsProperty<bool>('autocorrect', autocorrect, defaultValue: true));
+    properties.add(DiagnosticsProperty<bool>('enableSmartDashes', enableSmartDashes, defaultValue: true));


The defaultValue seems to be !obsecureText in the constructor. Is there a reason for the defaultValue to deviate from the constructor?

There is not, good catch. I'll change it to !obscureText.

Hixie · 2019-12-04T02:43:52Z

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

+  /// characters.
+  ///
+  /// See also:
+  ///  * [enableSmartQuotes]


you need a blank line after See also:, see our style guide. Last sentence of https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#leave-breadcrumbs-in-the-comments (this matters because the rendering is broken otherwise).

justinmc · 2019-12-05T00:22:34Z

Ready for re-review @LongCatIsLooong @GaryQian.

Now I'm using an enum instead of a bool to allow for more flexibility in the future. Some things to note:

~~The enum is passed to the engine as a string (i.e. "SmartQuotesType.enabled"). Is there a better way to do that?~~ Using an index as suggested by Hixie.
UITextSmartDashesTypeDefault is no longer possible to set, since it doesn't really offer any difference from Yes for Flutter. We can change this in the future if we need to support finer grained control.
Any idea where I can declare the enums so that they're available to the user and to the necessary files? I just got a dependency loop error from the analyzer after trying to put them in widgets/basic.dart. Declaring inside the text input service and re-exporting elsewhere worked.

Hixie · 2019-12-05T00:34:58Z

You can get the index for the enum value by doing .index on the enum value, that would let you pass an int instead of a string.

Hixie · 2019-12-05T00:36:54Z

Re where to define them, define them in the lowest-level library that needs them (likely services), then reexport them from any library that exposes them (cupertino/text_field.dart, material/text_field.dart, widgets/editable_text.dart, etc).

LongCatIsLooong · 2019-12-10T17:52:02Z

packages/flutter/lib/src/material/text_field.dart

    properties.add(DiagnosticsProperty<bool>('autofocus', autofocus, defaultValue: false));
    properties.add(DiagnosticsProperty<bool>('obscureText', obscureText, defaultValue: false));
    properties.add(DiagnosticsProperty<bool>('autocorrect', autocorrect, defaultValue: true));
+    properties.add(DiagnosticsProperty<SmartDashesType>('smartDashesType', smartDashesType, defaultValue: obscureText ? SmartDashesType.disabled : SmartDashesType.enabled));


nit: I think there's an EnumProperty class

magicleon94 · 2020-04-27T14:06:55Z

Any idea on when this is gonna hit the stable channel?
Are there any workarounds to get it working on stable meanwhile?

justinmc · 2020-04-27T15:31:54Z

It looks like it's in v1.16.3 and above, so it's in dev and beta now, and should be in the next stable release.

If you need this now but can't use dev or beta, you could cherry pick the merge commit c06bf65 into your local copy of Flutter. Not an official way of doing things but it should work.

enableSmartQuotes and enableSmartDashes proof of concept with engine …

b3b702a

…changes

justinmc added a: text input Entering text in a text field or keyboard related problems platform-ios iOS applications specifically framework flutter/packages/flutter repository. See also f: labels. work in progress; do not review labels Nov 14, 2019

justinmc self-assigned this Nov 14, 2019

fluttergithubbot added the f: material design flutter/packages/flutter/material repository. label Nov 14, 2019

googlebot added the cla: yes label Nov 14, 2019

justinmc mentioned this pull request Nov 14, 2019

Smart quote/dash configuration support in iOS flutter/engine#13863

Merged

justinmc added 4 commits November 15, 2019 10:24

Set defaults correctly

97c239d

Support in CupertinoTextField and TextFormField too

b3769f4

Pass through to engine

fa21d73

Tests

c7b0e80

justinmc changed the title ~~WIP iOS smart quote/dash configuration~~ iOS smart quote/dash configuration Nov 15, 2019

fluttergithubbot added the f: cupertino flutter/packages/flutter/cupertino repository label Nov 15, 2019

justinmc removed the work in progress; do not review label Nov 15, 2019

justinmc requested review from GaryQian and LongCatIsLooong November 15, 2019 18:51

Documentation

7ea5cde

GaryQian reviewed Nov 15, 2019

View reviewed changes

justinmc added 2 commits November 18, 2019 09:07

Fix failing tests

9874088

Docs improvement per code review

aa37552

LongCatIsLooong approved these changes Nov 20, 2019

View reviewed changes

Hixie reviewed Dec 4, 2019

View reviewed changes

justinmc added 4 commits December 4, 2019 13:42

Code review

30de5e4

enum instead of bool

c487976

Tests updated with enum

207ca45

Docs formatting

ca7891f

Default to 'Yes' instead of 'Default'

b11b119

justinmc added 4 commits December 6, 2019 09:43

Use an index instead of a string for the enum value

945e76e

Declare enums in service and re-export where needed by user

9d2af26

EditableText test uses index instead of enum string

e11cf3a

Enum values documented

c01decc

justinmc force-pushed the smart-quotes-dashes branch from d3dac89 to c01decc Compare December 10, 2019 17:06

LongCatIsLooong approved these changes Dec 10, 2019

View reviewed changes

justinmc added 3 commits December 10, 2019 13:06

EnumProperty instead of DiagnosticsProperty

05071c9

Fix test after switch to enumProperty

28b2220

Merge branch 'master' into smart-quotes-dashes

fddd122

justinmc added the waiting for tree to go green label Dec 13, 2019

justinmc merged commit c06bf65 into flutter:master Dec 13, 2019

justinmc deleted the smart-quotes-dashes branch December 13, 2019 22:42

justinmc mentioned this pull request Dec 13, 2019

Obfuscated text fields should disable the IME autocorrect (iOS) #26684

Closed

github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021

	/// Defaults to true, unless obscureText is true, when it defaults to false.
	/// Defaults to true, unless [obscureText] is true, when it defaults to false.

iOS smart quote/dash configuration #44923

iOS smart quote/dash configuration #44923

Uh oh!

Conversation

justinmc commented Nov 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Uh oh!

fluttergithubbot commented Nov 14, 2019

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!

LongCatIsLooong Nov 20, 2019 • 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!

justinmc commented Dec 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hixie commented Dec 5, 2019

Uh oh!

Hixie commented Dec 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

magicleon94 commented Apr 27, 2020

Uh oh!

justinmc commented Apr 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

justinmc commented Nov 14, 2019 •

edited

Loading

LongCatIsLooong Nov 20, 2019 •

edited

Loading

justinmc commented Dec 5, 2019 •

edited

Loading