-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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
base: master
Are you sure you want to change the base?
Conversation
This allows overriding the default behavior of highlighting all the text on focus when using web or desktop
There was a problem hiding this 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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
@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!! |
There was a problem hiding this 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; |
There was a problem hiding this comment.
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
@justinmc No worries! I realize that you have probably a ton of stuff going on 😆 |
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.