[Payments] Fix up field widths in desktop editors.

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

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/payments/editor_view_controller.h"
 
 #include <map>
 #include <memory>
 #include <utility>
 
@@ skipping 172 matching lines @@
   text_fields_.clear();
   comboboxes_.clear();
 
   std::unique_ptr<views::GridLayout> editor_layout =
       base::MakeUnique<views::GridLayout>(editor_view.get());
 
   // The editor grid layout is padded horizontally.
   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_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 ;)

 
-  columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER, 0,
-                     views::GridLayout::USE_PREF, 0, 0);
+  // 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.
   views::GridLayout* layout_handle = editor_layout.get();
   editor_view->SetLayoutManager(editor_layout.release());
   std::vector<EditorField> fields = GetFieldDefinitions();
   for (const auto& field : fields)
     CreateInputField(layout_handle, field);
 
   return editor_view;
 }
 
 // Each input field is a 4-quadrant grid.
 // +----------------------------------------------------------+
 // | Field Label           | Input field (textfield/combobox) |
 // |_______________________|__________________________________|
 // |   (empty)             | Error label                      |
 // +----------------------------------------------------------+
 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);
   // A very long label will wrap. Value picked so that left + right label
   // padding bring the label to half-way in the dialog (~225).
   constexpr int kMaximumLabelWidth = 192;
   label->SetMultiLine(true);
   label->SetMaximumWidth(kMaximumLabelWidth);
   layout->AddView(label.release());
 
@@ skipping 27 matching lines @@
     if (!first_field_view_)
       first_field_view_ = combobox;
 
     // |combobox| will now be owned by |row|.
     layout->AddView(combobox, 1, 1, views::GridLayout::FILL,
                     views::GridLayout::FILL, 0, kInputFieldHeight);
   } else {
     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>();
   error_label_view->SetLayoutManager(new views::FillLayout);
   error_labels_[field] = error_label_view.get();
   layout->AddView(error_label_view.release());
 
   // Bottom padding for the row.
   layout->AddPaddingRow(0, kInputRowSpacing);
 }
 
 }  // namespace payments