[Payments] Display payment method and contact summary in a single line in bottom sheet

Display payment method and contact summary in a single line in bottom sheet

BUG=655823, 655822
Committed: https://crrev.com/3458e46a4da95a4cb769a61908af8772b9fc3a8c
Cr-Commit-Position: refs/heads/master@{#433296}

[gogerald1, 2016-11-18 04:18:29]

Patchset #1 (id:1) has been deleted

[gogerald1, 2016-11-18 16:42:53]

Patchset #1 (id:20001) has been deleted

[gogerald1, 2016-11-18 16:44:25]

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

[commit-bot: I haz the power, 2016-11-18 16:45:39]

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

[gogerald1, 2016-11-18 16:48:53]

Description was changed from

==========
format


display summary in a single line for payment and contact section

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

to

==========
Display payment method and contact summary in a single line in bottom sheet

BUG=655823,655822
==========

[gogerald1, 2016-11-18 17:30:17]

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

[gogerald1, 2016-11-18 17:31:07]

Patchset #1 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-11-18 17:31:21]

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

[gogerald1, 2016-11-18 17:34:33]

gogerald@chromium.org changed reviewers:
+ rouslan@chromium.org

[gogerald1, 2016-11-18 17:34:34]

Hi Rouslan, PTAL. This CL does not contain much Android UI related stuff, so I
did not send to Android UI reviewers.

[please use gerrit instead, 2016-11-18 18:02:26]

LGTM after comments. Good job!

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
(right):

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:251:
* @param leftTruncate How to truncate the left summary text.  Set to null to
clear.
Add @param lines for leftIsSingleLine and rightIsSingleLine.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:845:
mOption, mDelegate.isBoldLabelNeeded(OptionSection.this), false));
Add a comment about what false is. Like so:

mDelegate.isBoldLabelNeeded(OptionSection.this), false /* parameterName */));

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:949:
* single line.
"Indicates whether the summary should be a single line."

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:1186:
String labelSeparator = singleLine ? ", " : "\n";
Please use R.string.autofill_address_summary_separator, which is translated.
Some languages use different characters for commas or don't use commas at all.

You can find that string here:

https://cs.chromium.org/chromium/src/components/autofill_strings.grdp?rcl=0&l=81

It needs formatter_data="android_java" to be used here.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java
(right):

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:457:
mShippingSummarySection.setSummaryProperties(null, false, null, false);
As a general rule of thumb, add parameter names in comments after using literals
like "true", "false", and "null" when it's not immediately clear from the
function name what these parameter are.

mShippingSummarySection.setSummaryProperties(null /* parameterName0 */,
        false /* parameterName1 */, null /* parameterName2 */,
        false /* parameterName3 */);

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:467:
TruncateAt.END, true, null, false);
Add parameter name comments.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:481:
mContactDetailsSection.setSummaryProperties(TruncateAt.END, true, null, false);
Add parameter name comments. Here TruncateAt.END does not need a comment,
because one may be able to figure out what that parameter does.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:486:
mPaymentMethodSection.setSummaryProperties(TruncateAt.END, true, null, false);
Add parameter name comments.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:982:
mContactDetailsSection.setSummaryProperties(null, false, null, false);
Add parameter name comments.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:983:
mPaymentMethodSection.setSummaryProperties(null, false, null, false);
Add parameter name comments.

[gogerald1, 2016-11-18 18:50:59]

Patchset #2 (id:80001) has been deleted

[gogerald1, 2016-11-18 18:57:26]

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

[commit-bot: I haz the power, 2016-11-18 18:58:04]

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

[gogerald1, 2016-11-18 19:10:30]

Patchset #2 (id:100001) has been deleted

[gogerald1, 2016-11-18 19:12:39]

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

[commit-bot: I haz the power, 2016-11-18 19:13:15]

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

[gogerald1, 2016-11-18 19:13:54]

gogerald@chromium.org changed reviewers:
+ mathp@chromium.org

[gogerald1, 2016-11-18 19:13:55]

Hi Mathieu, PTAL of the change in autofill_strings.grdp, Thanks.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
(right):

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:251:
* @param leftTruncate How to truncate the left summary text.  Set to null to
clear.
On 2016/11/18 18:02:26, rouslan wrote:
> Add @param lines for leftIsSingleLine and rightIsSingleLine.

Done.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:845:
mOption, mDelegate.isBoldLabelNeeded(OptionSection.this), false));
On 2016/11/18 18:02:26, rouslan wrote:
> Add a comment about what false is. Like so:
> 
> mDelegate.isBoldLabelNeeded(OptionSection.this), false /* parameterName */));

Done.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:949:
* single line.
On 2016/11/18 18:02:26, rouslan wrote:
> "Indicates whether the summary should be a single line."

Done.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:1186:
String labelSeparator = singleLine ? ", " : "\n";
On 2016/11/18 18:02:26, rouslan wrote:
> Please use R.string.autofill_address_summary_separator, which is translated.
> Some languages use different characters for commas or don't use commas at all.
> 
> You can find that string here:
> 
>
https://cs.chromium.org/chromium/src/components/autofill_strings.grdp?rcl=0&l=81
> 
> It needs formatter_data="android_java" to be used here.

Done.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java
(right):

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:457:
mShippingSummarySection.setSummaryProperties(null, false, null, false);
On 2016/11/18 18:02:26, rouslan wrote:
> As a general rule of thumb, add parameter names in comments after using
literals
> like "true", "false", and "null" when it's not immediately clear from the
> function name what these parameter are.
> 
> mShippingSummarySection.setSummaryProperties(null /* parameterName0 */,
>         false /* parameterName1 */, null /* parameterName2 */,
>         false /* parameterName3 */);

Done.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:467:
TruncateAt.END, true, null, false);
On 2016/11/18 18:02:26, rouslan wrote:
> Add parameter name comments.

Done.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:481:
mContactDetailsSection.setSummaryProperties(TruncateAt.END, true, null, false);
On 2016/11/18 18:02:26, rouslan wrote:
> Add parameter name comments. Here TruncateAt.END does not need a comment,
> because one may be able to figure out what that parameter does.

Done.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:486:
mPaymentMethodSection.setSummaryProperties(TruncateAt.END, true, null, false);
On 2016/11/18 18:02:26, rouslan wrote:
> Add parameter name comments.

Done.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:982:
mContactDetailsSection.setSummaryProperties(null, false, null, false);
On 2016/11/18 18:02:26, rouslan wrote:
> Add parameter name comments.

Done.

https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:983:
mPaymentMethodSection.setSummaryProperties(null, false, null, false);
On 2016/11/18 18:02:26, rouslan wrote:
> Add parameter name comments.

Done.

[Mathieu, 2016-11-18 19:16:53]

On 2016/11/18 19:13:55, gogerald1 wrote:
> Hi Mathieu, PTAL of the change in autofill_strings.grdp, Thanks.
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java
> (right):
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:251:
> * @param leftTruncate How to truncate the left summary text.  Set to null to
> clear.
> On 2016/11/18 18:02:26, rouslan wrote:
> > Add @param lines for leftIsSingleLine and rightIsSingleLine.
> 
> Done.
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:845:
> mOption, mDelegate.isBoldLabelNeeded(OptionSection.this), false));
> On 2016/11/18 18:02:26, rouslan wrote:
> > Add a comment about what false is. Like so:
> > 
> > mDelegate.isBoldLabelNeeded(OptionSection.this), false /* parameterName
*/));
> 
> Done.
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:949:
> * single line.
> On 2016/11/18 18:02:26, rouslan wrote:
> > "Indicates whether the summary should be a single line."
> 
> Done.
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java:1186:
> String labelSeparator = singleLine ? ", " : "\n";
> On 2016/11/18 18:02:26, rouslan wrote:
> > Please use R.string.autofill_address_summary_separator, which is translated.
> > Some languages use different characters for commas or don't use commas at
all.
> > 
> > You can find that string here:
> > 
> >
>
https://cs.chromium.org/chromium/src/components/autofill_strings.grdp?rcl=0&l=81
> > 
> > It needs formatter_data="android_java" to be used here.
> 
> Done.
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java
> (right):
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:457:
> mShippingSummarySection.setSummaryProperties(null, false, null, false);
> On 2016/11/18 18:02:26, rouslan wrote:
> > As a general rule of thumb, add parameter names in comments after using
> literals
> > like "true", "false", and "null" when it's not immediately clear from the
> > function name what these parameter are.
> > 
> > mShippingSummarySection.setSummaryProperties(null /* parameterName0 */,
> >         false /* parameterName1 */, null /* parameterName2 */,
> >         false /* parameterName3 */);
> 
> Done.
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:467:
> TruncateAt.END, true, null, false);
> On 2016/11/18 18:02:26, rouslan wrote:
> > Add parameter name comments.
> 
> Done.
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:481:
> mContactDetailsSection.setSummaryProperties(TruncateAt.END, true, null,
false);
> On 2016/11/18 18:02:26, rouslan wrote:
> > Add parameter name comments. Here TruncateAt.END does not need a comment,
> > because one may be able to figure out what that parameter does.
> 
> Done.
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:486:
> mPaymentMethodSection.setSummaryProperties(TruncateAt.END, true, null, false);
> On 2016/11/18 18:02:26, rouslan wrote:
> > Add parameter name comments.
> 
> Done.
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:982:
> mContactDetailsSection.setSummaryProperties(null, false, null, false);
> On 2016/11/18 18:02:26, rouslan wrote:
> > Add parameter name comments.
> 
> Done.
> 
>
https://codereview.chromium.org/2514693002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:983:
> mPaymentMethodSection.setSummaryProperties(null, false, null, false);
> On 2016/11/18 18:02:26, rouslan wrote:
> > Add parameter name comments.
> 
> Done.

Lgtm

[commit-bot: I haz the power, 2016-11-18 20:17:02]

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

[commit-bot: I haz the power, 2016-11-18 20:17:03]

Dry run: This issue passed the CQ dry run.

[gogerald1, 2016-11-18 20:20:33]

The CQ bit was checked by gogerald@chromium.org

[gogerald1, 2016-11-18 20:20:34]

The patchset sent to the CQ was uploaded after l-g-t-m from rouslan@chromium.org
Link to the patchset: https://codereview.chromium.org/2514693002/#ps120001
(title: "address comments")

[commit-bot: I haz the power, 2016-11-18 20:21:31]

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, 2016-11-18 21:00:47]

Description was changed from

==========
Display payment method and contact summary in a single line in bottom sheet

BUG=655823,655822
==========

to

==========
Display payment method and contact summary in a single line in bottom sheet

BUG=655823,655822
==========

[commit-bot: I haz the power, 2016-11-18 21:00:50]

Committed patchset #2 (id:120001)

[commit-bot: I haz the power, 2016-11-18 21:05:03]

Description was changed from

==========
Display payment method and contact summary in a single line in bottom sheet

BUG=655823,655822
==========

to

==========
Display payment method and contact summary in a single line in bottom sheet

BUG=655823,655822

Committed: https://crrev.com/3458e46a4da95a4cb769a61908af8772b9fc3a8c
Cr-Commit-Position: refs/heads/master@{#433296}
==========

[commit-bot: I haz the power, 2016-11-18 21:05:04]

Patchset 2 (id:??) landed as
https://crrev.com/3458e46a4da95a4cb769a61908af8772b9fc3a8c
Cr-Commit-Position: refs/heads/master@{#433296}