Skip to content

Add Non-Uniform Border to Border. #121921

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.

Sign up for GitHub

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

Jump to bottom
Merged
auto-submit merged 6 commits into from
Mar 24, 2023
Merged

Add Non-Uniform Border to Border. #121921

auto-submit merged 6 commits into from
Mar 24, 2023

Conversation

@bernaferrari
Copy link
Contributor

@bernaferrari bernaferrari commented Mar 4, 2023 •
edited
Loading

Fix #12583.
Fix #109954.

Exactly like Figma:

image

image

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

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Mar 4, 2023
@bernaferrari bernaferrari force-pushed the Non-Uniform branch 8 times, most recently from 2c66e80 to 8e822a2 Compare March 5, 2023 16:15
@flutter-dashboard flutter-dashboard bot added the f: material design flutter/packages/flutter/material repository. label Mar 5, 2023
@bernaferrari bernaferrari force-pushed the Non-Uniform branch 3 times, most recently from 9c63189 to 7dfad4b Compare March 5, 2023 16:39
bernaferrari
bernaferrari commented Mar 5, 2023
View reviewed changes
packages/flutter/lib/src/painting/box_border.dart Outdated
Copy link
Contributor Author

@bernaferrari bernaferrari Mar 5, 2023

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@gspencergoog gspencergoog Mar 20, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@bernaferrari bernaferrari Mar 20, 2023

Choose a reason for hiding this comment

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

Thanks!

@bernaferrari bernaferrari force-pushed the Non-Uniform branch 2 times, most recently from 4bdeb98 to 7575642 Compare March 6, 2023 01:39
@bernaferrari bernaferrari marked this pull request as ready for review March 6, 2023 02:28
@bernaferrari
Copy link
Contributor Author

bernaferrari commented Mar 6, 2023

Hey @gspencergoog, did you miss me? 😛

@HansMuller HansMuller requested a review from gspencergoog March 17, 2023 22:03
@bernaferrari bernaferrari force-pushed the Non-Uniform branch 7 times, most recently from 11c6e01 to a288699 Compare March 19, 2023 16:06
gspencergoog
gspencergoog reviewed Mar 20, 2023
View reviewed changes
packages/flutter/lib/src/painting/box_border.dart Outdated
Copy link
Contributor

@gspencergoog gspencergoog Mar 20, 2023

Choose a reason for hiding this comment

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

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 Outdated
Copy link
Contributor

@gspencergoog gspencergoog Mar 20, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@bernaferrari bernaferrari Mar 20, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@bernaferrari bernaferrari Mar 20, 2023

Choose a reason for hiding this comment

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

Done. Making tests.

packages/flutter/lib/src/painting/box_border.dart Outdated
Copy link
Contributor

@gspencergoog gspencergoog Mar 20, 2023

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@bernaferrari bernaferrari Mar 20, 2023

Choose a reason for hiding this comment

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

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

packages/flutter/test/painting/border_test.dart Outdated
Copy link
Contributor

@gspencergoog gspencergoog Mar 20, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@bernaferrari bernaferrari Mar 20, 2023

Choose a reason for hiding this comment

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

Yessss. Hold on.

@gspencergoog
Copy link
Contributor

gspencergoog commented Mar 20, 2023

Hey @gspencergoog, did you miss me? 😛

Of course! I love your PRs. :-)

@bernaferrari
Copy link
Contributor Author

bernaferrari commented Mar 20, 2023 •
edited
Loading

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
Copy link
Contributor

gspencergoog commented Mar 20, 2023

Only uniform colors would need to be required.

Again, what happens if we add a new style?

@bernaferrari bernaferrari force-pushed the Non-Uniform branch 4 times, most recently from 900dcf3 to acf8bef Compare March 23, 2023 14:50
Piinks
Piinks approved these changes Mar 23, 2023
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

LGTM

@Piinks Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 23, 2023
@auto-submit auto-submit bot merged commit 5054b6e into flutter:master Mar 24, 2023
@bernaferrari bernaferrari deleted the Non-Uniform branch March 24, 2023 01:41
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 24, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 24, 2023
@engine-flutter-autoroll
5054b6e Add Non-Uniform Border to Border. (flutter/flutter#121921)
33394cc
@CaseyHillers CaseyHillers mentioned this pull request Mar 24, 2023
Closed
@HansMuller HansMuller mentioned this pull request Mar 24, 2023
Closed
This was referenced Mar 28, 2023
Merged
Merged
@reidbaker reidbaker mentioned this pull request Apr 5, 2023
Merged
8 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
5054b6e 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
@engine-flutter-autoroll
5054b6e Add Non-Uniform Border to Border. (flutter/flutter#121921)
e742daa
@korca0220 korca0220 mentioned this pull request Jul 21, 2025
Closed
9 tasks
@korca0220 korca0220 mentioned this pull request Aug 4, 2025
Closed
@korca0220 korca0220 mentioned this pull request Sep 22, 2025
Merged
9 tasks
github-merge-queue bot pushed a commit that referenced this pull request Sep 24, 2025
@korca0220
Add non uniform TableBorder (#175773)
35b5342 
Re-implementation of PR #172441

<!--
Thanks for filing a pull request!
Reviewers are typically assigned within a week of filing a request.
To learn more about code review, see our documentation on Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
-->

## Add non-uniform border radius support to TableBorder
This PR extends TableBorder to support border radius even when the outer
border sides have different widths but the same color, similar to the
non-uniform border support added to Border in #121921.

### Changes
- Enhanced border rendering logic: TableBorder can now apply border
radius when outer border sides have uniform colors but non-uniform
widths/styles
- New helper methods: Added `outerBorderIsUniform` property and
`distinctVisibleOuterColors()` method to determine border uniformity
- Optimized paint method: Refactored the paint logic to handle three
scenarios:
  1. Fully uniform borders (existing optimized path)
2. Outer borders with uniform colors but non-uniform widths (new
non-uniform border radius support)
3. Completely non-uniform borders (fallback to standard border painting)

### Before/After
- Before: TableBorder with border radius required all sides (including
inner borders) to be completely identical.
- After: TableBorder with border radius only requires outer border
colors to be the same, allowing different widths per side.

### Example Usage
```dart
TableBorder(
  top: BorderSide(color: Colors.blue, width: 3.0),
  right: BorderSide(color: Colors.blue, width: 1.0),  // Different width
  bottom: BorderSide(color: Colors.blue, width: 2.0), // Different width  
  left: BorderSide.none,                              // No border
  horizontalInside: BorderSide(color: Colors.red, width: 1.0), // Different color OK
  verticalInside: BorderSide(color: Colors.red, width: 1.0),   // Different color OK
  borderRadius: BorderRadius.circular(8), // ✅ Now works!
)
```
Fixes: #173193


## 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.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Jaineel-Mamtora pushed a commit to Jaineel-Mamtora/flutter_forked that referenced this pull request Sep 24, 2025
@korca0220 @Jaineel-Mamtora
Add non uniform TableBorder (flutter#175773)
3c10ba9 
Re-implementation of PR flutter#172441

<!--
Thanks for filing a pull request!
Reviewers are typically assigned within a week of filing a request.
To learn more about code review, see our documentation on Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
-->

## Add non-uniform border radius support to TableBorder
This PR extends TableBorder to support border radius even when the outer
border sides have different widths but the same color, similar to the
non-uniform border support added to Border in flutter#121921.

### Changes
- Enhanced border rendering logic: TableBorder can now apply border
radius when outer border sides have uniform colors but non-uniform
widths/styles
- New helper methods: Added `outerBorderIsUniform` property and
`distinctVisibleOuterColors()` method to determine border uniformity
- Optimized paint method: Refactored the paint logic to handle three
scenarios:
  1. Fully uniform borders (existing optimized path)
2. Outer borders with uniform colors but non-uniform widths (new
non-uniform border radius support)
3. Completely non-uniform borders (fallback to standard border painting)

### Before/After
- Before: TableBorder with border radius required all sides (including
inner borders) to be completely identical.
- After: TableBorder with border radius only requires outer border
colors to be the same, allowing different widths per side.

### Example Usage
```dart
TableBorder(
  top: BorderSide(color: Colors.blue, width: 3.0),
  right: BorderSide(color: Colors.blue, width: 1.0),  // Different width
  bottom: BorderSide(color: Colors.blue, width: 2.0), // Different width  
  left: BorderSide.none,                              // No border
  horizontalInside: BorderSide(color: Colors.red, width: 1.0), // Different color OK
  verticalInside: BorderSide(color: Colors.red, width: 1.0),   // Different color OK
  borderRadius: BorderRadius.circular(8), // ✅ Now works!
)
```
Fixes: flutter#173193


## 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.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
@korca0220 @reidbaker
Add non uniform TableBorder (flutter#175773)
f87a656 
Re-implementation of PR flutter#172441

<!--
Thanks for filing a pull request!
Reviewers are typically assigned within a week of filing a request.
To learn more about code review, see our documentation on Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
-->

## Add non-uniform border radius support to TableBorder
This PR extends TableBorder to support border radius even when the outer
border sides have different widths but the same color, similar to the
non-uniform border support added to Border in flutter#121921.

### Changes
- Enhanced border rendering logic: TableBorder can now apply border
radius when outer border sides have uniform colors but non-uniform
widths/styles
- New helper methods: Added `outerBorderIsUniform` property and
`distinctVisibleOuterColors()` method to determine border uniformity
- Optimized paint method: Refactored the paint logic to handle three
scenarios:
  1. Fully uniform borders (existing optimized path)
2. Outer borders with uniform colors but non-uniform widths (new
non-uniform border radius support)
3. Completely non-uniform borders (fallback to standard border painting)

### Before/After
- Before: TableBorder with border radius required all sides (including
inner borders) to be completely identical.
- After: TableBorder with border radius only requires outer border
colors to be the same, allowing different widths per side.

### Example Usage
```dart
TableBorder(
  top: BorderSide(color: Colors.blue, width: 3.0),
  right: BorderSide(color: Colors.blue, width: 1.0),  // Different width
  bottom: BorderSide(color: Colors.blue, width: 2.0), // Different width  
  left: BorderSide.none,                              // No border
  horizontalInside: BorderSide(color: Colors.red, width: 1.0), // Different color OK
  verticalInside: BorderSide(color: Colors.red, width: 1.0),   // Different color OK
  borderRadius: BorderRadius.circular(8), // ✅ Now works!
)
```
Fixes: flutter#173193


## 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.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gspencergoog gspencergoog gspencergoog approved these changes

@Piinks Piinks Piinks approved these changes

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Proposal] Revamp Border / Add Multiple Border Width Support non-uniform Borders with a non-zero borderRadius

3 participants

@bernaferrari @gspencergoog @Piinks