Skip to content

Conversation

@bernaferrari
Copy link
Contributor

@bernaferrari bernaferrari commented Apr 18, 2022

Fix #88621.

This is not a breaking change, however, we probably want BorderSide.toString() to include the new StrokeAlign property, which kind of makes it a breaking change. I had to update 20 tests. We could live without this, it works the same without toString(), but I think it was the correct thing to do.

Left, Figma. Right, Flutter:
image

The change is relatively simple and allows nice things in the future, such as rings which are popular in Tailwind.

Important to know:

  • Lerp is HARD. There is no 'simple way' to lerp from inside to outside or center to outside. But, all strokeAlign values share a similar state: zero. If the strokeAlign changes, from t = 0 to t = 0.49, a.width becomes 0, then from t = 0.50 to t = 1, 0 becomes b.width. This makes the animation fluid (however kind of weird, as there are two different speeds, so it doesn't feel linear). We could also get the t where they would linearly intersect and apply the same strategy there, so it is REALLY linear, instead of fast until 0.49 and slow until 1.
Screen.Recording.2022-04-18.at.02.31.43.mov
  • Border(left: BorderSide(strokeAlign: StrokeAlign.outside)) is not supported because its implementation is different (and we probably don't want this to happen, like BorderRadius is also forbidden).

  • Border and all OutlinedBorders were updated so it gets half the padding on center and no padding at all at outside.

Code sample to play with my changes:

import 'package:flutter/material.dart';

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

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData.dark(),
      home: const Scaffold(body: MyHomePage()),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({Key? key}) : super(key: key);

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

List<Widget> spaceRow(double gap, Iterable<Widget> children) => children
    .expand((item) sync* {
      yield SizedBox(width: gap);
      yield item;
    })
    .skip(1)
    .toList();

List<Widget> spaceColumn(double gap, Iterable<Widget> children) => children
    .expand((item) sync* {
      yield SizedBox(height: gap);
      yield item;
    })
    .skip(1)
    .toList();

class _MyHomePageState extends State<MyHomePage> {
  @override
  Widget build(BuildContext context) {
    const double width = 40;
    final Color borderColor = Colors.green.withOpacity(0.3);
    const debugStrokeAlign = StrokeAlign.outside;

    final circleLerp = CircleBorder(
      side: BorderSide(
        color: Colors.red.withOpacity(0.5),
        width: 15,
        strokeAlign: debugStrokeAlign,
      ),
    );

    final stadiumLerp = StadiumBorder(
      side: BorderSide(
        color: Colors.red.withOpacity(0.5),
        width: 15,
        strokeAlign: debugStrokeAlign,
      ),
    );

    final roundedRectangleLerp = RoundedRectangleBorder(
      side: BorderSide(
        color: Colors.red.withOpacity(0.5),
        width: 15,
        strokeAlign: debugStrokeAlign,
      ),
    );

    ShapeBorder stadiumToCircleLerp(double t) =>
        stadiumLerp.lerpTo(circleLerp, t)!;

    ShapeBorder stadiumToRoundedLerp(double t) =>
        stadiumLerp.lerpTo(roundedRectangleLerp, t)!;

    ShapeBorder roundedToCircleLerp(double t) =>
        roundedRectangleLerp.lerpTo(circleLerp, t)!;

    return ListView(
      padding: const EdgeInsets.all(50),
      children: [
        const Text(
          "Material Borders",
          textAlign: TextAlign.center,
        ),
        SizedBox(height: 16),
        Row(
          crossAxisAlignment: CrossAxisAlignment.center,
          mainAxisAlignment: MainAxisAlignment.center,
          children: spaceRow(
            width * 2,
            [
              for (StrokeAlign align in StrokeAlign.values)
                Column(
                  children: spaceColumn(
                    16,
                    [
                      Text(align.name),
                      const SizedBox(height: 16),
                      SizedBox(
                        width: 60,
                        height: 40,
                        child: Material(
                          color: Colors.black,
                          child: Center(
                            child: Container(
                              width: 40,
                              height: 2,
                              color: Colors.red,
                            ),
                          ),
                          shape: CircleBorder(
                            side: BorderSide(
                              color: borderColor,
                              width: 10,
                              strokeAlign: align,
                            ),
                          ),
                        ),
                      ),
                      const SizedBox(height: 16),
                      Material(
                        color: Colors.black,
                        child: SizedBox(
                          width: 100,
                          height: 40,
                          child: Center(
                            child: Container(
                              width: 100,
                              height: 2,
                              color: Colors.red,
                            ),
                          ),
                        ),
                        shape: RoundedRectangleBorder(
                          side: BorderSide(
                            color: borderColor,
                            width: 20,
                            strokeAlign: align,
                          ),
                        ),
                      ),
                      const SizedBox(height: 40),
                      Material(
                        color: Colors.black,
                        child: SizedBox(
                          width: 100,
                          height: 20,
                          child: Center(
                            child: Container(
                              width: 100,
                              height: 2,
                              color: Colors.red,
                            ),
                          ),
                        ),
                        shape: StadiumBorder(
                          side: BorderSide(
                            color: borderColor,
                            width: 5,
                            strokeAlign: align,
                          ),
                        ),
                      ),
                      const SizedBox(height: 16),
                      Material(
                        color: Colors.black,
                        child: SizedBox(
                          width: 100,
                          height: 40,
                          child: Center(
                            child: Container(
                              width: 100,
                              height: 2,
                              color: Colors.red,
                            ),
                          ),
                        ),
                        shape: BeveledRectangleBorder(
                          borderRadius: BorderRadius.circular(10),
                          side: BorderSide(
                            color: borderColor,
                            width: 5,
                            strokeAlign: align,
                          ),
                        ),
                      ),
                      const SizedBox(height: 16),
                      Material(
                        color: Colors.black,
                        child: SizedBox(
                          width: 100,
                          height: 40,
                          child: Center(
                            child: Container(
                              width: 100,
                              height: 2,
                              color: Colors.red,
                            ),
                          ),
                        ),
                        shape: ContinuousRectangleBorder(
                          side: BorderSide(
                            color: Colors.red.withOpacity(0.5),
                            width: 15,
                            strokeAlign: align,
                          ),
                          borderRadius: BorderRadius.circular(30),
                        ),
                      ),
                      // const SizedBox(height: 16),
                      // OutlinedButton(
                      //   onPressed: () {},
                      //   child: const Text("helloo"),
                      //   style: OutlinedButton.styleFrom(
                      //     backgroundColor: Colors.blue,
                      //     side: BorderSide(
                      //       color: Colors.red.withOpacity(0.5),
                      //       width: 5,
                      //       strokeAlign: align,
                      //     ),
                      //   ),
                      // ),
                    ],
                  ),
                ),
            ],
          ),
        ),
        const SizedBox(height: 16),
        const Text(
          "Lerp StadiumBorder -> RoundedRectangleBorder",
          textAlign: TextAlign.center,
        ),
        const SizedBox(height: 16),
        Row(
          children: spaceRow(32, [
            Container(
              width: 100,
              height: 60,
              decoration: ShapeDecoration(
                shape: stadiumToRoundedLerp(0.01),
              ),
            ),
            Container(
              width: 100,
              height: 60,
              decoration: ShapeDecoration(
                color: Colors.yellow.withOpacity(0.5),
                shape: stadiumToRoundedLerp(0.5),
              ),
            ),
            Container(
              width: 100,
              height: 60,
              decoration: ShapeDecoration(
                color: Colors.blue,
                shape: stadiumToRoundedLerp(0.99),
              ),
            ),
            Container(
              width: 100,
              height: 60,
              decoration: ShapeDecoration(
                color: Colors.purple,
                shape: stadiumToRoundedLerp(1.0),
              ),
            ),
            Container(
              width: 100,
              height: 60,
              decoration: ShapeDecoration(shape: roundedRectangleLerp),
            ),
          ]),
        ),
        const SizedBox(height: 16),
        const Text(
          "Lerp StadiumBorder -> CircleBorder",
          textAlign: TextAlign.center,
        ),
        const SizedBox(height: 16),
        Row(
          children: spaceRow(32, [
            Container(
              width: 100,
              height: 60,
              decoration: ShapeDecoration(
                shape: stadiumToCircleLerp(0.01),
              ),
            ),
            Container(
              width: 100,
              height: 60,
              decoration: ShapeDecoration(
                color: Colors.yellow.withOpacity(0.5),
                shape: stadiumToCircleLerp(0.5),
              ),
            ),
            Container(
              width: 100,
              height: 60,
              decoration: ShapeDecoration(
                color: Colors.blue,
                shape: stadiumToCircleLerp(0.99),
              ),
            ),
            Container(
              width: 100,
              height: 60,
              decoration: ShapeDecoration(
                color: Colors.purple,
                shape: stadiumToCircleLerp(1.0),
              ),
            ),
            Container(
              width: 100,
              height: 60,
              decoration: ShapeDecoration(shape: circleLerp),
            ),
          ]),
        ),
        const SizedBox(height: 16),
        const Text(
          "Lerp RoundedRectangleBorder -> CircleBorder",
          textAlign: TextAlign.center,
        ),
        const SizedBox(height: 16),
        Row(
          children: spaceRow(32, [
            Container(
              width: 100,
              height: 100,
              decoration: ShapeDecoration(
                shape: roundedToCircleLerp(0.01),
              ),
            ),
            Container(
              width: 100,
              height: 100,
              decoration: ShapeDecoration(
                color: Colors.yellow.withOpacity(0.5),
                shape: roundedToCircleLerp(0.5),
              ),
            ),
            Container(
              width: 100,
              height: 100,
              decoration: ShapeDecoration(
                color: Colors.blue,
                shape: roundedToCircleLerp(0.99),
              ),
            ),
            Container(
              width: 100,
              height: 100,
              decoration: ShapeDecoration(
                color: Colors.purple,
                shape: roundedToCircleLerp(1.0),
              ),
            ),
            Container(
              width: 100,
              height: 100,
              decoration: ShapeDecoration(
                shape: circleLerp,
              ),
            ),
          ]),
        ),
        const SizedBox(height: 48),
        const Text(
          "Transitions",
          textAlign: TextAlign.center,
        ),
        const SizedBox(height: 20),
        const AnimatedTransitions(),
        const SizedBox(height: 48),
        const Text(
          "Lerp",
          textAlign: TextAlign.center,
        ),
        const SizedBox(height: 20),
        const Lerpness(),
        const SizedBox(height: 20),
        const Text(
          "Container",
          textAlign: TextAlign.center,
        ),
        const SizedBox(height: 40),
        Row(
          crossAxisAlignment: CrossAxisAlignment.center,
          mainAxisAlignment: MainAxisAlignment.center,
          children: spaceRow(
            width * 2,
            [
              for (StrokeAlign align in StrokeAlign.values)
                Container(
                  width: 100,
                  height: 100,
                  decoration: BoxDecoration(
                    color: Colors.red,
                    borderRadius: BorderRadius.circular(20),
                    border: Border.all(
                      color: Colors.black.withOpacity(0.75),
                      width: width,
                      strokeAlign: align,
                    ),
                  ),
                  child: Center(
                      child: Container(
                          width: 100, height: 100, color: Colors.blue)),
                ),
            ],
          ),
        ),
        const SizedBox(height: width * 2),
        Row(
          crossAxisAlignment: CrossAxisAlignment.center,
          mainAxisAlignment: MainAxisAlignment.center,
          children: spaceRow(
            width * 2,
            [
              for (StrokeAlign align in StrokeAlign.values)
                Container(
                  width: 100,
                  height: 100,
                  decoration: BoxDecoration(
                    color: Colors.green,
                    border: Border.all(
                      color: Colors.black.withOpacity(0.75),
                      width: width,
                      strokeAlign: align,
                    ),
                  ),
                ),
            ],
          ),
        ),
        const SizedBox(height: width * 2),
        Row(
          crossAxisAlignment: CrossAxisAlignment.center,
          mainAxisAlignment: MainAxisAlignment.center,
          children: spaceRow(
            width * 2,
            [
              for (StrokeAlign align in StrokeAlign.values)
                Container(
                  width: 100,
                  height: 100,
                  decoration: BoxDecoration(
                    color: Colors.blue,
                    shape: BoxShape.circle,
                    border: Border.all(
                      color: Colors.black.withOpacity(0.75),
                      width: width,
                      strokeAlign: align,
                    ),
                  ),
                ),
            ],
          ),
        ),
      ],
    );
  }
}

class AnimatedTransitions extends StatefulWidget {
  const AnimatedTransitions({Key? key}) : super(key: key);

  @override
  _AnimatedTransitionsState createState() => _AnimatedTransitionsState();
}

class _AnimatedTransitionsState extends State<AnimatedTransitions>
    with TickerProviderStateMixin {
  late AnimationController _controller;
  late Tween<double> tween;

  @override
  initState() {
    super.initState();
    _controller = AnimationController(
      duration: const Duration(seconds: 3),
      vsync: this,
    )..repeat();
  }

  @override
  dispose() {
    _controller.dispose();
    super.dispose();
  }

  @override
  Widget build(BuildContext context) {
    return AnimatedBuilder(
        animation:
            CurvedAnimation(parent: _controller, curve: Curves.slowMiddle),
        builder: (context, child) {
          return Center(
            child: Row(
              mainAxisAlignment: MainAxisAlignment.center,
              children: <Widget>[
                Material(
                  color: Colors.black,
                  child: SizedBox(
                    width: 100,
                    height: 40,
                    child: Center(
                      child: Container(
                        width: 100,
                        height: 2,
                        color: Colors.red,
                      ),
                    ),
                  ),
                  shape: _controller.value > 0.5
                      ? RoundedRectangleBorder(
                          side: BorderSide(
                            color: Colors.red.withOpacity(0.5),
                            width: 15,
                            strokeAlign: StrokeAlign.center,
                          ),
                          borderRadius: BorderRadius.circular(30),
                        )
                      : CircleBorder(
                          side: BorderSide(
                            color: Colors.red.withOpacity(0.5),
                            width: 15,
                            strokeAlign: StrokeAlign.outside,
                          ),
                        ),
                ),
                const SizedBox(width: 20),
                Material(
                  color: Colors.black,
                  child: SizedBox(
                    width: 100,
                    height: 40,
                    child: Center(
                      child: Container(
                        width: 100,
                        height: 2,
                        color: Colors.red,
                      ),
                    ),
                  ),
                  shape: _controller.value > 0.5
                      ? StadiumBorder(
                          side: BorderSide(
                            color: Colors.red.withOpacity(0.5),
                            width: 15,
                            strokeAlign: StrokeAlign.outside,
                          ),
                        )
                      : CircleBorder(
                          side: BorderSide(
                            color: Colors.red.withOpacity(0.5),
                            width: 15,
                            strokeAlign: StrokeAlign.outside,
                          ),
                        ),
                ),
                const SizedBox(width: 20),
                Material(
                  color: Colors.black,
                  child: SizedBox(
                    width: 100,
                    height: 40,
                    child: Center(
                      child: Container(
                        width: 100,
                        height: 2,
                        color: Colors.red,
                      ),
                    ),
                  ),
                  shape: _controller.value > 0.5
                      ? StadiumBorder(
                          side: BorderSide(
                            color: Colors.red.withOpacity(0.5),
                            width: 15,
                            strokeAlign: StrokeAlign.outside,
                          ),
                        )
                      : RoundedRectangleBorder(
                          side: BorderSide(
                            color: Colors.red.withOpacity(0.5),
                            width: 15,
                            strokeAlign: StrokeAlign.outside,
                          ),
                        ),
                ),
              ],
            ),
          );
        });
  }
}

class Lerpness extends StatefulWidget {
  const Lerpness({Key? key}) : super(key: key);

  @override
  State<Lerpness> createState() => _LerpnessState();
}

class _LerpnessState extends State<Lerpness> {
  bool direction = true;

  @override
  Widget build(BuildContext context) {
    return GestureDetector(
      onTap: () {
        setState(() {
          direction = !direction;
        });
      },
      child: AnimatedContainer(
        width: 100,
        height: 100,
        decoration: BoxDecoration(
          color: Colors.red,
          borderRadius: BorderRadius.circular(20),
          border: Border.all(
            color: Colors.black.withOpacity(0.75),
            width: direction ? 20 : 10,
            strokeAlign: direction ? StrokeAlign.inside : StrokeAlign.outside,
          ),
        ),
        duration: const Duration(seconds: 2),
      ),
    );
  }
}

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Apr 18, 2022
@bernaferrari bernaferrari force-pushed the border branch 4 times, most recently from cdb7295 to 75b6580 Compare April 22, 2022 16:31
@gspencergoog gspencergoog self-requested a review April 22, 2022 21:52
@bernaferrari
Copy link
Contributor Author

bernaferrari commented May 15, 2022

I just committed big changes refactoring my PR (it was already ready for review, but while making another PR I learned a few tricks and came back here):

  • ✅ Add StrokeAlign to basically all OutlinedBorders. RoundedRectangleBorder, StadiumBorder and BeveledRectangleBorder (except ContinousRectangleBorder because it doesn't make sense there).
  • ✅ I had changed dimensions for center and outside. Now, I also changed innerPath depending on StrokeAlign. The "issue" is, outerPath must not be changed or stranger things happen (like it affects the widget width). So, the innerPath gets smaller when StrokeAlign.center and is the same as outerPath for StrokeAlign.outside. The difference on StrokeAlign.outside should be in the drawing only, the Border shouldn't occupy space. I think this is fine, but I wanted to document this behavior. I didn't forget outerPath implementations :P.
  • There is a lot of duplicated code between the borders now. Should we keep it that way or maybe we could change OutlinedBorder to include a default dimensions and two helper functions for getInnerPath and paint, that every inherited class could use? What do you think? I tried to be more conservative (the changes are already kind of big and hard to test/see/understand).
  • I updated the sample code, basically showing how everything becomes now:

image

  • ✅ In doing so, I also fixed how CircleBorder lerps to RoundedRectangleBorder and so on. You can see on the sample, it is perfect.

  • There are two outlier behaviors:

  1. ✅ BeveledRectangleBorder by default has a few pixels set to border outside, and when you set to StrokeAlign.outside, it has a few pixels to the inside. I think this is fine. I'm not breaking its behavior and it is consistent.

  2. ⚠️ I decided to remove support for ContinousRectangleBorder because it only makes sense as a single strokeAlign value. Changing that would get the widget in a visually unpleasant way.

  • ⚠️ Tests are hard. RoundedRectangleBorder has a few goldens, the other ones don't. I tried to test based on some points I knew would exist/not exist, but testing paint is a pain.

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, this is really a cool addition!

Just some minor things, I think it looks pretty good overall.

@bernaferrari
Copy link
Contributor Author

Yay! I just fixed everything you mentioned and updated the sample for lerping between different Borders so we can see they are in a great state.

image

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

This is great! Thanks!

@gspencergoog
Copy link
Contributor

Hmm. Looks like you got hit by a random CI issue where it didn't launch all the checks. To fix it, push an empty commit (i.e. use git commit --allow-empty -m "Pushing empty commit" and push) to try triggering it again.

@bernaferrari
Copy link
Contributor Author

It seems tree is broken by something else (all PR are broken). Maybe you should mark as "wait tree to go green".

@gspencergoog
Copy link
Contributor

Yes, the tree is also broken at the moment, but because the "ci.yaml" step failed, you'll need to push an empty commit. That's the step that starts most of the CI checks.

@bernaferrari
Copy link
Contributor Author

I just pushed, but I think someone still didn't fix the ci.yaml issue. I can push again when they do.

@bernaferrari
Copy link
Contributor Author

rebasing to the top

@gspencergoog
Copy link
Contributor

Okay, looks like the checks fired off this time. And the tree is green, so once the tests pass, this will go in!

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • Please get at least one approved review if you are already a member or two member reviews if you are not a member before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@gspencergoog
Copy link
Contributor

Ahh, sorry, I forgot we'll need a second review. Let me see who can take a look.

Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

LGTM as well. Just a couple of readability comments below. Nice work. Some tricky cases here.


bool get _strokeAlignIsUniform {
final StrokeAlign topStrokeAlign = top.strokeAlign;
return start.strokeAlign == topStrokeAlign && bottom.strokeAlign == topStrokeAlign && end.strokeAlign == topStrokeAlign;
Copy link
Contributor

@darrenaustin darrenaustin May 18, 2022

Choose a reason for hiding this comment

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

Suggested change
return start.strokeAlign == topStrokeAlign && bottom.strokeAlign == topStrokeAlign && end.strokeAlign == topStrokeAlign;
return start.strokeAlign == topStrokeAlign
&& bottom.strokeAlign == topStrokeAlign
&& end.strokeAlign == topStrokeAlign;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That also works. My original idea was to make it similar to the others 😅. Fixed.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is funny you prefer && at the start than end. The file has both patterns, so it doesn't matter, I'll use exactly the way you wrote. But first time I see someone doing that.

Copy link
Contributor

Choose a reason for hiding this comment

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

We do it because with four spaces before the &&, and one after, it makes all of the terms line up under the return. It's not something we apply religiously, only in cases where there are a lot of conditions stacked up.

@bernaferrari
Copy link
Contributor Author

I fixed it, thanks Darren!

- Fix BoxDecoration + BorderRadius(0) (which is different than BoxDecoration without a BorderRadius).
- Fix getInnerPath for CircleBorder.
Remove variable that may not always be needed.
Style fix.
Add indentation.
@fluttergithubbot fluttergithubbot merged commit 440e0e2 into flutter:master May 19, 2022
@bernaferrari bernaferrari deleted the border branch May 19, 2022 16:33
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 19, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request May 21, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Request] Add strokeAlign (inside/center/outside) to Border

4 participants