Remove references to ui/views/layout/layout_constants.h

Remove references to ui/views/layout/layout_constants.h

Replace references to constants in ui/views/layout/layout_constants.h with their
equivalents using ChromeLayoutProvider.

BUG=691897
TBR=oshima

Review-Url: https://codereview.chromium.org/2928253002
Cr-Commit-Position: refs/heads/master@{#478873}
Committed: https://chromium.googlesource.com/chromium/src/+/b1e8ef3d59841b147bf079b59cfba23abfa80aa8

[ananta, 2017-06-10 01:22:05]

ananta@chromium.org changed reviewers:
+ pkasting@chromium.org

[Peter Kasting, 2017-06-10 01:41:37]

https://codereview.chromium.org/2928253002/diff/20001/chrome/browser/ui/ash/DEPS
File chrome/browser/ui/ash/DEPS (right):

https://codereview.chromium.org/2928253002/diff/20001/chrome/browser/ui/ash/D...
chrome/browser/ui/ash/DEPS:6:
"+chrome/browser/ui/views/harmony/chrome_layout_provider.h",
I'm not sure what the right answer here is.

Maybe this DEPS file should just add +chrome/browser/ui/views/, if it makes
sense in general for ash files to depend on views stuff.

I would like sky's opinion.

https://codereview.chromium.org/2928253002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc (right):

https://codereview.chromium.org/2928253002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc:222:
views::DISTANCE_DIALOG_CONTENTS_VERTICAL_MARGIN)));
This doesn't look like the right constant.  AFAICT it should be the unrelated
vertical spacing?  What does this dialog look like, and where in the dialog is
this showing up?

https://codereview.chromium.org/2928253002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc:226:
views::DISTANCE_DIALOG_CONTENTS_HORIZONTAL_MARGIN);
It looks to me like these are the left() and right() parts of the dialog insets.

[ananta, 2017-06-10 03:36:43]

ananta@chromium.org changed reviewers:
+ sky@chromium.org

[ananta, 2017-06-10 03:36:44]

+sky

https://codereview.chromium.org/2928253002/diff/20001/chrome/browser/ui/ash/DEPS
File chrome/browser/ui/ash/DEPS (right):

https://codereview.chromium.org/2928253002/diff/20001/chrome/browser/ui/ash/D...
chrome/browser/ui/ash/DEPS:6:
"+chrome/browser/ui/views/harmony/chrome_layout_provider.h",
On 2017/06/10 01:41:37, Peter Kasting wrote:
> I'm not sure what the right answer here is.
> 
> Maybe this DEPS file should just add +chrome/browser/ui/views/, if it makes
> sense in general for ash files to depend on views stuff.
> 
> I would like sky's opinion.

Will add sky to the patchset.

https://codereview.chromium.org/2928253002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc (right):

https://codereview.chromium.org/2928253002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc:222:
views::DISTANCE_DIALOG_CONTENTS_VERTICAL_MARGIN)));
On 2017/06/10 01:41:37, Peter Kasting wrote:
> This doesn't look like the right constant.  AFAICT it should be the unrelated
> vertical spacing?  What does this dialog look like, and where in the dialog is
> this showing up?

Sent you an email with the image comparing the two side by side. This is the
padding between the "The account is managed by xyz.com" text and the explanation
label just under it.

I changed to DISTANCE_UNRELATED_CONTROL_VERTICAL

https://codereview.chromium.org/2928253002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc:226:
views::DISTANCE_DIALOG_CONTENTS_HORIZONTAL_MARGIN);
On 2017/06/10 01:41:37, Peter Kasting wrote:
> It looks to me like these are the left() and right() parts of the dialog
insets.

Thanks done.

[chromium-reviews, 2017-06-10 03:38:47]

Attached a bitmap showing the differences between the two dialogs. The left
one is the one with the current offset which is the kPanelVertMargin (13).
The right one is one with DISTANCE_UNRELATED_CONTROL_VERTICAL (20)



On Fri, Jun 9, 2017 at 8:36 PM, <ananta@chromium.org> wrote:

> +sky
>
>
> https://codereview.chromium.org/2928253002/diff/20001/
> chrome/browser/ui/ash/DEPS
> File chrome/browser/ui/ash/DEPS (right):
>
> https://codereview.chromium.org/2928253002/diff/20001/
> chrome/browser/ui/ash/DEPS#newcode6
> chrome/browser/ui/ash/DEPS:6:
> "+chrome/browser/ui/views/harmony/chrome_layout_provider.h",
> On 2017/06/10 01:41:37, Peter Kasting wrote:
> > I'm not sure what the right answer here is.
> >
> > Maybe this DEPS file should just add +chrome/browser/ui/views/, if it
> makes
> > sense in general for ash files to depend on views stuff.
> >
> > I would like sky's opinion.
>
> Will add sky to the patchset.
>
> https://codereview.chromium.org/2928253002/diff/20001/
> chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc
> File chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc
> (right):
>
> https://codereview.chromium.org/2928253002/diff/20001/
> chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc#
> newcode222
> chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc:222:
> views::DISTANCE_DIALOG_CONTENTS_VERTICAL_MARGIN)));
> On 2017/06/10 01:41:37, Peter Kasting wrote:
> > This doesn't look like the right constant. AFAICT it should be the
> unrelated
> > vertical spacing? What does this dialog look like, and where in the
> dialog is
> > this showing up?
>
> Sent you an email with the image comparing the two side by side. This is
> the padding between the "The account is managed by xyz.com" text and the
> explanation label just under it.
>
> I changed to DISTANCE_UNRELATED_CONTROL_VERTICAL
>
> https://codereview.chromium.org/2928253002/diff/20001/
> chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc#
> newcode226
> chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc:226:
> views::DISTANCE_DIALOG_CONTENTS_HORIZONTAL_MARGIN);
> On 2017/06/10 01:41:37, Peter Kasting wrote:
> > It looks to me like these are the left() and right() parts of the
> dialog insets.
>
> Thanks done.
>
> https://codereview.chromium.org/2928253002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[ananta, 2017-06-10 03:41:20]

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

[commit-bot: I haz the power, 2017-06-10 03:41:27]

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-06-10 04:25:57]

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

[commit-bot: I haz the power, 2017-06-10 04:25:58]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-06-12 22:56:01]

LGTM with change

https://codereview.chromium.org/2928253002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc (right):

https://codereview.chromium.org/2928253002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc:220:
views::DISTANCE_UNRELATED_CONTROL_VERTICAL));
OK, this is the rare case where using the .top() of INSETS_DIALOG_CONTENTS
actually makes sense; this is effectively the start of the "main content area".

[ananta, 2017-06-12 23:07:53]

https://codereview.chromium.org/2928253002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc (right):

https://codereview.chromium.org/2928253002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/forced_reauthentication_dialog.cc:220:
views::DISTANCE_UNRELATED_CONTROL_VERTICAL));
On 2017/06/12 22:56:01, Peter Kasting wrote:
> OK, this is the rare case where using the .top() of INSETS_DIALOG_CONTENTS
> actually makes sense; this is effectively the start of the "main content
area".

Thanks done.

[ananta, 2017-06-12 23:07:58]

The CQ bit was checked by ananta@chromium.org

[ananta, 2017-06-12 23:07:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2928253002/#ps60001
(title: "Address review comments")

[commit-bot: I haz the power, 2017-06-12 23:08:36]

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-06-12 23:19:52]

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

[commit-bot: I haz the power, 2017-06-12 23:19:53]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ananta, 2017-06-12 23:34:19]

Description was changed from

==========
Remove references to ui/views/layout/layout_constants.h

Replace references to constants in ui/views/layout/layout_constants.h with their
equivalents using ChromeLayoutProvider.

BUG=691897
==========

to

==========
Remove references to ui/views/layout/layout_constants.h

Replace references to constants in ui/views/layout/layout_constants.h with their
equivalents using ChromeLayoutProvider.

BUG=691897
TBR=oshima
==========

[ananta, 2017-06-12 23:34:19]

ananta@chromium.org changed reviewers:
+ oshima@chromium.org

[ananta, 2017-06-12 23:34:54]

+oshima for chrome/browser/chromeos owners. TBR'ing as it is a simple formatting
change in the DEPS file

[ananta, 2017-06-12 23:35:02]

The CQ bit was checked by ananta@chromium.org

[commit-bot: I haz the power, 2017-06-12 23:35:32]

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-06-13 00:50:23]

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

[commit-bot: I haz the power, 2017-06-13 00:50:24]

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

[ananta, 2017-06-13 01:03:07]

The CQ bit was checked by ananta@chromium.org

[ananta, 2017-06-13 01:03:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2928253002/#ps80001
(title: "Fix redness")

[commit-bot: I haz the power, 2017-06-13 01:03:42]

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-06-13 01:19:07]

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

[commit-bot: I haz the power, 2017-06-13 01:19:08]

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

[ananta, 2017-06-13 01:35:47]

The CQ bit was checked by ananta@chromium.org

[ananta, 2017-06-13 01:35:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2928253002/#ps100001
(title: "Fix build errors")

[commit-bot: I haz the power, 2017-06-13 01:36:16]

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-06-13 02:56:19]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1497317747362840,
"parent_rev": "ba7195e2875e1e36a709caa5c0dabaa934e01c1a", "commit_rev":
"b1e8ef3d59841b147bf079b59cfba23abfa80aa8"}

[commit-bot: I haz the power, 2017-06-13 02:56:34]

Description was changed from

==========
Remove references to ui/views/layout/layout_constants.h

Replace references to constants in ui/views/layout/layout_constants.h with their
equivalents using ChromeLayoutProvider.

BUG=691897
TBR=oshima
==========

to

==========
Remove references to ui/views/layout/layout_constants.h

Replace references to constants in ui/views/layout/layout_constants.h with their
equivalents using ChromeLayoutProvider.

BUG=691897
TBR=oshima

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

[commit-bot: I haz the power, 2017-06-13 02:56:35]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/b1e8ef3d59841b147bf079b59cfb...