Skip to content

Conversation

@xster
Copy link
Member

@xster xster commented Jun 14, 2018

Part 1 of #18037

  • Back port some doc updates from MaterialApp to WidgetsApp
  • Extract common parts to doc templates to prevent future drift
  • Add a CupertinoApp (that reuses CupertinoTabView instead of re-implementing navigator routing yet one more time)
  • Clean all the tests

@xster xster requested a review from Hixie June 14, 2018 03:55
Copy link
Contributor

Choose a reason for hiding this comment

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

add an assert to check that

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Added some comments here and below too about not letting WidgetsApp make a navigator.

xster added a commit to flutter/goldens that referenced this pull request Jun 14, 2018
Copy link
Contributor

Choose a reason for hiding this comment

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

iirc, some of the docs in this file were subtly different than those in widgets/app.dart in various minor ways. Please verify that the ones you've removed are in fact the same byte-for-byte.

Copy link
Member Author

Choose a reason for hiding this comment

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

Reverted some unintended merging of MaterialApp's doc into WidgetsApp. Mainly references to routes. Double checked that the delete docs from MaterialApp appear word for word in the template.

@Hixie
Copy link
Contributor

Hixie commented Jun 14, 2018

can we get away with not including the Navigator in CupertinoApp at all?

@xster
Copy link
Member Author

xster commented Jun 14, 2018

In the current state, for consistency with MaterialApp, no :(
Users don't necessarily want to use bottom tabs which also provides navigators.

Though I think if you extract a router out of the MaterialApp in a future refactor, it's worth removing it from here as well.

Copy link
Member Author

@xster xster Jun 14, 2018

Choose a reason for hiding this comment

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

Semi tangential, I don't think this is true in the MaterialApp. We never actually create an actual Route and put a widget inside that says you did something wrong. We just print FlutterErrors.

I'm gonna remove this sentence.

@sandrasandeep sandrasandeep reopened this Jun 19, 2018
sandrasandeep pushed a commit to flutter/goldens that referenced this pull request Jun 19, 2018
This reverts commit 828a92a.

Reverting in order to push segmented control golden files.
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be private

@Hixie
Copy link
Contributor

Hixie commented Jun 20, 2018

LGTM

xster added a commit to flutter/goldens that referenced this pull request Jul 3, 2018
@xster
Copy link
Member Author

xster commented Jul 4, 2018

Waiting for the engine roll (which has a golden update queued up) before merging.

@xster xster merged commit 06f63aa into flutter:master Jul 4, 2018
@xster xster deleted the cupertino_app branch July 4, 2018 23:55
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 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.

4 participants