[Payments] Style the add address option in billing address selection.

[Payments] Style the add address option in billing address selection.

BUG=661249
Committed: https://crrev.com/6ff2921f019f376f6099436d0b4f8f3ddcac5a45
Cr-Commit-Position: refs/heads/master@{#430740}

[sebsg, 2016-11-03 18:02:10]

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

[sebsg, 2016-11-03 18:02:10]

Hi Rouslan, PTAL?

I have a few ideas on how to generalize the adapter but I thought I should
probably wait for a use-case before doing it. WDYT?

Thanks!

[please use gerrit instead, 2016-11-03 18:46:13]

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

[please use gerrit instead, 2016-11-03 18:46:14]

ianwen, Android expert ptal.

sebsg, there's no need to generalize, because this UI is not going to be used
anywhere else, from what I understand.

https://codereview.chromium.org/2479613002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/HintArrayAdapter.java
(right):

https://codereview.chromium.org/2479613002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/HintArrayAdapter.java:23:
* options. The last element is the hint.
Add a comment about the + icon in the last element.

https://codereview.chromium.org/2479613002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/HintArrayAdapter.java:27:
public class HintArrayAdapter<T> extends ArrayAdapter<T> {
This is used only for billing addresses. Let's call it BillingAddressAdapter?

[sebsg, 2016-11-04 13:21:41]

Patchset #2 (id:20001) has been deleted

[sebsg, 2016-11-04 13:26:45]

Patchset #2 (id:40001) has been deleted

[sebsg, 2016-11-04 13:42:06]

Thanks Rouslan!

https://codereview.chromium.org/2479613002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/HintArrayAdapter.java
(right):

https://codereview.chromium.org/2479613002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/HintArrayAdapter.java:23:
* options. The last element is the hint.
On 2016/11/03 18:46:13, rouslan wrote:
> Add a comment about the + icon in the last element.

Done.

https://codereview.chromium.org/2479613002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/HintArrayAdapter.java:27:
public class HintArrayAdapter<T> extends ArrayAdapter<T> {
On 2016/11/03 18:46:13, rouslan wrote:
> This is used only for billing addresses. Let's call it BillingAddressAdapter? 

Done.

[Ian Wen, 2016-11-04 20:10:38]

ianwen@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[Ian Wen, 2016-11-04 20:10:38]

In general I prefer to use type in adapter rather than directly manipulating the
count and id. Yet I will pass to dfalcantara@ for scrutiny.

[gone, 2016-11-04 20:52:05]

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

https://codereview.chromium.org/2479613002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/BillingAddressAdapter.java:48:
return count > 0 ? count - 1 : count;
Yeah, Ian's right about this being slightly strange; we always count headers as
being proper items in the list because they're separate views.

You should just report the proper count and then key off that position when
getDropDownView() is called to give the View that's expected (one showing + ADD
ADDRESS) when the last index is requested.

[please use gerrit instead, 2016-11-04 20:54:08]

LGTM from my end, but please get an LG from either Ian or DAN as well.

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

https://codereview.chromium.org/2479613002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/BillingAddressAdapter.java:23:
* options. The last element is the hint and the element before has a "+" icon on
its left an a blue
s/an a blue/and a blue/

[sebsg, 2016-11-08 19:48:50]

Patchset #3 (id:80001) has been deleted

[sebsg, 2016-11-08 19:53:33]

Hi Dan modified the code and added comments to make it more clear. WDYT?

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

https://codereview.chromium.org/2479613002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/BillingAddressAdapter.java:23:
* options. The last element is the hint and the element before has a "+" icon on
its left an a blue
On 2016/11/04 20:54:08, rouslan wrote:
> s/an a blue/and a blue/

Done.

https://codereview.chromium.org/2479613002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/BillingAddressAdapter.java:48:
return count > 0 ? count - 1 : count;
On 2016/11/04 20:52:05, dfalcantara (check my queue) wrote:
> Yeah, Ian's right about this being slightly strange; we always count headers
as
> being proper items in the list because they're separate views.
> 
> You should just report the proper count and then key off that position when
> getDropDownView() is called to give the View that's expected (one showing +
ADD
> ADDRESS) when the last index is requested.

After chat it was decided to keep it as is but make it more clear.

[gone, 2016-11-08 21:10:47]

lgtm % comments

https://codereview.chromium.org/2479613002/diff/140001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/BillingAddressAdapter.java
(right):

https://codereview.chromium.org/2479613002/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/BillingAddressAdapter.java:43:
* Creates an array adapter for which the last element is a hint taht is not
shown in the
*that

https://codereview.chromium.org/2479613002/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/BillingAddressAdapter.java:61:
super.add(hint);
Shouldn't have to say super.add()... can you just call add()?

[sebsg, 2016-11-08 21:18:11]

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

[commit-bot: I haz the power, 2016-11-08 21:19:13]

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, 2016-11-08 22:30:22]

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

[commit-bot: I haz the power, 2016-11-08 22:30:24]

Dry run: This issue passed the CQ dry run.

[sebsg, 2016-11-08 22:41:51]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2016-11-08 22:41:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
rouslan@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2479613002/#ps160001
(title: "Addressed Dan's comments")

[commit-bot: I haz the power, 2016-11-08 22:42:21]

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-08 22:50:51]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2016-11-08 22:53:23]

Description was changed from

==========
[Payments] Style the add address option in billing address selection.

BUG=661249
==========

to

==========
[Payments] Style the add address option in billing address selection.

BUG=661249

Committed: https://crrev.com/6ff2921f019f376f6099436d0b4f8f3ddcac5a45
Cr-Commit-Position: refs/heads/master@{#430740}
==========

[commit-bot: I haz the power, 2016-11-08 22:53:24]

Patchset 6 (id:??) landed as
https://crrev.com/6ff2921f019f376f6099436d0b4f8f3ddcac5a45
Cr-Commit-Position: refs/heads/master@{#430740}