Skip to content

Scaffold appBar is-a PreferredSizeWidget, etc - #9380

Merged
HansMuller merged 3 commits into
flutter:masterfrom
HansMuller:scaffold_appbar
Apr 13, 2017
Merged

HansMuller merged 3 commits into
flutter:masterfrom
HansMuller:scaffold_appbar

Conversation

@HansMuller

@HansMuller HansMuller commented Apr 13, 2017 •

Copy link
Copy Markdown
Contributor

Requiring a Scaffold's appBar to be an AppBar widget was at least inconvenient and definitely inconsistent with the overall Flutter API. Similarly the bottom of an AppBar had to be a widget that implemented AppBarBottomWidget; somewhat less of an obstacle but still odd.

These quirks in the API existed because sometimes it's useful for a widget to advertise its preferred default size without actually constraining its own size.

The PreferredSizeWidget interface can be used for these situations. AppBar and TabBar implement PreferredSizeWidget:

abstract class PreferredSizeWidget extends Widget {
  Size get preferredSize;
}

In both cases the returned size only limits the height, like new Size.fromHeight(100.0).

Scaffold's appBar and AppBar's bottom properties are now PreferredSizeWidgets. In the common cases where appBar is an AppBar and bottom is a TabBar, nothing needs to change.

To substitute an abitrary widget for PreferredSizeWidget, use PreferredSize. For example:

new Scaffold(
  appBar: new PreferredSize(
    preferredSize: const Size.fromHeight(200.0),
    child: const Text('HELLO WORLD'),
  ),
  body: ...
)

Scaffold now has a default-true primary property, like AppBar does. If true then the height of the [appBar] will be extended by the height of the screen's status bar. Apps that set the Scaffold's primary property to false will very likely also want to set the appBar's property to false.

The AppBar minExtent property has been replaced by preferredSize.

Fixes: #8970
Fixes: #9349

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

docs are out of date now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops, good catch, I missed that one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add some color, e.g. mention that Scaffold uses this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe explicitly point to PreferredSizeWidget.preferredSize, saying that this implements that interface?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

docs are out of date

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please inline this assert and make the constructor const while you're here.

Inlining asserts and making Widget constructors const where possible is the new "add a trailing comma" of code reviews.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add docs similar to AppBar.preferredSize ?

@Hixie Hixie Apr 13, 2017 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/extends/implements/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

actually never mind

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"PreferredSizeWidget" is confusing because this is not strictly a widget. I think maybe "PreferredSizeInterface" or "PreferredSizeProvider" or some such?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think this would be less confusing if it was abstract class PreferredSizeProvider extends Object implements Widget.

@Hixie Hixie Apr 13, 2017 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Interface for Widgets that can...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh I got confused, never mind.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't get confused. I think this should start off as "Interface for Widgets that can return the size this widget would prefer if...".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this needs docs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

also see:

 * [PreferredSizeWidget], the interface which this widget implements to expose the preferred size.
 * [AppBar] and [TabBar], which implement that same interface.

@Hixie

Hixie commented Apr 13, 2017

Copy link
Copy Markdown
Contributor

This approach is great. LGTM.

@HansMuller
HansMuller merged commit c96201a into flutter:master Apr 13, 2017
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scaffold.appBar doesn't accept widgets that aren't AppBars. Scaffold and AppBar are very restrictive compared to other widgets

3 participants