Skip to content

Update default selectionHeightStyle and selectionWidthStyle for EditableText - #167762

Merged
auto-submit[bot] merged 30 commits into
flutter:masterfrom
Renzo-Olivares:default-text-height-style
Jun 16, 2025
Merged

auto-submit[bot] merged 30 commits into
flutter:masterfrom
Renzo-Olivares:default-text-height-style

Conversation

@Renzo-Olivares

@Renzo-Olivares Renzo-Olivares commented Apr 24, 2025 •

Copy link
Copy Markdown
Contributor

This change updates the default selectionHeightStyle and selectionWidthStyle for EditableText to more closely match the behavior on native platforms.

/// The default value for [selectionHeightStyle].
///
/// On web platforms, this defaults to [ui.BoxHeightStyle.max].
///
/// On native platforms, this defaults to [ui.BoxHeightStyle.includeLineSpacingMiddle] for all
/// platforms.
static ui.BoxHeightStyle get defaultSelectionHeightStyle {
  if (kIsWeb) {
    return ui.BoxHeightStyle.max;
  }
  switch (defaultTargetPlatform) {
    case TargetPlatform.android:
    case TargetPlatform.fuchsia:
    case TargetPlatform.iOS:
    case TargetPlatform.linux:
    case TargetPlatform.macOS:
    case TargetPlatform.windows:
      return ui.BoxHeightStyle.includeLineSpacingMiddle;
  }
}

/// The default value for [selectionWidthStyle].
///
/// On web platforms, this defaults to [ui.BoxWidthStyle.max] for Apple platforms and
/// [ui.BoxWidthStyle.tight] for all others.
///
/// On non-web platforms, this defaults to [ui.BoxWidthStyle.max] for all
/// platforms.
static ui.BoxWidthStyle get defaultSelectionWidthStyle {
  if (kIsWeb) {
    if (defaultTargetPlatform == TargetPlatform.iOS || WebBrowserDetection.browserIsSafari) {
      return ui.BoxWidthStyle.max;
    }
    return ui.BoxWidthStyle.tight;
  }
  switch (defaultTargetPlatform) {
    case TargetPlatform.android:
    case TargetPlatform.fuchsia:
    case TargetPlatform.iOS:
    case TargetPlatform.linux:
    case TargetPlatform.macOS:
    case TargetPlatform.windows:
      return ui.BoxWidthStyle.max;
  }
}

Native Selection Style Behavior

Android Native Behavior

Component Type UI Framework / Component Image
Static Text Android TextView native_android_static_textview
Static Text Jetpack Compose Text native_android_static_compose
Static Text Chrome (Web) "p element" android_chrome_web
Text Field Android EditText native_android_textfield_edittext
Text Field Jetpack Compose TextField native_android_textfield_compose

iOS Native Behavior

Component Type UI Framework / Component Image
Text Field Native SwiftUI TextEditor native_ios_texteditor
Text Field Native SwiftUI TextField native_ios_textfield
Static Text Safari (Web) "p element" mobile_safari_web

macOS Native Behavior

Component Type UI Framework / Component Image
Static Text Native SwiftUI Text native_macos_static_text
Static Text Chrome (Web) "p element" chrome_web
Static Text Firefox (Web) "p element" firefox_web
Static Text Safari (Web) "p element" safari_web
Text Field Native SwiftUI TextEditor native_macos_texteditor
Text Field Native SwiftUI TextField native_macos_textfield

Flutter Selection Style Behavior

Flutter Before Behavior

This was the behavior on all platforms before this change.

Component Type Behavior Image
Text Field Flutter TextField flutter_before_text_field
Static Text Flutter SelectableText flutter_before_selectabletext

Flutter Chrome After Behavior

This is the behavior on all chrome web after this change.

Component Type Behavior Image
Static Text Flutter SelectableText in Chrome flutter_after_chrome_selectabletext
Text Field Flutter TextField in Chrome flutter_after_chrome_textfield

Flutter Safari After Behavior

This is the behavior on Safari after this change. (all iOS browsers, and Safari on macOS).

Component Type Behavior Image
Text Field Flutter TextField in Safari flutter_after_safari_textfield
Static Text Flutter SelectableText in Safari flutter_after_safari_selectabletext

Flutter Native After Behavior

This is the behavior on native platforms after this change.

Component Type Behavior Image
Static Text Flutter SelectableText on Native platforms flutter_after_native_selectabletext
Text Field Flutter TextField on Native platforms flutter_after_native_textfield

Fixes #68675

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].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

@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. p: material_ui material_ui package in flutter/packages p: cupertino_ui cupertino_ui package in flutter/packages labels Apr 24, 2025
@flutter-dashboard

Copy link
Copy Markdown

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #167762 at sha 0437620

@flutter-dashboard flutter-dashboard Bot added the will affect goldens Changes to golden files label Apr 25, 2025
@flutter-dashboard

Copy link
Copy Markdown

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #167762 at sha b0a86f0

@flutter-dashboard

Copy link
Copy Markdown

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #167762 at sha 21b08e8

this.cursorRadius = const Radius.circular(2.0),
this.cursorOpacityAnimates = true,
this.cursorColor,
this.selectionHeightStyle = ui.BoxHeightStyle.tight,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see this a lot in the framework where higher level widgets set their default to the same as their basic counterpart, i.e. TextField sets the same defaults as EditableText. Is this a pattern we want to be strict about? Can we just keep the defaults in EditableText and have TextField fall back on that. This PR does this for selectionHeightStyle and selectionWidthStyle, but i'm curious what others think.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This has come up a few times and I have softly decided to support setting the default everywhere. I just sent you some notes I wrote about it.

@loic-sharma loic-sharma May 15, 2025 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm it seems a drawback of omitting the default value here is that the property becomes nullable? It seems like a small usability win to have a non-nullable property if we set the default everywhere.

Also, not setting the default property also makes this particular change a breaking change. I'd lean towards avoid a breaking change if possible.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah but since EditableText.defaultSelectionHeightStyle and EditableText.defaultSelectionWidthStyle are non-const, we can't actually use them as default values in a const constructor? If that's the case, I'm OK with changing these to be null by default.

@Renzo-Olivares Renzo-Olivares May 20, 2025 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah it won't be possible to set the defaults because the selectionStyle getters are non-const. Even in the non-const constructors if the parameter is a named one its default value has to be const. Is this considered a breaking change because TextField().selectionHeightStyle/selectionWidthStyle now return null?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this considered a breaking change because TextField().selectionHeightStyle/selectionWidthStyle now return null?

Strictly speaking, yes.

One way to avoid this breaking change would be by making TextField().selectionHeightStyle/selectionWidthStyle getters that return EditableText.defaultSelectionWidthStyle/defaultSelectionWidthStyle instead of null. That feels a bit weird though, I suspect the breaking change is the better solution long-term.

@flutter-dashboard

Copy link
Copy Markdown

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #167762 at sha 7fd1f42

double devicePixelRatio = 1.0,
ui.BoxHeightStyle selectionHeightStyle = ui.BoxHeightStyle.tight,
ui.BoxWidthStyle selectionWidthStyle = ui.BoxWidthStyle.tight,
ui.BoxHeightStyle selectionHeightStyle = ui.BoxHeightStyle.max,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I set this to a reasonable default that wouldn't cause a line with runs of different heights to create "ledges".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm on board with that. I've seen the "ledges" problem come up before and wondered why we didn't do this in the first place. We can always undo it if it breaks something we're not thinking of.

composingRect = editable.getRectForComposingRange(const TextRange(start: 6, end: 7))!;
expect(composingRect, const Rect.fromLTRB(0.0, 14.0, 14.0, 28.0));
composingRect = editable.getRectForComposingRange(const TextRange(start: 7, end: 8))!; // H
expect(composingRect, const Rect.fromLTRB(14.0, 18.0, 24.0, 28.0));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This test change makes sense since now that the default is BoxHeightStyle.max, these rects have a height of the largest span on the line which are the WidgetSpans in this test with a height of 14.0.

@flutter-dashboard

Copy link
Copy Markdown

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #167762 at sha 9a92460

@Renzo-Olivares Renzo-Olivares changed the title Test changing default selectionHeightStyle Update default selectionHeightStyle for EditableText May 8, 2025
@flutter-dashboard

Copy link
Copy Markdown

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #167762 at sha 54c1043

@Renzo-Olivares

Copy link
Copy Markdown
Contributor Author

One style that doesn't seem possible using the currently available BoxHeightStyle is the firefox default. I was able to modify SkParagraph to achieve the behavior Renzo-Olivares/skia@375a285 , but it still needs some work. I'll try to tackle that change at a later time.

selection_height_style-max_expected

@Renzo-Olivares Renzo-Olivares changed the title Update default selectionHeightStyle for EditableText Update default selectionHeightStyle and selectionWidthStyle for EditableText May 15, 2025
// `_web_browser_detection.dart` for the web implementation.

/// A class that provides information about the current browser when on the web.
class WebBrowserDetection {

@Renzo-Olivares Renzo-Olivares May 15, 2025 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There might be a better home for this rather than in widgets.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the main purpose of creating this abstraction?

@loic-sharma loic-sharma May 15, 2025 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There might be a better home for this rather than in widgets.

Maybe services? Its library description makes it sound like a good fit: Platform services exposed to Flutter apps.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Another option might be foundation, that is where kIsWeb lives and defaultTargetPlatform.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What's the main purpose of creating this abstraction?

The main purpose for this class is because we can't directly import only dart:ui_web on web without a fallback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mdebbar do you have any thoughts on where this class should live?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I ended up just making it private for now, removing the public web_browser_detection.dart. The conditional import is now in editable_text.dart.

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See my comment about checking that we want to do browserIsSafari. Otherwise this LGTM.

this.cursorRadius = const Radius.circular(2.0),
this.cursorOpacityAnimates = true,
this.cursorColor,
this.selectionHeightStyle = ui.BoxHeightStyle.tight,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This has come up a few times and I have softly decided to support setting the default everywhere. I just sent you some notes I wrote about it.

double devicePixelRatio = 1.0,
ui.BoxHeightStyle selectionHeightStyle = ui.BoxHeightStyle.tight,
ui.BoxWidthStyle selectionWidthStyle = ui.BoxWidthStyle.tight,
ui.BoxHeightStyle selectionHeightStyle = ui.BoxHeightStyle.max,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm on board with that. I've seen the "ledges" problem come up before and wondered why we didn't do this in the first place. We can always undo it if it breaks something we're not thinking of.

Comment on lines +2090 to +2098
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.iOS:
case TargetPlatform.linux:
case TargetPlatform.macOS:
case TargetPlatform.windows:
return ui.BoxWidthStyle.max;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you get rid of this switch and just return it?

/// A class that provides information about the current browser when on the web.
class WebBrowserDetection {
/// Whether the current browser is webkit (Safari).
static bool get browserIsSafari => ui_web.BrowserDetection.instance.isSafari;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this would be the first time we are using BrowserDetection in the framework? Maybe we should check with someone from the web team before we do this.

Makes sense that we need to do this in order to get the styling right, though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CC @mdebbar from triage

Can we do feature detection?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I misunderstood the PR during triage. It looks like the purpose of the PR is to choose a different for specific platforms, so checking for iOS and/or isSafari is the way to go here.

Feature detection is for when you need to use a certain feature that you know only exists in a specific browser. It's better to check the existence of the feature at runtime, rather than just check for what browser you're running on.


/// A class that provides information about the current browser when on the web.
class WebBrowserDetection {
/// Whether the current browser is webkit (Safari).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we explain that this throws if called on non-web platforms?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alternatively, could we make this return false? We know the dart:io implementation won't be used on Safari, right?

@Renzo-Olivares Renzo-Olivares May 20, 2025 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, false sounds like a reasonable default!, yea the dart:io implementation is only used when not on the web.

// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

export '_web_browser_detection_io.dart' if (dart.library.js_util) '_web_browser_detection.dart';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't realize this was a pattern we did in the framework but looks like we do it in a couple of places already 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Renzo-Olivares in order for this conditional import to work properly everywhere, the 2nd one needs to be renamed to _web_browser_detection_web.dart (the files basically have to suffixed by _io.dart and _web.dart).

// `_web_browser_detection.dart` for the web implementation.

/// A class that provides information about the current browser when on the web.
class WebBrowserDetection {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the main purpose of creating this abstraction?

/// A class that provides information about the current browser when on the web.
class WebBrowserDetection {
/// Whether the current browser is webkit (Safari).
static bool get browserIsSafari => throw UnimplementedError();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What happens if you try to call ui_web.BrowserDetection.instance.isSafari on a non-web platform?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The dart:ui_web library is only available on web and can't be compiled for a non-web platform as far as I know. So it has to be imported conditionally on the web only (this is why Renzo had to have _web_browser_detection_io.dart and _web_browser_detection.dart).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You can't import ui_web on a non-web platform, you get a compile time error.

error: ../../flutter/packages/flutter/lib/src/widgets/editable_text.dart:20:8: Error: Dart library 'dart:ui_web' is not available on this platform.
import 'dart:ui_web' as ui_web;
       ^
Context: The unavailable library 'dart:ui_web' is imported through these packages:

    package:selectionplayground => package:flutter => dart:ui_web

@flutter-dashboard

Copy link
Copy Markdown

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #167762 at sha b1560ee

/// A class that provides information about the current browser when on the web.
class WebBrowserDetection {
/// Whether the current browser is webkit (Safari).
static bool get browserIsSafari => ui_web.BrowserDetection.instance.isSafari;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: WebBrowserDetection.browserIsSafari feels a bit redundant, what do you think of renaming browserIsSafari to isSafari? I don't feel strongly about this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sgtm

@loic-sharma loic-sharma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM assuming we can't switch to feature detection instead of browser detection. Excellent PR description!

/// A class that provides information about the current browser when on the web.
class WebBrowserDetection {
/// Whether the current browser is webkit (Safari).
static bool get browserIsSafari => throw UnimplementedError();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The dart:ui_web library is only available on web and can't be compiled for a non-web platform as far as I know. So it has to be imported conditionally on the web only (this is why Renzo had to have _web_browser_detection_io.dart and _web_browser_detection.dart).

// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

export '_web_browser_detection_io.dart' if (dart.library.js_util) '_web_browser_detection.dart';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Renzo-Olivares in order for this conditional import to work properly everywhere, the 2nd one needs to be renamed to _web_browser_detection_web.dart (the files basically have to suffixed by _io.dart and _web.dart).

/// platforms.
static ui.BoxWidthStyle get defaultSelectionWidthStyle {
if (kIsWeb) {
if (defaultTargetPlatform == TargetPlatform.iOS || WebBrowserDetection.isSafari) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is fine, but I'm curious if there's a reason you added the isSafari check? I would think TargetPlatform.iOS is enough. Is that not the case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

isSafari is mainly for the macOS case, since the behaviors differ between macOS chrome and macOS Safari.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm sure someone will see this code and will be curious in the future 🙂 Can you add a comment in the code?

/// A class that provides information about the current browser when on the web.
class WebBrowserDetection {
/// Whether the current browser is webkit (Safari).
static bool get browserIsSafari => ui_web.BrowserDetection.instance.isSafari;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I misunderstood the PR during triage. It looks like the purpose of the PR is to choose a different for specific platforms, so checking for iOS and/or isSafari is the way to go here.

Feature detection is for when you need to use a certain feature that you know only exists in a specific browser. It's better to check the existence of the feature at runtime, rather than just check for what browser you're running on.

@flutter-dashboard

Copy link
Copy Markdown

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #167762 at sha d22f2a1

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 19, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 19, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 19, 2025
mboetger pushed a commit to mboetger/flutter that referenced this pull request Jul 21, 2025
…EditableText` (flutter#167762)

This change updates the default `selectionHeightStyle` and
`selectionWidthStyle` for `EditableText` to more closely match the
behavior on native platforms.

```dart
/// The default value for [selectionHeightStyle].
///
/// On web platforms, this defaults to [ui.BoxHeightStyle.max].
///
/// On native platforms, this defaults to [ui.BoxHeightStyle.includeLineSpacingMiddle] for all
/// platforms.
static ui.BoxHeightStyle get defaultSelectionHeightStyle {
  if (kIsWeb) {
    return ui.BoxHeightStyle.max;
  }
  switch (defaultTargetPlatform) {
    case TargetPlatform.android:
    case TargetPlatform.fuchsia:
    case TargetPlatform.iOS:
    case TargetPlatform.linux:
    case TargetPlatform.macOS:
    case TargetPlatform.windows:
      return ui.BoxHeightStyle.includeLineSpacingMiddle;
  }
}

/// The default value for [selectionWidthStyle].
///
/// On web platforms, this defaults to [ui.BoxWidthStyle.max] for Apple platforms and
/// [ui.BoxWidthStyle.tight] for all others.
///
/// On non-web platforms, this defaults to [ui.BoxWidthStyle.max] for all
/// platforms.
static ui.BoxWidthStyle get defaultSelectionWidthStyle {
  if (kIsWeb) {
    if (defaultTargetPlatform == TargetPlatform.iOS || WebBrowserDetection.browserIsSafari) {
      return ui.BoxWidthStyle.max;
    }
    return ui.BoxWidthStyle.tight;
  }
  switch (defaultTargetPlatform) {
    case TargetPlatform.android:
    case TargetPlatform.fuchsia:
    case TargetPlatform.iOS:
    case TargetPlatform.linux:
    case TargetPlatform.macOS:
    case TargetPlatform.windows:
      return ui.BoxWidthStyle.max;
  }
}
```

## Native Selection Style Behavior

### Android Native Behavior

| Component Type | UI Framework / Component | Image |
|---|---|---|
| **Static Text** | Android TextView | <img height="400"
alt="native_android_static_textview"
src="https://github.com/user-attachments/assets/34da885c-c3d8-4ecc-baf5-65cd32850aa7"
/> |
| **Static Text** | Jetpack Compose Text | <img height="400"
alt="native_android_static_compose"
src="https://github.com/user-attachments/assets/ef916e8d-3627-4394-b5a1-aa59989ba826"
/> |
| **Static Text** | Chrome (Web) "p element" | <img height="400"
alt="android_chrome_web"
src="https://github.com/user-attachments/assets/b6537871-5d07-470b-bdee-ae0d222b2404"
/> |
| **Text Field** | Android EditText | <img height="400"
alt="native_android_textfield_edittext"
src="https://github.com/user-attachments/assets/ef91c6c3-eed0-4cf5-8b6f-182ecf8ce60b"
/> |
| **Text Field** | Jetpack Compose TextField | <img height="400"
alt="native_android_textfield_compose"
src="https://github.com/user-attachments/assets/891c28ba-66f7-4923-a1d3-839cad3b039c"
/> |

### iOS Native Behavior

| Component Type | UI Framework / Component | Image |
|---|---|---|
| **Text Field** | Native SwiftUI TextEditor | <img height="400"
alt="native_ios_texteditor"
src="https://github.com/user-attachments/assets/b99adb30-8cb3-454e-aa56-41f3cfadeeed"
/> |
| **Text Field** | Native SwiftUI TextField | <img height="400"
alt="native_ios_textfield"
src="https://github.com/user-attachments/assets/0544419b-b9a7-4641-930b-9a123f9a7720"
/> |
| **Static Text** | Safari (Web) "p element" | <img height="400"
alt="mobile_safari_web"
src="https://github.com/user-attachments/assets/6934ed80-4712-4000-bff0-4d5d2833abbe"
/> |

### macOS Native Behavior

| Component Type | UI Framework / Component | Image |
|---|---|---|
| **Static Text** | Native SwiftUI Text | <img height="400"
alt="native_macos_static_text"
src="https://github.com/user-attachments/assets/becf7690-878e-4cf1-b9d7-49725d58fd42"
/> |
| **Static Text** | Chrome (Web) "p element" | <img height="400"
alt="chrome_web"
src="https://github.com/user-attachments/assets/bf38e1c0-7bd6-42b7-8230-71b8365ac1d7"
/> |
| **Static Text** | Firefox (Web) "p element" | <img height="400"
alt="firefox_web"
src="https://github.com/user-attachments/assets/54817628-690b-4243-8bb0-0d94afaf5013"
/> |
| **Static Text** | Safari (Web) "p element" | <img height="400"
alt="safari_web"
src="https://github.com/user-attachments/assets/08ae8a6e-8ea5-4cc5-84f8-49c5f759e910"
/> |
| **Text Field** | Native SwiftUI TextEditor | <img height="400"
alt="native_macos_texteditor"
src="https://github.com/user-attachments/assets/43995c72-acf5-4ef0-99d7-faa3940b16e7"
/> |
| **Text Field** | Native SwiftUI TextField | <img
alt="native_macos_textfield"
src="https://github.com/user-attachments/assets/d5606b23-dff0-47da-a1e8-b1c71047c426"
/> |

## Flutter Selection Style Behavior

### Flutter Before Behavior

This was the behavior on all platforms before this change.

| Component Type | Behavior | Image |
|---|---|---|
| **Text Field** | Flutter TextField | <img height="400"
alt="flutter_before_text_field"
src="https://github.com/user-attachments/assets/014bfc9d-0bbe-49ee-9230-9da80addc675"
/> |
| **Static Text** | Flutter SelectableText | <img height="400"
alt="flutter_before_selectabletext"
src="https://github.com/user-attachments/assets/251956c6-4d7d-4d3e-802a-33718f30cbf3"
/> |

### Flutter Chrome After Behavior

This is the behavior on all chrome web after this change.

| Component Type | Behavior | Image |
|---|---|---|
| **Static Text** | Flutter SelectableText in Chrome | <img height="400"
alt="flutter_after_chrome_selectabletext"
src="https://github.com/user-attachments/assets/167ee998-c9c4-4367-ab77-149514fb2b15"
/> |
| **Text Field** | Flutter TextField in Chrome | <img height="400"
alt="flutter_after_chrome_textfield"
src="https://github.com/user-attachments/assets/76b123df-c818-4304-afb8-7034506e2031"
/> |

### Flutter Safari After Behavior

This is the behavior on Safari after this change. (all iOS browsers, and
Safari on macOS).

| Component Type | Behavior | Image |
|---|---|---|
| **Text Field** | Flutter TextField in Safari | <img height="400"
alt="flutter_after_safari_textfield"
src="https://github.com/user-attachments/assets/c37e80cf-d89a-43aa-baff-f32e94f39b56"
/> |
| **Static Text** | Flutter SelectableText in Safari | <img height="400"
alt="flutter_after_safari_selectabletext"
src="https://github.com/user-attachments/assets/c1a11d76-afb9-4265-af54-e20e183839e1"
/> |

### Flutter Native After Behavior

This is the behavior on native platforms after this change.

| Component Type | Behavior | Image |
|---|---|---|
| **Static Text** | Flutter SelectableText on Native platforms | <img
height="400" alt="flutter_after_native_selectabletext"
src="https://github.com/user-attachments/assets/5506eb4c-13ed-4c04-b680-9d097f20b159"
/> |
| **Text Field** | Flutter TextField on Native platforms | <img
height="400" alt="flutter_after_native_textfield"
src="https://github.com/user-attachments/assets/e65bba53-34d3-480c-9293-d597b587fe70"
/> |

Fixes flutter#68675

## Pre-launch Checklist

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

---------

Co-authored-by: Renzo Olivares <[email protected]>
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025
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 framework flutter/packages/flutter repository. See also f: labels. p: cupertino_ui cupertino_ui package in flutter/packages p: material_ui material_ui package in flutter/packages will affect goldens Changes to golden files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SelectableText] inconsistent selection background color between different character encoding

4 participants