[Payments] Fix billing address dropdown error marker overlaps the downward triangle

Fix billing address dropdown error marker overlaps the downward triangle

BUG=679758
Review-Url: https://codereview.chromium.org/2620143002
Cr-Commit-Position: refs/heads/master@{#442909}
Committed: https://chromium.googlesource.com/chromium/src/+/01e3259204180f90205e489c41bf7666effbf3ab

[gogerald1, 2017-01-10 21:38:29]

Description was changed from

==========
fix


fix overlaping

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

to

==========
Fix billing address dropdown error marker overlaps the downward triangle

BUG=679758
==========

[gogerald1, 2017-01-10 21:46:51]

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

[gogerald1, 2017-01-10 21:46:51]

Hi Ted, PTAL,

[gogerald1, 2017-01-10 21:53:23]

Minor update of the comment, ptal of the patch 2

[Ted C, 2017-01-10 22:15:33]

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

https://codereview.chromium.org/2620143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/BillingAddressAdapter.java:87:
view.getPaddingTop(), max(rect.right, rect.left), view.getPaddingBottom());
why the max here?  is this for RTL?

Do we always want the padding here to match the background of the parent?  if
so, should we not do non-relative padding?

view.setPadding(rect.left, view.getPaddingTop(), rect.right,
view.getPaddingBottom());

That should work in RTL states and I think is a bit clearer...that is of course
if this is what is intended.

[gogerald1, 2017-01-10 23:17:37]

Patchset #3 (id:40001) has been deleted

[gogerald1, 2017-01-10 23:18:48]

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

[commit-bot: I haz the power, 2017-01-10 23:19:33]

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

[gogerald1, 2017-01-10 23:21:06]

Patchset #3 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-01-10 23:22:33]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[gogerald1, 2017-01-10 23:22:58]

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

[gogerald1, 2017-01-10 23:23:16]

please take another look

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

https://codereview.chromium.org/2620143002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/BillingAddressAdapter.java:87:
view.getPaddingTop(), max(rect.right, rect.left), view.getPaddingBottom());
On 2017/01/10 22:15:33, Ted C wrote:
> why the max here?  is this for RTL?
> 
> Do we always want the padding here to match the background of the parent?  if
> so, should we not do non-relative padding?
> 
> view.setPadding(rect.left, view.getPaddingTop(), rect.right,
> view.getPaddingBottom());
> 
> That should work in RTL states and I think is a bit clearer...that is of
course
> if this is what is intended.

We do not always want the padding here to match the background of the parent
since we use "style="?android:attr/spinnerItemStyle"" for the view which defines
8dp left and right padding.

But it looks we do could solve the RTL problem by using setPadding instead of
setRelativePadding since the rect.left and rect.right value will change when
changing RTL

[commit-bot: I haz the power, 2017-01-10 23:23:48]

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

[Ted C, 2017-01-10 23:28:34]

lgtm

[commit-bot: I haz the power, 2017-01-11 00:53:02]

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

[commit-bot: I haz the power, 2017-01-11 00:53:03]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[gogerald1, 2017-01-11 03:26:07]

The CQ bit was checked by gogerald@chromium.org

[commit-bot: I haz the power, 2017-01-11 03:26: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-01-11 04:49:08]

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

[commit-bot: I haz the power, 2017-01-11 04:49:09]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[gogerald1, 2017-01-11 14:18:58]

The CQ bit was checked by gogerald@chromium.org

[commit-bot: I haz the power, 2017-01-11 14:19:13]

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-01-11 15:33:14]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1484144338482970,
"parent_rev": "072dc547306ce82b35b6ef90f0917cbcaeb65534", "commit_rev":
"01e3259204180f90205e489c41bf7666effbf3ab"}

[commit-bot: I haz the power, 2017-01-11 15:33:46]

Description was changed from

==========
Fix billing address dropdown error marker overlaps the downward triangle

BUG=679758
==========

to

==========
Fix billing address dropdown error marker overlaps the downward triangle

BUG=679758

Review-Url: https://codereview.chromium.org/2620143002
Cr-Commit-Position: refs/heads/master@{#442909}
Committed:
https://chromium.googlesource.com/chromium/src/+/01e3259204180f90205e489c41bf...
==========

[commit-bot: I haz the power, 2017-01-11 15:33:47]

Committed patchset #3 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/01e3259204180f90205e489c41bf...