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

Add Option to disable full selection on focus on TextField, TestFormField, and EditableText #163491

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

camfrandsen
Copy link

@camfrandsen camfrandsen commented Feb 17, 2025

This allows overriding the default behavior of highlighting all the text
on focus when using web or desktop

This is adding a field to EditableText, TextField, and TextFormField to disable highlighting the entire field on web and desktop when it receives focus. The field is called highlightAllOnFocus. It does nothing on other platforms because the other platforms don't highlight the entire field on focus.

Note: I am not attached to this variable name, but it was the best I could think of... But I am very open to better suggestions 😆

Thank you for everything!

Issue: #163399

Pre-launch Checklist

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

This allows overriding the default behavior of highlighting all the text
 on focus when using web or desktop
@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Feb 17, 2025
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.

Thanks for jumping in and submitting a PR for this. Some questions here. I always hesitate to add a new parameter to TextField, so I want to make sure we get it right.

When the user dismisses the modal, we want the focus to go back the text field with the same selection that it had before.

What platform is this on? Is this the same behavior that you would expect if you tab into that field?

Some other possible paths here for completeness:

  • Keep the boolean parameter as-is in this PR, but set it to a default value that considers the current platform.
  • Somehow give the user more generic control over what happens when the user focuses the field (make the parameter a function instead of a bool?).

/// web or desktop
///
/// By default this will highlight the text field on web and desktop, and can
/// only be turned off on those two platforms
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing a period at the end of the sentence here and above.

@@ -1792,6 +1793,15 @@ class EditableText extends StatefulWidget {
/// {@endtemplate}
bool get selectionEnabled => enableInteractiveSelection;

/// {@template flutter.widgets.editableText.highlightAllOnFocus}
/// Whether or not this field should highlight all text when gaining focus on
/// web or desktop
Copy link
Contributor

@justinmc justinmc Feb 26, 2025

Choose a reason for hiding this comment

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

The special case for web and desktop makes me hesitate here. I know that's how it works now, though. What happens in a native iOS or Android app if you connect a hardware keyboard and use the tab key to move between fields?

@camfrandsen
Copy link
Author

@justinmc Yeah! Thank you for taking the time to help me with this! The last thing I want to do is mess up flutter 😆 So I appreciate the questions!!
For us, this is a web only application. Which normally, we would want it to select all the text, but unfortunately in a few cases we don't... And for us, if they were to tab into these certain fields we also wouldn't want it to select everything.
Having said that, I love the idea of making it a function. It give a lot for flexibility in what would happen. I can make that change. Would I just call it onFocusChange similar to a focus node?
Actually, clarification question. Do you think it would be a void Function() that would be called instead of setting selection (if not null) or should it just be a TextSelection Function()?

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.

@camfrandsen Sorry for the slow response (vacation). Have you had any luck with the function? I'm thinking of something that would completely replace _adjustedSelectionWhenFocused, but that might not be practical since _adjustedSelectionWhenFocused uses private members currently. But would that kind of approach work for your use case or is it not doable?

Otherwise, what if your existing boolean parameter defaulted to something like this?

  static bool get defaultHighlightAllOnFocus {
    if (kIsWeb) {
      return true;
    }
    return switch (defaultTargetPlatform) {
      TargetPlatform.android => false,
      TargetPlatform.iOS => false,
      TargetPlatform.fuchsia => false,
      TargetPlatform.linux => true,
      TargetPlatform.macOS => true,
      TargetPlatform.windows => true,
    };
  }

Then we could remove the line about highlightAllOnFocus only applying to web and desktop? I'm just trying to make sure that this parameter makes sense and doesn't have any strange behavior that's dependent on our current private approach.

/// By default this will highlight the text field on web and desktop, and can
/// only be turned off on those two platforms
/// {@endtemplate}
final bool highlightAllOnFocus;
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually this should probably be called selectAllOnFocus.

setCustomSelectionOnFocus allows developers to set the selection when the text field gets focus
@camfrandsen
Copy link
Author

@justinmc No worries! I realize that you have probably a ton of stuff going on 😆
So, I personally love the callback idea! It allows for a lot of customization but keeps the API simple. They don't really need access to the private variables since you can access them through the public getters.
I pushed up my attempt at it. Let me know what you think 😄

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 f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants