Add a DialogExample example to views_examples

Add a DialogExample example to views_examples.

See http://crbug.com/671820#c10 for how it looks.

This is helpful for demonstrating behaviours and experimenting with
layout, e.g., for long strings.

BUG=671820
Review-Url: https://codereview.chromium.org/2701373002
Cr-Commit-Position: refs/heads/master@{#452311}
Committed: https://chromium.googlesource.com/chromium/src/+/b83507471b95f9fd24b95e6cc720c6e44ce92f8f

[tapted, 2017-02-20 05:03:27]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-20 05:03:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-20 06:22:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-20 06:22:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tapted, 2017-02-22 01:48:16]

Description was changed from

==========
Add a DialogExample example to views_examples

Remove constrained window dependency


Progress Lots of it

cl format

Example done

Fix frame

cl format

BUG=
==========

to

==========
Add a DialogExample example to views_examples.

See http://crbug/XXX for how it looks.

This is helpful for demonstrating behaviours and experimenting with layout,
e.g., for long strings.

BUG=671820
==========

[tapted, 2017-02-22 01:50:20]

Description was changed from

==========
Add a DialogExample example to views_examples.

See http://crbug/XXX for how it looks.

This is helpful for demonstrating behaviours and experimenting with layout,
e.g., for long strings.

BUG=671820
==========

to

==========
Add a DialogExample example to views_examples.

See http://crbug.com/671820#c10 for how it looks.

This is helpful for demonstrating behaviours and experimenting with
layout, e.g., for long strings.

BUG=671820
==========

[tapted, 2017-02-22 01:50:20]

tapted@chromium.org changed reviewers:
+ patricialor@chromium.org

[tapted, 2017-02-22 01:53:51]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[tapted, 2017-02-22 01:54:41]

Patchset #2 (id:20001) has been deleted

[tapted, 2017-02-22 01:54:57]

Hi Patti, could you do a local review for me - thanks!

[commit-bot: I haz the power, 2017-02-22 01:55:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 03:58:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 03:58:05]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[Patti Lor, 2017-02-22 05:58:34]

lgtm :O I mostly just had questions :)

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
File ui/views/examples/dialog_example.cc (right):

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
ui/views/examples/dialog_example.cc:42: this->SetLayoutManager(fill_layout);
Is the "this->" needed here? (and below?)

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
ui/views/examples/dialog_example.cc:67: return MdTextButton::Create(nullptr,
parent_->extra_button_label_->text());
I realise this is for testing MD/Harmony stuff, but do we want
CreateSecondaryUiButton() or CreateSecondaryUiBlueButton()? (Do we care? Is
there a way to enable secondary MD in views_examples?)

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
ui/views/examples/dialog_example.cc:106: // WidgetDelgate:
WidgetDelegate (typo)

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
File ui/views/examples/dialog_example.h (right):

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
ui/views/examples/dialog_example.h:40: template <class>
I haven't really used cpp templates much - does that need to be <class T>/<class
DialogType> or is it fine without?

[tapted, 2017-02-22 22:42:30]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 22:43:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-02-22 22:45:04]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[tapted, 2017-02-22 22:45:21]

Patchset #3 (id:60001) has been deleted

[tapted, 2017-02-22 22:45:30]

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
File ui/views/examples/dialog_example.cc (right):

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
ui/views/examples/dialog_example.cc:42: this->SetLayoutManager(fill_layout);
On 2017/02/22 05:58:34, Patti Lor wrote:
> Is the "this->" needed here? (and below?)

Yup - because the parent is a template argument, the compiler tries to find
`SetLayoutManager` before it knows what the base class is, and it can't find it.
But passing `this` tells the compiler to wait until it knows what the template
arg is before looking.

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
ui/views/examples/dialog_example.cc:67: return MdTextButton::Create(nullptr,
parent_->extra_button_label_->text());
On 2017/02/22 05:58:34, Patti Lor wrote:
> I realise this is for testing MD/Harmony stuff, but do we want
> CreateSecondaryUiButton() or CreateSecondaryUiBlueButton()? (Do we care? Is
> there a way to enable secondary MD in views_examples?)

ooh good point - this should use MdTextButton::CreateSecondaryUiButton(..) so it
matches (that picks MD or regular label button depending on settings). Done.
Also fixed the `show` button on the main dialog.

We can pass --secondary-ui-md to views_examples to switch between MD/non-MD.

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
ui/views/examples/dialog_example.cc:106: // WidgetDelgate:
On 2017/02/22 05:58:34, Patti Lor wrote:
> WidgetDelegate (typo)

Done.

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
File ui/views/examples/dialog_example.h (right):

https://codereview.chromium.org/2701373002/diff/40001/ui/views/examples/dialo...
ui/views/examples/dialog_example.h:40: template <class>
On 2017/02/22 05:58:34, Patti Lor wrote:
> I haven't really used cpp templates much - does that need to be <class
T>/<class
> DialogType> or is it fine without?

For forward-declares it's fine without -- compiler just needs to know the number
of parameters.

[commit-bot: I haz the power, 2017-02-22 22:45:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 00:19:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 00:19:15]

Dry run: This issue passed the CQ dry run.

[tapted, 2017-02-23 00:24:52]

The CQ bit was checked by tapted@chromium.org

[tapted, 2017-02-23 00:24:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
patricialor@chromium.org
Link to the patchset: https://codereview.chromium.org/2701373002/#ps80001
(title: "respond to comments")

[commit-bot: I haz the power, 2017-02-23 00:25:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 00:40:52]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487809492639010,
"parent_rev": "99fc4ddff50e25a6c65b2ab95a3d6d2cbe1c226f", "commit_rev":
"b83507471b95f9fd24b95e6cc720c6e44ce92f8f"}

[commit-bot: I haz the power, 2017-02-23 00:41:32]

Description was changed from

==========
Add a DialogExample example to views_examples.

See http://crbug.com/671820#c10 for how it looks.

This is helpful for demonstrating behaviours and experimenting with
layout, e.g., for long strings.

BUG=671820
==========

to

==========
Add a DialogExample example to views_examples.

See http://crbug.com/671820#c10 for how it looks.

This is helpful for demonstrating behaviours and experimenting with
layout, e.g., for long strings.

BUG=671820

Review-Url: https://codereview.chromium.org/2701373002
Cr-Commit-Position: refs/heads/master@{#452311}
Committed:
https://chromium.googlesource.com/chromium/src/+/b83507471b95f9fd24b95e6cc720...
==========

[commit-bot: I haz the power, 2017-02-23 00:41:35]

Committed patchset #3 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/b83507471b95f9fd24b95e6cc720...