Support automaticallyImplyLeading param in AppBar #11264
Conversation
| /// example, if the [AppBar] is in a [Scaffold] that also has a [Drawer], the | ||
| /// [Scaffold] will fill this widget with an [IconButton] that opens the | ||
| /// drawer. If there's no [Drawer] and the parent [Navigator] can go back, the | ||
| /// [AppBar] will use a [BackButton] that calls [Navigator.maybePop]. |
There was a problem hiding this comment.
This doc needs updating as well.
| /// Creates a material design app bar. | ||
| /// | ||
| /// Typically used in the [Scaffold.appBar] property. | ||
| AppBar({ |
There was a problem hiding this comment.
constructor doc should say that automaticallyImplyLeading must not be null (please copy similar text from another constructor to match style), then add an assert that it's not null.
|
SliverAppBar needs the same treatment. |
|
Fixes applied @Hixie PTAL. |
| /// Creates a material design app bar. | ||
| /// | ||
| /// Arguments [elevation], [primary], [toolbarOpacity], [bottomOpacity], | ||
| /// [automaticallyImplyLeading], must not be null. |
There was a problem hiding this comment.
Thanks for adding this. Can you add a "The" at the stand and an "and" between the last two and remove the final comma, to match the grammar used elsewhere? Thanks! (If this is the style you found somewhere, please let me know and I'll go fix it there too.)
There was a problem hiding this comment.
Copied it from somewhere else but I don't recall where.
| assert(primary != null), | ||
| assert(toolbarOpacity != null), | ||
| assert(bottomOpacity != null), | ||
| assert(automaticallyImplyLeading != null), |
There was a problem hiding this comment.
nit: the order of the asserts should match the order of the arguments (see the style guide - we try to keep the order of everything the same everywhere where the order wouldn't otherwise matter).
| /// [Scaffold] will fill this widget with an [IconButton] that opens the | ||
| /// drawer. If there's no [Drawer] and the parent [Navigator] can go back, the | ||
| /// [AppBar] will use a [BackButton] that calls [Navigator.maybePop]. | ||
| /// If this is null and automaticallyImplyLeading is set to true, the [AppBar] will |
There was a problem hiding this comment.
hyperlink "automaticallyImplyLeading"
| /// [Navigator.maybePop]. | ||
| final Widget leading; | ||
|
|
||
| /// Controls whether we should try to imply the leading widget if null. |
There was a problem hiding this comment.
hyperlink "leading"
(see the style guide; we try to add breadcrumbs everywhere so that people reading the docs have always got something new to read)
| /// Controls whether we should try to imply the leading widget if null. | ||
| /// | ||
| /// If true and leading is null, automatically try to deduce what the leading | ||
| /// widget should be. If false and leading is null, leading space is given to title. |
There was a problem hiding this comment.
hyperlink all occurrences of "leading", and "title".
| /// If true and leading is null, automatically try to deduce what the leading | ||
| /// widget should be. If false and leading is null, leading space is given to title. | ||
| /// If leading widget is not null, this parameter has no effect. | ||
| final bool automaticallyImplyLeading; |
| /// * <https://material.google.com/layout/structure.html#structure-toolbars> | ||
| class SliverAppBar extends StatefulWidget { | ||
| /// Creates a material design app bar that can be placed in a [CustomScrollView]. | ||
| const SliverAppBar({ |
There was a problem hiding this comment.
this constructor should mention non-nullable arguments too
|
This is great. Sorry for the nitpicky doc comments. The code looks great. |
|
All done! |
|
|
This reverts commit 94ed7dc.
3 participants
If automaticallyImplyLeading is true (by default), then we try to derive back or menu icon. If false, we do not.