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

Default TextField styles result in cursor overlapping labelText #141354

Closed
parlough opened this issue Jan 11, 2024 · 7 comments · Fixed by #146488
Closed

Default TextField styles result in cursor overlapping labelText #141354

parlough opened this issue Jan 11, 2024 · 7 comments · Fixed by #146488
Assignees
Labels
a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. found in release: 3.16 Found to occur in 3.16 found in release: 3.18 Found to occur in 3.18 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P1 High-priority issues at the top of the work list r: fixed Issue is closed as already fixed in a newer version team-design Owned by Design Languages team triaged-design Triaged by Design Languages team

Comments

@parlough
Copy link
Member

parlough commented Jan 11, 2024

Last update: #141354 (comment)


Steps to reproduce

  1. Have M3 enabled and a non-outlined TextField with a labelText specified
  2. Run the app (I tried macOS, CanvasKit, and skwasm)
  3. Enter the text field and start typing
  4. Notice the cursor overlaps the label text

Expected results

When typing in a TextField with labelText, the default styling won't result in the cursor overlapping the label text.

The M3 spec showcases the cursor as much thinner and shorter, preventing this from occurring.

Actual results

The cursor overlaps the label text, making it potentially harder to read.

Code sample

Code sample
import 'package:flutter/material.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        body: Center(
          child: SizedBox(
            width: 300,
            child: TextField(
              decoration: InputDecoration(
                labelText: 'Label text',
                filled: true,
              ),
            ),
          ),
        ),
      ),
    );
  }
}

Screenshots

Screenshot TextField with cursor overlapping label text

Flutter version info

Version
Flutter 3.19.0-3.0.pre.43 • channel main • https://github.com/flutter/flutter
Framework • revision f961fdf2ba (4 hours ago) • 2024-01-10 17:37:07 -0500
Engine • revision a045134c91
Tools • Dart 3.4.0 (build 3.4.0-14.0.dev) • DevTools 2.31.0
@dam-ease
Copy link

dam-ease commented Jan 11, 2024

Thanks for filing this!
I can reproduce this on the latest master and stable channels on macOS and Web. This doesn't overlap on mobile.

Mobile MacOS Web

✅ - Reproduces ❌ - Doesn't reproduce

useMaterial3: false useMaterial3: true
Screenshot 2024-01-11 at 13 42 44 Screenshot 2024-01-11 at 13 35 43
stable, master flutter doctor -v

[!] Flutter (Channel stable, 3.16.6, on macOS 14.2.1 23C71 darwin-arm64, locale
    en-NG)
    • Flutter version 3.16.6 on channel stable at
      /Users/damilolaalimi/sdks/flutter
    ! Warning: `dart` on your path resolves to
      /opt/homebrew/Cellar/dart/3.1.5/libexec/bin/dart, which is not inside your
      current Flutter SDK checkout at /Users/damilolaalimi/sdks/flutter.
      Consider adding /Users/damilolaalimi/sdks/flutter/bin to the front of your
      path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 46787ee49c (32 hours ago), 2024-01-09 14:36:07 -0800
    • Engine revision 3f3e560236
    • Dart version 3.2.3
    • DevTools version 2.28.4
    • If those were intentional, you can disregard the above warnings; however
      it is recommended to use "git" directly to perform update checks and
      upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/damilolaalimi/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • ANDROID_HOME = /Users/damilolaalimi/Library/Android/sdk
    • Java binary at: /Applications/Android
      Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build
      17.0.6+0-17.0.6b802.4-9586694)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 15C65
    • CocoaPods version 1.14.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build
      17.0.6+0-17.0.6b802.4-9586694)

[!] Android Studio (version unknown)
    • Android Studio at /Users/damilolaalimi/Downloads/Android Studio
      Preview.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    ✗ Unable to determine Android Studio version.
    • Java version OpenJDK Runtime Environment (build
      17.0.7+0-17.0.7b1000.6-10550314)

[✓] VS Code (version 1.85.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.78.0

[✓] VS Code (version 1.83.1)
    • VS Code at /Users/damilolaalimi/Downloads/Visual Studio Code.app/Contents
    • Flutter extension version 3.78.0

[✓] Connected device (5 available)
    • sdk gphone64 arm64 (mobile) • emulator-5554                        •
      android-arm64  • Android 14 (API 34) (emulator)
    • Damilola’s iPhone (mobile)  • 00008110-001964480AE1801E            • ios
      • iOS 17.1.1 21B91
    • iPhone 15 (mobile)          • F17D2919-6D61-4295-8408-1719692FE958 • ios
      • com.apple.CoreSimulator.SimRuntime.iOS-17-0 (simulator)
    • macOS (desktop)             • macos                                •
      darwin-arm64   • macOS 14.2.1 23C71 darwin-arm64
    • Chrome (web)                • chrome                               •
      web-javascript • Google Chrome 120.0.6099.216

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 2 categories.
[!] Flutter (Channel master, 3.19.0-3.0.pre.50, on macOS 14.2.1 23C71 darwin-arm64, locale en-NG)
    • Flutter version 3.19.0-3.0.pre.50 on channel master at /Users/damilolaalimi/fvm/versions/master
    ! Warning: `flutter` on your path resolves to /Users/damilolaalimi/sdks/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/damilolaalimi/fvm/versions/master. Consider adding /Users/damilolaalimi/fvm/versions/master/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /opt/homebrew/Cellar/dart/3.1.5/libexec/bin/dart, which is not inside your current Flutter SDK checkout at /Users/damilolaalimi/fvm/versions/master. Consider adding /Users/damilolaalimi/fvm/versions/master/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 3e5cad037c (3 hours ago), 2024-01-10 22:42:23 -0500
    • Engine revision ade9f18fa0
    • Dart version 3.4.0 (build 3.4.0-16.0.dev)
    • DevTools version 2.31.0
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/damilolaalimi/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • ANDROID_HOME = /Users/damilolaalimi/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 15C65
    • CocoaPods version 1.14.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2022.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)

[!] Android Studio (version unknown)
    • Android Studio at /Users/damilolaalimi/Downloads/Android Studio Preview.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    ✗ Unable to determine Android Studio version.
    • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314)

[✓] VS Code (version 1.85.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.78.0

[✓] VS Code (version 1.83.1)
    • VS Code at /Users/damilolaalimi/Downloads/Visual Studio Code.app/Contents
    • Flutter extension version 3.78.0

[✓] Connected device (3 available)
    • Damilola’s iPhone (mobile) • 00008110-001964480AE1801E • ios            • iOS 17.1.1 21B91
    • macOS (desktop)            • macos                     • darwin-arm64   • macOS 14.2.1 23C71 darwin-arm64
    • Chrome (web)               • chrome                    • web-javascript • Google Chrome 120.0.6099.216

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 2 categories.
exit code 0

@dam-ease dam-ease 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. team-design Owned by Design Languages team found in release: 3.16 Found to occur in 3.16 found in release: 3.18 Found to occur in 3.18 has reproducible steps The issue has been confirmed reproducible and is ready to work on and removed in triage Presently being triaged by the triage team labels Jan 11, 2024
@justinmc justinmc added P1 High-priority issues at the top of the work list triaged-design Triaged by Design Languages team labels Jan 11, 2024
@bleroux bleroux self-assigned this Jan 17, 2024
@bleroux
Copy link
Contributor

bleroux commented Jan 25, 2024

FYI, since #141943 has landed, the rendering is a little less worse (not very visible but the cursor in no more inside the 'b' of 'label'):

Capture d’écran 2024-01-25 à 16 07 57

This is because the main problem was related to the label positioning not taking properly line height into account (the label text line height is 1.5 for M3, for M2 it was 1.0, with this recent PR label line height is forced to 1.0, which is not compliant with M3 spec, but gives a better result). I will see if it is possible to get a proper positioning for the label text without forcing line height to 1.0.

A secondary problem is that the cursor height is too much. This is because it defaults to preferredLineHeight, and with M3 the text inside a TextField also has a line height of 1.5. One way to somewhat mitigate this issue is to explicitly set cursor height (unfortunately there is no way for the moment to set this at a theme level and also the vertical alignement of the cursor does not seem right).

Updated code sample with cursor height sets to 18 :
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      home: Scaffold(
        body: Center(
          child: SizedBox(
            width: 300,
            child: TextField(
              cursorHeight: 18.0,
              decoration: InputDecoration(
                labelText: 'Label text',
                filled: true,
              ),
            ),
          ),
        ),
      ),
    );
  }
}

Which renders this way:

Capture d’écran 2024-01-25 à 16 09 30

@justinmc Do you know if there is some ongoing work related to adjusting the cursor position?

@bleroux

This comment was marked as outdated.

auto-submit bot pushed a commit that referenced this issue Feb 7, 2024
…igration (#142981)

## Description

This PR main purpose is to make progress on the M3 test migration for `InputDecorator` (see #139076).

Before this PR more than 80 of the 156 tests defined in `input_decorator_test.dart` fail on M3.
Migrating all those tests in one shot is not easy at all because many failures are related to wrong positionning due to M3 typography changes. Another reason is that several M3 specific changes are required in order to get a proper M3 compliant text field, for instance:
- #142972
- #141354

Most of the tests were relying on an helper function (`buildInputDecorator`) which had a `useMaterial3` parameter. Unfortunately when `useMaterial3: true `was passed to this function it forced `useMaterial3: false` at the top level but overrided it at a lower level, which was very misleading because people could assume that the tests are ok with M3 (but in fact they were run using M2 typography but have some M3 logic in use).
I considered various way to make this change and I finally decided to run all existing tests only on M2 for the moment. Next step will be to move most of those tests to M3. In this PR, I migrated two of these existing tests for illustration.

Because many of the existing tests are checking input decorator height, I think it would also make sense to fix #142972 first. That's why I choosed to include a fix to #142972 in this PR.

A M3 filled `TextField` on Android:

| Before this PR | After this PR |
|--------|--------|
| ![image](https://github.com/flutter/flutter/assets/840911/403225b7-4c91-4aee-b19c-0490447ae7e3) | ![image](https://github.com/flutter/flutter/assets/840911/e96cf786-a9f5-4e15-bcdd-078350ff1608) | 

## Related Issue

Fixes #142972
Related to #139076

## Tests

Updates many existing tests 
+ adds 2 tests related to the fix for #142972
+ adds 1 tests for the M3 migration
+ move 1 tests related to M3
@justinmc

This comment was marked as outdated.

@bleroux bleroux removed their assignment Mar 5, 2024
@bleroux
Copy link
Contributor

bleroux commented Mar 5, 2024

Unassigning myself for the moment as there is no consensus on a valid solution.

Summary of this issue: on Apple platforms, the cursor height is 26 for a default Material 3 text field (font size is 16, text field height is 24, as specified on M3 spec).

Why the current cursor height is 26 on Apple platforms? :

  • Cursor height defaults to preferredLineHeight (which is fontSize * lineHeight).
  • Material 3 default text style for TextField has a font size of 16 and a line height of 1.5.
  • Cursor height is increased by 2 on iOS and macOS.

In order to make progress we should answer the following questions:

  1. Is this cursor height wrong or WAI?
  2. Does setting default cursor height to preferredLineHeight is something we are willing to change?
  3. Does cursor height and selection highlight height should be the same?

For reference, a video illustrating current default cursor height and default selection highlight height:

Enregistrement.de.l.ecran.2024-02-29.a.17.32.43.mov

@bleroux
Copy link
Contributor

bleroux commented Apr 9, 2024

While migrating InputDecorator tetss, I found that in addition to the cursor being too tall on Apple platforms, the label is wrongly posiitoned on desktop (Visual density is ignored).

Filed #146488.

auto-submit bot pushed a commit that referenced this issue Apr 9, 2024
## Description

This PRs makes the label vertical position depend on visual density for filled text field.
Previously, for M2 and M3, the label vertical offset was always the same (12 on M2, 8 and M3) despite different visual density configuration.
This was noticable on desktop and can lead to weird rendering especially on M3 where line height makes the cursor taller.

Screenshots for a filled text field:

| | Before | After |
|--------|--------|--------|
|M3 macOs| ![image](https://github.com/flutter/flutter/assets/840911/bd9bdb6e-477c-4db0-ae8f-74e18d19f29e) | ![image](https://github.com/flutter/flutter/assets/840911/25e59c44-0139-4813-be28-472302d6970e) | 
|M2 macOs| ![image](https://github.com/flutter/flutter/assets/840911/1c52493b-b17b-407b-93cc-69120207b716) | ![image](https://github.com/flutter/flutter/assets/840911/1fc4a8b6-429b-476c-b5bf-ff2934bf5293) | 

</details> 

## Related Issue

Fixes #141354

## Tests

Adds 2 tests, updates 2 tests.
@bleroux bleroux added the r: fixed Issue is closed as already fixed in a newer version label Apr 9, 2024
gilnobrega pushed a commit to gilnobrega/flutter that referenced this issue Apr 22, 2024
## Description

This PRs makes the label vertical position depend on visual density for filled text field.
Previously, for M2 and M3, the label vertical offset was always the same (12 on M2, 8 and M3) despite different visual density configuration.
This was noticable on desktop and can lead to weird rendering especially on M3 where line height makes the cursor taller.

Screenshots for a filled text field:

| | Before | After |
|--------|--------|--------|
|M3 macOs| ![image](https://github.com/flutter/flutter/assets/840911/bd9bdb6e-477c-4db0-ae8f-74e18d19f29e) | ![image](https://github.com/flutter/flutter/assets/840911/25e59c44-0139-4813-be28-472302d6970e) | 
|M2 macOs| ![image](https://github.com/flutter/flutter/assets/840911/1c52493b-b17b-407b-93cc-69120207b716) | ![image](https://github.com/flutter/flutter/assets/840911/1fc4a8b6-429b-476c-b5bf-ff2934bf5293) | 

</details> 

## Related Issue

Fixes flutter#141354

## Tests

Adds 2 tests, updates 2 tests.
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. found in release: 3.16 Found to occur in 3.16 found in release: 3.18 Found to occur in 3.18 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P1 High-priority issues at the top of the work list r: fixed Issue is closed as already fixed in a newer version team-design Owned by Design Languages team triaged-design Triaged by Design Languages team
Projects
Status: Done (PR merged)
4 participants