TextField border gap padding improvement #92940

justinmc · 2021-11-02T22:39:39Z

Previously, the calculations for the border gap weren't quite right in all cases, and would sometimes result in extra unwanted padding. Here's an example where gapPadding is set to zero, meaning that the border should just touch the label, but it didn't:

          child: TextField(
            decoration: InputDecoration(
              labelText: 'Hello label',
              border: OutlineInputBorder(
                borderRadius: BorderRadius.circular(32.0),
                gapPadding: 0.0,
             ),
            ),
          ),

With this PR, the same code gives the correct result:

Todos

Test.
Make sure the current math doesn't break any other cases.
Check other code paths and make sure they don't need their math updated as well.

Related issues

Fixes #82321
Old PR that may have caused this bug: #34515

… doesnt break other cases

justinmc · 2021-11-02T22:46:16Z

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

@@ -418,12 +418,11 @@ class OutlineInputBorder extends InputBorder {

    const double cornerArcSweep = math.pi / 2.0;
    final double tlCornerArcSweep = start < scaledRRect.tlRadiusX
-      ? math.asin((start / scaledRRect.tlRadiusX).clamp(-1.0, 1.0))
+      ? math.acos(1 - start / scaledRRect.tlRadiusX)


Here's the PR where this line originally came from: #34515. The new calculation appears to work for that case as well.

A suggestion

final double tlCornerArcSweep = math.acos((1 - start / scaledRRect.tlRadiusX).clamp(0.0, 1.0));

Thanks, I was worried about not having the clamp in there anymore. Will try it now.

justinmc · 2021-11-02T22:47:22Z

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

@@ -432,7 +431,7 @@ class OutlineInputBorder extends InputBorder {
    const double trCornerArcSweep = cornerArcSweep;
    if (start + extent < scaledRRect.width - scaledRRect.trRadiusX) {
      path
-        ..relativeMoveTo(extent, 0.0)
+        ..moveTo(scaledRRect.left + start + extent, scaledRRect.top)


This change and the previous changes near line 425 make the gap the correct width. Before, it was too wide.

skia-gold · 2021-11-02T22:51:25Z

Gold has detected about 2 new digest(s) on patchset 1.
View them at https://flutter-gold.skia.org/cl/github/92940

justinmc · 2021-11-03T17:56:26Z

The broken goldens are practically unnoticeable 1 pixel differences, so that's good. I'll plan to just accept those once I finalize this PR. I should also run the Google tests on this and watch out for breaking other visual diff tests.

werainkhatri · 2021-11-03T18:06:00Z

I have raised a pull request on your fork with the working solution I wrote. See if that help 😬.

skia-gold · 2021-11-03T18:11:24Z

Gold has detected about 2 new digest(s) on patchset 3.
View them at https://flutter-gold.skia.org/cl/github/92940

Correct math for top left and top right border corners

skia-gold · 2021-11-04T19:16:25Z

Gold has detected about 4 new digest(s) on patchset 6.
View them at https://flutter-gold.skia.org/cl/github/92940

skia-gold · 2021-11-04T23:01:24Z

Gold has detected about 12 new digest(s) on patchset 8.
View them at https://flutter-gold.skia.org/cl/github/92940

skia-gold · 2021-11-12T23:29:20Z

Gold has detected about 12 new digest(s) on patchset 9.
View them at https://flutter-gold.skia.org/cl/github/92940

skia-gold · 2021-11-13T00:29:20Z

Gold has detected about 1 new digest(s) on patchset 10.
View them at https://flutter-gold.skia.org/cl/github/92940

flutter-dashboard · 2021-11-13T01:08:50Z

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 #92940 at sha 4224a9f

justinmc · 2021-11-15T21:23:10Z

I've accepted the goldens changes, which as I expected were just two unnoticeable 1 pixel color changes plus the new golden tests I created.

justinmc · 2021-11-15T22:53:52Z

I ran the Google test and got a few failures, but they were all the same as the goldens: a pixel or two that's a slightly different color. I'll change it to pass and plan to accept the changes after merge.

LongCatIsLooong · 2021-11-17T00:45:05Z

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

-      ? math.asin((start / scaledRRect.tlRadiusX).clamp(-1.0, 1.0))
-      : math.pi / 2.0;
+    final double tlCornerArcSweep = math.acos(
+      (1 - start / scaledRRect.tlRadiusX).clamp(0.0, 1.0),


question: is it guaranteed that the radiusX can't be 0?

Good catch. I just tried this out and it can in fact be zero, but the clamp will make sure that it gets back on track and the behavior is correct. No errors are thrown.

TIL! So NaN.clamp(0, 1) == 1? But shouldn't the 1 - NaN be evaluated first and throw an error?

turns out, 1 - NaN = -NaN. So it finally clamps to 0.0.

And 1/0 is actually Infinity in Dart and not NaN, so it makes a little more sense.

This reverts commit e4b78ff.

…93925)" This reverts commit c82e78d.

Quick touch up on border gap math for typical case, need to make sure…

ea9883b

… doesnt break other cases

justinmc self-assigned this Nov 2, 2021

flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Nov 2, 2021

google-cla bot added the cla: yes label Nov 2, 2021

justinmc commented Nov 2, 2021

View reviewed changes

justinmc mentioned this pull request Nov 2, 2021

TextField with rounded corners - floating label shows extra whitespace #82321

Closed

fixes gap calculation for ltr and rtl directions

a1f9afd

justinmc added 2 commits November 3, 2021 10:59

Comment assumption

92f3c87

Clamp for things like huge radii

bc1678b

justinmc added 2 commits November 4, 2021 12:08

Merge branch 'label-border-gap' into patch-1

7b19851

Merge pull request #5 from werainkhatri/patch-1

a950ca2

Correct math for top left and top right border corners

justinmc added 2 commits November 4, 2021 15:51

Line length

9bf62dd

Test point on border and add new goldens

8a86acd

Analyzer fix

c54e3ee

Fix bug causing failed goldens due to antialiasing in top of border

4224a9f

justinmc marked this pull request as ready for review November 13, 2021 00:38

justinmc requested a review from LongCatIsLooong November 13, 2021 00:38

flutter-dashboard bot added the will affect goldens Changes to golden files label Nov 13, 2021

justinmc mentioned this pull request Nov 15, 2021

Allow users to center align the floating label #90157

Merged

8 tasks

LongCatIsLooong approved these changes Nov 17, 2021

View reviewed changes

justinmc added the waiting for tree to go green label Nov 18, 2021

fluttergithubbot merged commit e4b78ff into flutter:master Nov 19, 2021

jmagman added a commit that referenced this pull request Nov 19, 2021

Revert "TextField border gap padding improvement (#92940)"

a69604f

This reverts commit e4b78ff.

jmagman mentioned this pull request Nov 19, 2021

Revert "TextField border gap padding improvement" #93925

Merged

jmagman added a commit that referenced this pull request Nov 19, 2021

Revert "TextField border gap padding improvement (#92940)" (#93925)

c82e78d

This reverts commit e4b78ff.

justinmc added a commit that referenced this pull request Nov 19, 2021

Revert "Revert "TextField border gap padding improvement (#92940)" (#…

4d4e939

…93925)" This reverts commit c82e78d.

justinmc mentioned this pull request Nov 19, 2021

[unrevert] TextField border gap padding improvement #93933

Merged

justinmc deleted the label-border-gap branch November 23, 2021 00:28

Piinks mentioned this pull request Nov 24, 2021

Add InputDecorator label color on error examples #93480

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TextField border gap padding improvement #92940

TextField border gap padding improvement #92940

justinmc commented Nov 2, 2021 •

edited

Loading

justinmc Nov 2, 2021

werainkhatri Nov 2, 2021 •

edited

Loading

justinmc Nov 3, 2021

justinmc Nov 2, 2021

skia-gold commented Nov 2, 2021

justinmc commented Nov 3, 2021

werainkhatri commented Nov 3, 2021

skia-gold commented Nov 3, 2021

skia-gold commented Nov 4, 2021

skia-gold commented Nov 4, 2021

skia-gold commented Nov 12, 2021

skia-gold commented Nov 13, 2021

flutter-dashboard bot commented Nov 13, 2021

justinmc commented Nov 15, 2021

justinmc commented Nov 15, 2021

LongCatIsLooong Nov 17, 2021 •

edited

Loading

justinmc Nov 18, 2021

LongCatIsLooong Nov 18, 2021

werainkhatri Nov 18, 2021

justinmc Nov 18, 2021

TextField border gap padding improvement #92940

TextField border gap padding improvement #92940

Conversation

justinmc commented Nov 2, 2021 • edited Loading

Todos

Related issues

justinmc Nov 2, 2021

Choose a reason for hiding this comment

werainkhatri Nov 2, 2021 • edited Loading

Choose a reason for hiding this comment

justinmc Nov 3, 2021

Choose a reason for hiding this comment

justinmc Nov 2, 2021

Choose a reason for hiding this comment

skia-gold commented Nov 2, 2021

justinmc commented Nov 3, 2021

werainkhatri commented Nov 3, 2021

skia-gold commented Nov 3, 2021

skia-gold commented Nov 4, 2021

skia-gold commented Nov 4, 2021

skia-gold commented Nov 12, 2021

skia-gold commented Nov 13, 2021

flutter-dashboard bot commented Nov 13, 2021

justinmc commented Nov 15, 2021

justinmc commented Nov 15, 2021

LongCatIsLooong Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

justinmc Nov 18, 2021

Choose a reason for hiding this comment

LongCatIsLooong Nov 18, 2021

Choose a reason for hiding this comment

werainkhatri Nov 18, 2021

Choose a reason for hiding this comment

justinmc Nov 18, 2021

Choose a reason for hiding this comment

justinmc commented Nov 2, 2021 •

edited

Loading

werainkhatri Nov 2, 2021 •

edited

Loading

LongCatIsLooong Nov 17, 2021 •

edited

Loading