It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Added a test

@LongCatIsLooong may be the right person to review. The original reason this was added was to match iOS's default behavior, especially around back button labels when the text gets long. If that behavior still matches, we should remove this (especially since this affects Material too)

Also tagging @justinmc

BoxConstraints.loose() fails tests:

/usr/local/google/home/aweiland/git/flutter/bin/flutter --no-color test --machine --start-paused test/material/app_bar_test.dart
Testing started at 6:17 PM ...

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: <25.0>
  Actual: <28.0>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (file:///usr/local/google/home/aweiland/git/flutter/packages/flutter/test/material/app_bar_test.dart:530:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///usr/local/google/home/aweiland/git/flutter/packages/flutter/test/material/app_bar_test.dart line 530
The test description was:
  AppBar actions are vertically centered
════════════════════════════════════════════════════════════════════════════════════════════════════

Test failed. See exception logs above.
The test description was: AppBar actions are vertically centered

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: Size:<Size(56.0, 56.0)>
  Actual: _DebugSize:<Size(56.0, 48.0)>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (file:///usr/local/google/home/aweiland/git/flutter/packages/flutter/test/material/app_bar_test.dart:714:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///usr/local/google/home/aweiland/git/flutter/packages/flutter/test/material/app_bar_test.dart line 714
The test description was:
  leading widget extends to edge and is square
════════════════════════════════════════════════════════════════════════════════════════════════════

Test failed. See exception logs above.
The test description was: leading widget extends to edge and is square

You might need to center the leading widget vertically (similar to how the trailing widget is positioned: final double trailingY = (size.height - trailingSize.height) / 2.0;), now that the vertical constraint is no longer tight.

I feel like loosening the constraints is a much riskier change than what I set out to do, and I'd rather keep this change small and safe.

Ok I'll try that after this is merged.

Hello, everyone!

I know this PR is considerably old, but we recently upgraded from Flutter 3.3 to Flutter 3.10 and some things broke in our app, I think it's related with this change.

Is this a breaking change? If so, it was not documented in the breaking changes page.

In our case, we relied on the constraint to use a Row with an Expanded inside, and after this change it made the middle widget go out of place.

Exemplifying what @mateusfccp just said:

In Flutter 3.3, we have:

In Flutter 3.10, we have:

Notice how the title widget just disappears and there is no overflow warning showing.

The code for the examples above:

AppBar(
    leadingWidth: 500,
    title: Text(widget.title),
    leading: Row(
        children: [BackButton(), Text('A widget')],
    ),
    actions: const [Text('Another widget')],
)

Btw, If I remove the leadingWidth, in both cases the title becomes visible and the leading overlaps the title, and shows an overflow warning.

This change isn't considered breaking per the definition here: https://github.com/flutter/flutter/wiki/Tree-hygiene#1-determine-if-your-change-is-a-breaking-change. You can register your tests in customer_testing to make sure future changes do not break your app (see the linked wiki page for the instructions).

For this specific problem I think you can always use a LayoutBuilder to get the incoming constraints and limit the width to be 1/3 of the incoming max width?

This change isn't considered breaking per the definition here.

Yeah, you may be right strictly speaking. Should we reconsider this definition? I mean, our app simply broke in terms of layout, but we didn't notice at the time of migration, just later. Maybe we could say that it's our fault for not testing it thoroughly, but considering this kind of change affects potentially thousands of consumers and is a silent breakage (different, for instance, from an API breakage that will break as soon as possible instead of in runtime), maybe we should review what are our definitions of a breaking change.

I know this is not the purpose of this PR, but I am trying to collect early feedback. Do you think this is a valid discussion that we should open as an issue?

@LongCatIsLooong

For this specific problem I think you can always use a LayoutBuilder to get the incoming constraints and limit the width to be 1/3 of the incoming max width?

When directly using NavigationToolbar it's possible, but in cases where we use components that internally uses NavigationToolbar, no. For instance, the example provided by @opaulovieira , where the AppBar broke. AppBar uses NavigationToolbar internally, so there's no way for use to use a LayoutBuilder wrapping the internal NavigationToolbar.

The only options we have is:

• copying the AppBar code and changing it, which is totally not viable, because we will have to keep copying it every update;
• put the LayoutBuilder around the entire Scaffold, which is not as bad the first option but is also not a nice workaround. Maybe this one is what we are going to use here, but we are not satisfied with this solution, as it seems very invasive.

Maybe there's another solution we didn't consider?

Your second bullet is exactly what I would suggest. What is your concern with it?

Ah I meant using a LayoutBuilder to wrap the leading widget not the scaffold, I didn't notice the AppBar widget is giving its leading widget a tight width constraint. But you can still wrap the AppBar in a PreferredSize widget + LayoutBuilder to compute the desired leading width?

The current option is to register your tests in the customer_testing shard as mentioned in the wiki. Regarding the current breaking change policy I would ask @Hixie (he's very active on discord you should be able to find him there).

Your second bullet is exactly what I would suggest. What is your concern with it?

Not a big concern, but overall I don't think it's a good practice to wrap a build around a big widget tree when only a small portion of it needs it.

Ah I meant using a LayoutBuilder to wrap the leading widget not the scaffold, I didn't notice the AppBar widget is giving its leading widget a tight width constraint. But you can still wrap the AppBar in a PreferredSize widget + LayoutBuilder to compute the desired leading width?

The issue is that I have to pass a preferred size to PreferredSize, and the preferred size of an AppBar is computed internally. I would have to replicate this computation somehow outside of the AppBar, and if AppBar preferredSize implementation ever change I have to change externally too, which is a little burdensome.

In this sense, wrapping Scaffold is actually better, as it's low-maintenance.

the preferred size of an AppBar is computed internally

It's part of AppBar's interface you could just return appBar.preferredSize right?

the preferred size of an AppBar is computed internally

It's part of AppBar's interface you could just return appBar.preferredSize right?

I am not sure, as I need LayoutBuilder constraints to be passed to the AppBar parameters, which means I can't extract appBar to a name.

final appBar = LayoutBuilder(
  (context, constraints) => AppBar(leadingWidth: constraints.maxWidth / 3.0, ...),
); // appBar is `LayoutBuilder`, so can't be used

If I assign AppBar to a name to be used inside LayoutBuilder it won't have the constraints, while if I assign the LayoutBuilder with the AppBar to a name, I can't use .preferredSize.