Add a confirmation dialog to stock app Settings page and style it by default

Add a confirmation dialog to stock app Settings page and style it by default

R=ianh@google.com, abarth, hixie

Committed: https://chromium.googlesource.com/external/mojo/+/71ee41e2366114126718bb49928a47c4111dec28

[eseidel, 2015-06-23 19:37:23]

eseidel@chromium.org changed reviewers:
+ eseidel@chromium.org

[eseidel, 2015-06-23 19:37:25]

https://codereview.chromium.org/1201273002/diff/1/sky/examples/stocks2/lib/st...
File sky/examples/stocks2/lib/stock_settings.dart (right):

https://codereview.chromium.org/1201273002/diff/1/sky/examples/stocks2/lib/st...
sky/examples/stocks2/lib/stock_settings.dart:50:
navigator.pushState("/settings/confirm", (_) {
I didn't realize dialogs were a navigation state.  That's interesting.  I guess
it's not safe to navigate back to a dialog url though?  State wouldn't be set
up...

[Hixie, 2015-06-23 19:40:27]

ianh@google.com changed reviewers:
- eseidel@chromium.org

[Hixie, 2015-06-23 19:40:28]

lgtm

https://codereview.chromium.org/1201273002/diff/1/sky/examples/stocks2/lib/st...
File sky/examples/stocks2/lib/stock_settings.dart (right):

https://codereview.chromium.org/1201273002/diff/1/sky/examples/stocks2/lib/st...
sky/examples/stocks2/lib/stock_settings.dart:28: bool showDialog = false;
Have a more specific name for this. We could end up with many dialogs.

https://codereview.chromium.org/1201273002/diff/1/sky/examples/stocks2/lib/st...
sky/examples/stocks2/lib/stock_settings.dart:101: class _ConfirmDialog extends
Component {
Why doesn't this extend Dialog? It seems weird that a Confirmation Dialog would
_have_ a dialog rather than _be_ a dialog.

[jackson, 2015-06-23 20:20:45]

https://codereview.chromium.org/1201273002/diff/1/sky/examples/stocks2/lib/st...
File sky/examples/stocks2/lib/stock_settings.dart (right):

https://codereview.chromium.org/1201273002/diff/1/sky/examples/stocks2/lib/st...
sky/examples/stocks2/lib/stock_settings.dart:28: bool showDialog = false;
On 2015/06/23 19:40:27, Hixie wrote:
> Have a more specific name for this. We could end up with many dialogs.

Done.

https://codereview.chromium.org/1201273002/diff/1/sky/examples/stocks2/lib/st...
sky/examples/stocks2/lib/stock_settings.dart:50:
navigator.pushState("/settings/confirm", (_) {
On 2015/06/23 19:37:24, eseidel wrote:
> I didn't realize dialogs were a navigation state.  That's interesting.  I
guess
> it's not safe to navigate back to a dialog url though?  State wouldn't be set
> up...

This is a route that the navigator knows how to back out of, but doesn't know
how to navigate to.

https://codereview.chromium.org/1201273002/diff/1/sky/examples/stocks2/lib/st...
sky/examples/stocks2/lib/stock_settings.dart:101: class _ConfirmDialog extends
Component {
On 2015/06/23 19:40:27, Hixie wrote:
> Why doesn't this extend Dialog? It seems weird that a Confirmation Dialog
would
> _have_ a dialog rather than _be_ a dialog.

Done.

[Hixie, 2015-06-23 20:27:44]

lgtm

[abarth-chromium, 2015-06-23 20:52:51]

https://codereview.chromium.org/1201273002/diff/60001/sky/examples/stocks2/li...
File sky/examples/stocks2/lib/stock_settings.dart (right):

https://codereview.chromium.org/1201273002/diff/60001/sky/examples/stocks2/li...
sky/examples/stocks2/lib/stock_settings.dart:100: child: new
ShrinkWrapWidth(child: new Text('NO THANKS')),
Text should automatically ShrinkWrapWidth.  We shouldn't need a ShrinkWrapWidth
widget in the tree.

https://codereview.chromium.org/1201273002/diff/60001/sky/examples/stocks2/li...
sky/examples/stocks2/lib/stock_settings.dart:110: ], justifyContent:
FlexJustifyContent.flexEnd),
Should the dialog have the opinion that the actions list go into a flex
container with flexEnd?

https://codereview.chromium.org/1201273002/diff/60001/sky/sdk/lib/widgets/fla...
File sky/sdk/lib/widgets/flat_button.dart (right):

https://codereview.chromium.org/1201273002/diff/60001/sky/sdk/lib/widgets/fla...
sky/sdk/lib/widgets/flat_button.dart:5: import 'dart:sky' show Color;
Color should already be available from basic.dart.

[jackson, 2015-06-23 21:02:36]

https://codereview.chromium.org/1201273002/diff/60001/sky/examples/stocks2/li...
File sky/examples/stocks2/lib/stock_settings.dart (right):

https://codereview.chromium.org/1201273002/diff/60001/sky/examples/stocks2/li...
sky/examples/stocks2/lib/stock_settings.dart:100: child: new
ShrinkWrapWidth(child: new Text('NO THANKS')),
On 2015/06/23 20:52:51, abarth wrote:
> Text should automatically ShrinkWrapWidth.  We shouldn't need a
ShrinkWrapWidth
> widget in the tree.

Done.

https://codereview.chromium.org/1201273002/diff/60001/sky/examples/stocks2/li...
sky/examples/stocks2/lib/stock_settings.dart:110: ], justifyContent:
FlexJustifyContent.flexEnd),
On 2015/06/23 20:52:51, abarth wrote:
> Should the dialog have the opinion that the actions list go into a flex
> container with flexEnd?

Done.

https://codereview.chromium.org/1201273002/diff/60001/sky/sdk/lib/widgets/fla...
File sky/sdk/lib/widgets/flat_button.dart (right):

https://codereview.chromium.org/1201273002/diff/60001/sky/sdk/lib/widgets/fla...
sky/sdk/lib/widgets/flat_button.dart:5: import 'dart:sky' show Color;
On 2015/06/23 20:52:51, abarth wrote:
> Color should already be available from basic.dart.

Done.

[jackson, 2015-06-23 21:19:04]

Committed patchset #7 (id:120001) manually as
71ee41e2366114126718bb49928a47c4111dec28 (presubmit successful).