Add Non-Uniform Border to Border. #121921

bernaferrari · 2023-03-04T03:17:36Z

Exactly like Figma:

What if it has multiple colors + borderRadius? It will crash as usual.
Side effects: material data tables will paint faster, because drrect > path.

bernaferrari · 2023-03-05T16:56:00Z

packages/flutter/lib/src/painting/box_border.dart

@@ -567,7 +632,7 @@ class Border extends BoxBorder {
    assert(() {
      if (!_strokeAlignIsUniform || top.strokeAlign != BorderSide.strokeAlignInside) {
        throw FlutterError.fromParts(<DiagnosticsNode>[
-          ErrorSummary('A Border can only draw strokeAlign different than BorderSide.strokeAlignInside on uniform borders.'),
+          ErrorSummary('A Border can only draw strokeAlign different than BorderSide.strokeAlignInside on borders with uniform colors.'),


uniform colors/styles instead? Uniform colors is way more common and style == BorderStyle.none is kind of an invisible color.

But if we added a new border style, it might complicate things a lot. For instance, what if we added a "striped" border style that drew diagonal caution stripes? Or a "dashed" border style? I think it would be better if it were both color and style.

bernaferrari · 2023-03-06T02:32:22Z

Hey @gspencergoog, did you miss me? 😛

gspencergoog · 2023-03-20T17:52:15Z

packages/flutter/lib/src/painting/box_border.dart

@@ -567,7 +632,7 @@ class Border extends BoxBorder {
    assert(() {
      if (!_strokeAlignIsUniform || top.strokeAlign != BorderSide.strokeAlignInside) {
        throw FlutterError.fromParts(<DiagnosticsNode>[
-          ErrorSummary('A Border can only draw strokeAlign different than BorderSide.strokeAlignInside on uniform borders.'),
+          ErrorSummary('A Border can only draw strokeAlign different than BorderSide.strokeAlignInside on borders with uniform colors.'),


But if we added a new border style, it might complicate things a lot. For instance, what if we added a "striped" border style that drew diagonal caution stripes? Or a "dashed" border style? I think it would be better if it were both color and style.

packages/flutter/lib/src/painting/box_border.dart

gspencergoog · 2023-03-20T18:04:48Z

packages/flutter/lib/src/painting/box_border.dart

@@ -538,15 +578,25 @@ class Border extends BoxBorder {
      }
    }

+    // Allow painting non-uniform borders if the color is uniform.


Suggested change

// Allow painting non-uniform borders if the color is uniform.

// Allow painting non-uniform borders if the shape is rectangular, and the color and style are uniform.

I think I'll allow BoxShape.circle. I've been trying to deprecate it for 8 months, but that will give a reason for it to exist.

Let me see if I can implement non-uniform border for circles.

Done. Making tests.

gspencergoog · 2023-03-20T18:05:13Z

packages/flutter/lib/src/painting/box_border.dart

+    // Allow painting non-uniform borders if the color is uniform.
+    //
+    // See also:
+    //  * <https://pub.dev/packages/non_uniform_border>, a package that


Can you move this "See also" to a real doc comment so people can see it?

Done, check if you prefer in Border or paint (probably paint, right?).

gspencergoog · 2023-03-20T18:07:45Z

packages/flutter/test/painting/border_test.dart

+      end: BorderSide(width: 15, strokeAlign: BorderSide.strokeAlignOutside),
+      bottom: BorderSide(width: 20),
+    );
+    expect(nonUniformBorderDirectional.dimensions, const EdgeInsetsDirectional.fromSTEB(5, 5, 0, 20));


Can you also test to make sure that it asserts when non-uniform borders of various types are tried? You don't have to test for the assert message itself, but at least that it asserts under the right conditions.

Yessss. Hold on.

gspencergoog · 2023-03-20T18:34:27Z

Hey @gspencergoog, did you miss me? 😛

Of course! I love your PRs. :-)

bernaferrari · 2023-03-20T18:57:28Z

Do you think BorderStyle.none could be considered as width == 0 (right now they are color == transparent)? If I make it the same as width == 0 in non-uniform borders, it is one less exception. Only uniform colors would need to be required.

Edit: ignore it for now. I think the implementation will be ugly.

gspencergoog · 2023-03-20T19:34:55Z

Only uniform colors would need to be required.

Again, what happens if we add a new style?

gspencergoog

gspencergoog · 2023-03-20T21:14:23Z

packages/flutter/lib/src/painting/box_border.dart

@@ -212,7 +212,8 @@ abstract class BoxBorder extends ShapeBorder {
  ///
  /// See also:
  ///
-  ///  * [paintBorder], which is used if the border is not uniform.
+  ///  * [paintBorder], which is used if the border has non-uniform colors or styles and no borderRadius.
+  ///  * [Border.paint], which is the similar method in [Border] with additional comments.


What do you mean by "additional comments"? More context? I'd reword this in any case.

Yes. I'll reword. BorderDirectional is shorter and gives less detail on what each parameter do than the equivalent Border.

Done. Replacement is:

[Border.paint], similar to this method, includes additional comments and provides more details on each parameter than described here.

packages/flutter/lib/src/painting/box_border.dart

packages/flutter/test/painting/border_test.dart

Piinks

LGTM

Also add more tests.

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Mar 4, 2023

bernaferrari force-pushed the Non-Uniform branch 8 times, most recently from 2c66e80 to 8e822a2 Compare March 5, 2023 16:15

flutter-dashboard bot added the f: material design flutter/packages/flutter/material repository. label Mar 5, 2023

bernaferrari force-pushed the Non-Uniform branch 3 times, most recently from 9c63189 to 7dfad4b Compare March 5, 2023 16:39

bernaferrari commented Mar 5, 2023

View reviewed changes

bernaferrari force-pushed the Non-Uniform branch 2 times, most recently from 4bdeb98 to 7575642 Compare March 6, 2023 01:39

bernaferrari marked this pull request as ready for review March 6, 2023 02:28

HansMuller requested a review from gspencergoog March 17, 2023 22:03

bernaferrari force-pushed the Non-Uniform branch 7 times, most recently from 11c6e01 to a288699 Compare March 19, 2023 16:06

gspencergoog reviewed Mar 20, 2023

View reviewed changes

bernaferrari mentioned this pull request Mar 20, 2023

Deprecate BoxDecoration.shape. #108052

Open

bernaferrari force-pushed the Non-Uniform branch 3 times, most recently from a85d907 to 48337da Compare March 20, 2023 21:18

gspencergoog approved these changes Mar 20, 2023

View reviewed changes

bernaferrari force-pushed the Non-Uniform branch 5 times, most recently from 900dcf3 to acf8bef Compare March 23, 2023 14:50

Piinks approved these changes Mar 23, 2023

View reviewed changes

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 23, 2023

bernaferrari added 6 commits March 23, 2023 21:22

Add Non-Uniform Border to Border.

d399574

Fix BorderDirectional and dimensions.

861d89f

Relax strokeAlign requirements.

67566ce

Fix test.

d9c6f1c

Also add more tests.

Replace _clampRadius with Radius.clamp.

304dbd0

Simplify + Fix Dart 3 issues.

72be3b9

bernaferrari force-pushed the Non-Uniform branch from acf8bef to 72be3b9 Compare March 24, 2023 00:22

auto-submit bot merged commit 5054b6e into flutter:master Mar 24, 2023
71 checks passed

bernaferrari deleted the Non-Uniform branch March 24, 2023 01:41

engine-flutter-autoroll mentioned this pull request Mar 24, 2023

Roll Flutter from d79f3aab0fa8 to a8afbfa22e34 (16 revisions) flutter/packages#3542

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 24, 2023

5054b6e51 Add Non-Uniform Border to Border. (flutter/flutter#121921)

33394cc

CaseyHillers mentioned this pull request Mar 24, 2023

Test DataTable2 renders with border and background decoration fails on flutter master maxim-saplin/data_table_2#178

Closed

HansMuller mentioned this pull request Mar 24, 2023

Flutter Framework Team: Notable Commits #121415

Open

This was referenced Mar 28, 2023

i123643 print java version gradle #123644

Merged

Add Java-Gradle-AGP validation to flutter analyze #123916

Merged

reidbaker mentioned this pull request Apr 5, 2023

Update getGradleVersion to ignore commented out lines #124260

Merged

8 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

5054b6e51 Add Non-Uniform Border to Border. (flutter/flutter#121921)

60d6f9c

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

5054b6e51 Add Non-Uniform Border to Border. (flutter/flutter#121921)

e742daa

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Non-Uniform Border to Border. #121921

Add Non-Uniform Border to Border. #121921

bernaferrari commented Mar 4, 2023 •

edited

bernaferrari Mar 5, 2023

gspencergoog Mar 20, 2023

bernaferrari Mar 20, 2023

bernaferrari commented Mar 6, 2023

gspencergoog Mar 20, 2023

gspencergoog Mar 20, 2023

bernaferrari Mar 20, 2023

bernaferrari Mar 20, 2023

gspencergoog Mar 20, 2023

bernaferrari Mar 20, 2023

gspencergoog Mar 20, 2023

bernaferrari Mar 20, 2023

gspencergoog commented Mar 20, 2023

bernaferrari commented Mar 20, 2023 •

edited

gspencergoog commented Mar 20, 2023

gspencergoog left a comment

gspencergoog Mar 20, 2023

bernaferrari Mar 20, 2023

bernaferrari Mar 21, 2023 •

edited

Piinks left a comment

	// Allow painting non-uniform borders if the color is uniform.
	// Allow painting non-uniform borders if the shape is rectangular, and the color and style are uniform.

Add Non-Uniform Border to Border. #121921

Add Non-Uniform Border to Border. #121921

Conversation

bernaferrari commented Mar 4, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernaferrari commented Mar 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gspencergoog commented Mar 20, 2023

bernaferrari commented Mar 20, 2023 • edited

gspencergoog commented Mar 20, 2023

gspencergoog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernaferrari Mar 21, 2023 • edited

Choose a reason for hiding this comment

Piinks left a comment

Choose a reason for hiding this comment

bernaferrari commented Mar 4, 2023 •

edited

bernaferrari commented Mar 20, 2023 •

edited

bernaferrari Mar 21, 2023 •

edited