[Payments] Fix up field widths in desktop editors.

chrome/browser/ui/views/payments/editor_view_controller.cc

Index: chrome/browser/ui/views/payments/editor_view_controller.cc
diff --git a/chrome/browser/ui/views/payments/editor_view_controller.cc b/chrome/browser/ui/views/payments/editor_view_controller.cc
index fa3ab44d70d1f4f7a61a4486a79ad16f0829fdcd..f7ebb63acb01414eaf13f1a4c642222e38b9e615 100644
--- a/chrome/browser/ui/views/payments/editor_view_controller.cc
+++ b/chrome/browser/ui/views/payments/editor_view_controller.cc
@@ -190,16 +190,29 @@ std::unique_ptr<views::View> EditorViewController::CreateEditorView() {
   editor_layout->SetInsets(0, payments::kPaymentRequestRowHorizontalInsets, 0,
                            payments::kPaymentRequestRowHorizontalInsets);
 
-  views::ColumnSet* columns = editor_layout->AddColumnSet(0);
-  columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER, 0,
-                     views::GridLayout::USE_PREF, 0, 0);
-
+  // Column set for short fields.
+  constexpr int kLabelWidth = 140;
+  views::ColumnSet* columns_short = editor_layout->AddColumnSet(0);
+  columns_short->AddColumn(views::GridLayout::LEADING,
+                           views::GridLayout::CENTER, 0,
+                           views::GridLayout::FIXED, kLabelWidth, 0);
   // This is the horizontal padding between the label and the input field.
   constexpr int kLabelInputFieldHorizontalPadding = 16;
-  columns->AddPaddingColumn(0, kLabelInputFieldHorizontalPadding);
-
-  columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER, 0,
-                     views::GridLayout::USE_PREF, 0, 0);
+  columns_short->AddPaddingColumn(0, kLabelInputFieldHorizontalPadding);
+  constexpr int kShortInputFieldMinimumWidth = 176;
+  columns_short->AddColumn(
+      views::GridLayout::LEADING, views::GridLayout::CENTER, 0,
+      views::GridLayout::FIXED, kShortInputFieldMinimumWidth, 0);

[anthonyvd, 2017/05/09 21:01:21]

These variables are named "minimum width" but you're passing them to the fixed
width argument. They should either be renamed or passed to the min_width
argument.

Also, the dialog might have different widths soon in non-english locales. You
might want to make sure this works *without* a fixed width for the field (fixed
for the labels is fine).

[Mathieu, 2017/05/10 12:30:35]

Done. 

The fixed width for input fields (176/272px) is a requirement of the spec. Are
you saying the dialog could get narrower? I think there is still a little space
on the right to allow for that. We could also make kLabelWidth a bit shorter.
Can we open a bug for non-english locales? The UI is kinda broken right now
without this change.

[anthonyvd, 2017/05/10 13:26:02]

It's the opposite: if the strings in buttons are long, the dialog will have to
grow in 16px increments according the Harmony. Having fixed fields in that case
will make it so there's potentially a lot of white space to the right of the
fields.

I think the spec gives fixed values that assume a fixed width dialog, which
isn't the case. We should probably have either

1. Fixed width expressed as fixed_width = (current_dialog_width - label_width -
trailing_padding) for a value of trailing_padding that matches the current spec
for the base dialog width (512px)
2. Preserve the short/long field width ratio by not specifying a fixed width but
specifying the proper resize_percent on the field's column and maybe a trailing
padding column

FWIW I think 1. makes more sense (and is probably simpler).
I mean we haven't shipped yet, might as well do it right from the start ;)

+
+  // Column set for long fields.
+  views::ColumnSet* columns_long = editor_layout->AddColumnSet(1);
+  columns_long->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER,
+                          0, views::GridLayout::FIXED, kLabelWidth, 0);
+  columns_long->AddPaddingColumn(0, kLabelInputFieldHorizontalPadding);
+  constexpr int kLongInputFieldMinimumWidth = 272;
+  columns_long->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER,
+                          0, views::GridLayout::FIXED,
+                          kLongInputFieldMinimumWidth, 0);
 
   // The LayoutManager needs to be set before input fields are created, so we
   // keep a handle to it before we release it to the view.
@@ -220,9 +233,12 @@ std::unique_ptr<views::View> EditorViewController::CreateEditorView() {
 // +----------------------------------------------------------+
 void EditorViewController::CreateInputField(views::GridLayout* layout,
                                             const EditorField& field) {
+  int column_set =
+      field.length_hint == EditorField::LengthHint::HINT_SHORT ? 0 : 1;
+
   // This is the top padding for every row.
   constexpr int kInputRowSpacing = 6;
-  layout->StartRowWithPadding(0, 0, 0, kInputRowSpacing);
+  layout->StartRowWithPadding(0, column_set, 0, kInputRowSpacing);
 
   std::unique_ptr<views::Label> label = base::MakeUnique<views::Label>(
       field.required ? field.label + base::ASCIIToUTF16("*") : field.label);
@@ -270,7 +286,7 @@ void EditorViewController::CreateInputField(views::GridLayout* layout,
     NOTREACHED();
   }
 
-  layout->StartRow(0, 0);
+  layout->StartRow(0, column_set);
   layout->SkipColumns(1);
   std::unique_ptr<views::View> error_label_view =
       base::MakeUnique<views::View>();