Skip to content
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.

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

Changed AppBar constraints break layouts #44550

Closed
slightfoot opened this issue Nov 10, 2019 · 7 comments
Closed

Changed AppBar constraints break layouts #44550

slightfoot opened this issue Nov 10, 2019 · 7 comments
Assignees
Labels
c: regression It was better in the past than it is now customer: wednesday Hump day Q&A f: material design flutter/packages/flutter/material repository. found in release: 1.20 Found to occur in 1.20 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list

Comments

@slightfoot
Copy link
Member

slightfoot commented Nov 10, 2019

I recently compiled one of our application with the latest dev channel version v1.10.15 and I noticed that the logo placed in the AppBar suddenly was broken. Confirmed this is not the case in v1.10.14..

I've tracked it down to a change made by @HansMuller in PR #42936: Support AppBars with jumbo titles. This change makes the AppBar's title area layout with an unconstrained height. This also seems to layout outside the parent bounds as pictured below.

// Layout the AppBar's title with unconstrained height, vertically

This wasn't reported as a breaking change. So perhaps there is thought into what conforms to a breaking change here? Also, since the AppBar is a preferredSize widget I think there is an expectation that a height should be provided to the title widget?

I've seen these types of layout issues before and I kinda feel minWidth/minHeight should resolve these. But the framework seems to fail on this to some degree. For example in this situation, we would expect their to be a minWidth/minHeight of the AppBar title area size and then have the maxHeight unconstrained. So that child widget need to be at least the size of the title area like a Tight constraint but can also get larger if required.

Old v1.10.14 New v1.10.15
old new

Example Code

import 'package:flutter/material.dart';

void main() {
  runApp(MaterialApp(
    debugShowCheckedModeBanner: false,
    home: Example(),
  ));
}

class Example extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Center(
          child: Padding(
            padding: const EdgeInsets.all(8.0),
            child: AspectRatio(
              aspectRatio: (4 / 1),
              child: Placeholder(color: Colors.white.withOpacity(0.5)),
            ),
          ),
        ),
      ),
    );
  }
}

Logs

[√] Flutter (Channel dev, v1.10.15, on Microsoft Windows [Version 10.0.17763.805], locale en-GB)
    • Flutter version 1.10.15 at C:\Android\flutter
    • Framework revision fbabb264e0 (9 days ago), 2019-11-02 01:36:07 +0300
    • Engine revision 8ea19b1c76
    • Dart version 2.6.0 (build 2.6.0-dev.8.2 bbe2ac28c9)
@slightfoot slightfoot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. customer: wednesday Hump day Q&A labels Nov 10, 2019
@slightfoot slightfoot changed the title New AppBar layout constraints break layouts. Changed AppBar constraints break layouts Nov 10, 2019
@HansMuller HansMuller added the c: regression It was better in the past than it is now label Nov 11, 2019
@HansMuller
Copy link
Contributor

This is indeed an unintended regression due to a breaking change in AppBar's layout; see #42936. The breaking change should have been announced along with a remedy that restored the original behavior.

Sorry about that.

Here's an app that demonstrates the problem:

import 'package:flutter/material.dart';

class Home extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    const String logoAsset = 'images/logo.png';
    return Scaffold(
      appBar: AppBar(
        title: Image.asset(logoAsset),
      ),
      body: Padding(
        padding: EdgeInsets.all(64),
        child: Image.asset(logoAsset),
      ),
    );
  }
}

void main() {
  runApp(MaterialApp(home: Home()));
}

The images/logo.png image is from pixabay.

To restore the original AppBar layout behavior, wrap the AppBar's title in a SizedBox with height: kToolbarHeight.

appBar: AppBar(
  title: SizedBox(
    height: kToolbarHeight,
    child: Image.asset(logoAsset),
  ),
),
Current Correct
appbar_bad appbar_ok

|

Will update the API docs and Stack Overflow.

@slightfoot
Copy link
Member Author

@HansMuller I understand the workaround. I have implemented as much at the moment. The problem is really the way the jumbo title solution is implemented surely? I see other layout problems coming in with FlexibleSpace and other related changes to AppBar size. That really needs the constraints to be passed down.

@HansMuller
Copy link
Contributor

@slightfoot Do you have a specific example of an AppBar layout failure related to FlexibleSpace or the AppBar's preferred size?

@HansMuller HansMuller added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Jan 3, 2020
@Hixie
Copy link
Contributor

Hixie commented Jan 26, 2020

This (the jumbo appbar PR) also broke my personal app's appbars. I had a column in the AppBar so that it would align things to the bottom, and that stopped working (now the Column just shrink-wraps). Testcase:

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';

import 'package:dummy_appbar/main.dart';

void main() {
  testWidgets('Counter increments smoke test', (WidgetTester tester) async {
    final UniqueKey key = UniqueKey();
    await tester.pumpWidget(MaterialApp(
      home: Scaffold(
        appBar: AppBar(
          centerTitle: true,
          title: Column(
            mainAxisAlignment: MainAxisAlignment.end,
            children: <Widget>[
              SizedBox(
                width: 10.0,
                height: 10.0,
                child: DecoratedBox(
                  key: key,
                  decoration: BoxDecoration(
                    color: Colors.yellow,
                  ),
                ),
              ),
            ],
          ),
        ),
      ),
    ));
    expect(tester.getRect(find.byKey(key)), Rect.fromLTWH(395, 46, 10, 10));
  });
}

@Hixie Hixie added this to the Goals milestone Jan 26, 2020
@Hixie Hixie removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Jan 26, 2020
Hixie added a commit to Hixie/cruisemonkey that referenced this issue Jan 26, 2020
Hixie added a commit to jocosocial/rainbowmonkey that referenced this issue Jan 27, 2020
@kf6gpe kf6gpe added the P2 Important issues not at the top of the work list label May 29, 2020
@HansMuller HansMuller modified the milestones: [DEPRECATED] Goals, 1.22 - August 2020 Jun 5, 2020
@TahaTesser
Copy link
Member

Code Sample
import 'package:flutter/material.dart';

void main() {
  runApp(MaterialApp(
    debugShowCheckedModeBanner: false,
    home: Example(),
  ));
}

class Example extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Center(
          child: Padding(
            padding: const EdgeInsets.all(8.0),
            child: AspectRatio(
              aspectRatio: (4 / 1),
              child: Placeholder(color: Colors.white.withOpacity(0.5)),
            ),
          ),
        ),
      ),
    );
  }
}

flutter doctor -v
[✓] Flutter (Channel dev, 1.20.0-2.0.pre, on Mac OS X 10.15.5 19F101, locale
    en-GB)
    • Flutter version 1.20.0-2.0.pre at /Users/tahatesser/Code/flutter_dev
    • Framework revision 15a28159bc (9 days ago), 2020-06-23 04:52:58 -0700
    • Engine revision 91a63d6a44
    • Dart version 2.9.0 (build 2.9.0-19.0.dev 7e72c9ae7e)

 
[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.0)
    • Android SDK at /Users/tahatesser/Code/sdk
    • Platform android-30, build-tools 30.0.0
    • ANDROID_HOME = /Users/tahatesser/Code/sdk
    • Java binary at: /Applications/Android
      Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build
      1.8.0_242-release-1644-b3-6222593)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 11.5)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 11.5, Build version 11E608c
    • CocoaPods version 1.9.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 4.0)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin version 47.1.2
    • Dart plugin version 193.7361
    • Java version OpenJDK Runtime Environment (build
      1.8.0_242-release-1644-b3-6222593)

[✓] VS Code (version 1.46.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.12.1

[✓] Connected device (5 available)
    • SM M305F      • 32003c30dc19668f          • android-arm64  • Android 10
      (API 29)
    • Taha’s iPhone • 00008020-001059882212002E • ios            • iOS 13.5.1
    • macOS desktop • macos                     • darwin-x64     • Mac OS X
      10.15.5 19F101
    • Web Server    • web-server                • web-javascript • Flutter Tools
    • Chrome        • chrome                    • web-javascript • Google Chrome
      83.0.4103.116

• No issues found!

@TahaTesser TahaTesser added found in release: 1.20 Found to occur in 1.20 has reproducible steps The issue has been confirmed reproducible and is ready to work on labels Jul 2, 2020
@HansMuller
Copy link
Contributor

I'm going to close this as the current AppBar layout behavior has been documented #49583 and has been around for long enough to make adding another incompatible layout change worse than leaving things as they are. Sorry about the long delay in resolving this.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: regression It was better in the past than it is now customer: wednesday Hump day Q&A f: material design flutter/packages/flutter/material repository. found in release: 1.20 Found to occur in 1.20 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list
Projects
None yet
Development

No branches or pull requests

5 participants