Fix graph labels on Data Saver settings page for RTL

Fix graph labels on Data Saver settings page for RTL

Currently, the labels are swapped on RTL layouts and both pushed to the
left. The start date label should remain on the left and the end date
label should remain on the right even when the layout is RTL.

BUG=710651
Review-Url: https://codereview.chromium.org/2809123003
Cr-Commit-Position: refs/heads/master@{#464218}
Committed: https://chromium.googlesource.com/chromium/src/+/52ecff3c080ea3727c0948d404dfeedf7022ce88

[megjablon, 2017-04-11 23:51:10]

megjablon@chromium.org changed reviewers:
+ twellington@chromium.org

[megjablon, 2017-04-11 23:51:11]

PTAL, thanks!

[Theresa, 2017-04-11 23:55:01]

https://codereview.chromium.org/2809123003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionStatsPreference.java
(right):

https://codereview.chromium.org/2809123003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionStatsPreference.java:136:
startDateParams.gravity = Gravity.LEFT;
Can you set android:gravity="left" in the XML rather than programatically here?

It'd also be good to leave a code comment explaining what the bug states (that
plots and labels remain LTR in RTL languages).

[megjablon, 2017-04-12 00:04:42]

https://codereview.chromium.org/2809123003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionStatsPreference.java
(right):

https://codereview.chromium.org/2809123003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionStatsPreference.java:136:
startDateParams.gravity = Gravity.LEFT;
On 2017/04/11 23:55:01, Theresa wrote:
> Can you set android:gravity="left" in the XML rather than programatically
here?
> 
> It'd also be good to leave a code comment explaining what the bug states (that
> plots and labels remain LTR in RTL languages).
Added comment.

When I do it in XML I get the AssertNotDeprecatedAttribute build error:

https://cs.chromium.org/chromium/src/build/android/gyp/generate_v14_compatibl...

I wasn't sure the best way to get around that, and was hoping if this wasn't the
correct way you might have a suggestion.

[Theresa, 2017-04-12 00:36:21]

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

[Theresa, 2017-04-12 00:36:22]

Unfortunately both of the engineers who worked on that build rule are no longer
on the team. +tedchoc@ - do you know if there's any way to suppress the build
error thrown when specifying android:gravity="right" or ="left" in XML?

[Ted C, 2017-04-12 16:18:12]

On 2017/04/12 00:36:22, Theresa wrote:
> Unfortunately both of the engineers who worked on that build rule are no
longer
> on the team. +tedchoc@ - do you know if there's any way to suppress the build
> error thrown when specifying android:gravity="right" or ="left" in XML?

Sadly, no I don't think there is a way.

See a similar comment in:
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...

I think it is worth thinking about how we could do this via suppressions
in XML, but I don't have a great suggestion off the top of my head for
how to solve this.  I think it uses just standard xml traversal and we'd
have to find a way to put these suppressions in there in a way that doesn't
break v17+ since I believe we use the files as is.

[megjablon, 2017-04-12 18:57:35]

On 2017/04/12 16:18:12, Ted C wrote:
> On 2017/04/12 00:36:22, Theresa wrote:
> > Unfortunately both of the engineers who worked on that build rule are no
> longer
> > on the team. +tedchoc@ - do you know if there's any way to suppress the
build
> > error thrown when specifying android:gravity="right" or ="left" in XML?
> 
> Sadly, no I don't think there is a way.
> 
> See a similar comment in:
>
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...
> 
> I think it is worth thinking about how we could do this via suppressions
> in XML, but I don't have a great suggestion off the top of my head for
> how to solve this.  I think it uses just standard xml traversal and we'd
> have to find a way to put these suppressions in there in a way that doesn't
> break v17+ since I believe we use the files as is.

Can I land this while we come up with a solution to do this in suppressions?

[Theresa, 2017-04-12 21:21:11]

lgtm % one comment

https://codereview.chromium.org/2809123003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionStatsPreference.java
(right):

https://codereview.chromium.org/2809123003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionStatsPreference.java:138:
new FrameLayout.LayoutParams(LayoutParams.WRAP_CONTENT,
LayoutParams.WRAP_CONTENT);
You should be able to get the LayoutParams from the view and set the gravity in
one line:


((FrameLayout.LayoutParams) mStartDateTextView.getLayoutParams()).gravity =
Gravity.LEFT;

[megjablon, 2017-04-12 23:01:08]

https://codereview.chromium.org/2809123003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionStatsPreference.java
(right):

https://codereview.chromium.org/2809123003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionStatsPreference.java:138:
new FrameLayout.LayoutParams(LayoutParams.WRAP_CONTENT,
LayoutParams.WRAP_CONTENT);
On 2017/04/12 21:21:11, Theresa wrote:
> You should be able to get the LayoutParams from the view and set the gravity
in
> one line:
> 
> 
> ((FrameLayout.LayoutParams) mStartDateTextView.getLayoutParams()).gravity =
> Gravity.LEFT;

Done.

[megjablon, 2017-04-12 23:02:07]

The CQ bit was checked by megjablon@chromium.org

[megjablon, 2017-04-12 23:02:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org
Link to the patchset: https://codereview.chromium.org/2809123003/#ps60001
(title: "twellington comment")

[commit-bot: I haz the power, 2017-04-12 23:04:02]

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-04-13 00:16:11]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492038127764040,
"parent_rev": "797458650de1777d8334a35f7aa558c7d2b538cb", "commit_rev":
"52ecff3c080ea3727c0948d404dfeedf7022ce88"}

[commit-bot: I haz the power, 2017-04-13 00:21:17]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492038127764040,
"parent_rev": "797458650de1777d8334a35f7aa558c7d2b538cb", "commit_rev":
"52ecff3c080ea3727c0948d404dfeedf7022ce88"}

[commit-bot: I haz the power, 2017-04-13 00:21:20]

Description was changed from

==========
Fix graph labels on Data Saver settings page for RTL

Currently, the labels are swapped on RTL layouts and both pushed to the
left. The start date label should remain on the left and the end date
label should remain on the right even when the layout is RTL.

BUG=710651
==========

to

==========
Fix graph labels on Data Saver settings page for RTL

Currently, the labels are swapped on RTL layouts and both pushed to the
left. The start date label should remain on the left and the end date
label should remain on the right even when the layout is RTL.

BUG=710651

Review-Url: https://codereview.chromium.org/2809123003
Cr-Commit-Position: refs/heads/master@{#464218}
Committed:
https://chromium.googlesource.com/chromium/src/+/52ecff3c080ea3727c0948d404df...
==========

[commit-bot: I haz the power, 2017-04-13 00:21:21]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/52ecff3c080ea3727c0948d404df...