[MD settings] use arrow-forward to switch direction in rtl

[MD settings] use arrow-forward to switch direction in rtl

This CL flips the arrow-forward to left or right based on the rtl/ltr
direction setting and corrects the placement of the paper ripple under
the icon on rtl.

Minor changes: changes the old style name from .arrow-right to
.arrow-forward (since left or right changes in rtl/ltr); there are also
some css variables added to replace some constants.

BUG=651716, 586579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/f76c6502e7f4a6d57fe57aed25a60ce3845dd82a
Cr-Commit-Position: refs/heads/master@{#425837}

[dschuyler, 2016-10-03 21:57:51]

Description was changed from

==========
[MD settings] use arrow-forward to switch direction in rtl

This CL flips the arrow-forward to left or right based on the rtl/ltr
direction setting and corrects the placement of the paper ripple under
the icon on rtl.

Minor changes: changes the old style name from .arrow-right to
.arrow-forward (since left or right changes in rtl/ltr); there are also
some css variables added to replace some constants.

BUG=651716
==========

to

==========
[MD settings] use arrow-forward to switch direction in rtl

This CL flips the arrow-forward to left or right based on the rtl/ltr
direction setting and corrects the placement of the paper ripple under
the icon on rtl.

Minor changes: changes the old style name from .arrow-right to
.arrow-forward (since left or right changes in rtl/ltr); there are also
some css variables added to replace some constants.

BUG=651716
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dschuyler, 2016-10-03 21:57:53]

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

[commit-bot: I haz the power, 2016-10-03 21:58:37]

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

[dschuyler, 2016-10-03 22:02:22]

dschuyler@chromium.org changed reviewers:
+ dpapad@chromium.org

[dschuyler, 2016-10-03 22:02:22]

This CL is smaller than it appears. 

The real changes are in settings_shared_css.html.
Most of the other changes are a class name change.

[Dan Beam, 2016-10-03 22:04:54]

https://codereview.chromium.org/2390053002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2390053002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_shared_css.html:37:
:host-context([dir=rtl]) [actionable]
button[is="paper-icon-button-light"].icon-arrow-forward {
wrap

https://codereview.chromium.org/2390053002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_shared_css.html:38: background-image:
url(images/arrow_left.svg);
why can't we just transform?

[dschuyler, 2016-10-03 22:17:51]

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

[commit-bot: I haz the power, 2016-10-03 22:18:09]

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

[dschuyler, 2016-10-03 22:20:52]

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

[commit-bot: I haz the power, 2016-10-03 22:21:46]

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

[dschuyler, 2016-10-03 22:22:41]

Patch#1 and Patch#3 are both viable. 

Patch#1 uses an arrow_left.svg file.

Patch#3 uses a workaround for -webkit-margin-start.

https://codereview.chromium.org/2390053002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2390053002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_shared_css.html:37:
:host-context([dir=rtl]) [actionable]
button[is="paper-icon-button-light"].icon-arrow-forward {
On 2016/10/03 22:04:54, Dan Beam wrote:
> wrap

Removed.

https://codereview.chromium.org/2390053002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_shared_css.html:38: background-image:
url(images/arrow_left.svg);
On 2016/10/03 22:04:54, Dan Beam wrote:
> why can't we just transform?

We can. I had trouble with the -webkit-margin-start.
I've made patch#2 that shows a working version 
without the arrow_left.svg. It has some extra comments
talking about the workaround for the margin.

[dpapad, 2016-10-03 22:48:29]

dpapad@chromium.org changed reviewers:
+ dbeam@chromium.org

[dpapad, 2016-10-03 22:48:29]

Deferring this CL to dbeam@, as a CSS expert.

[dpapad, 2016-10-03 22:48:47]

dpapad@chromium.org changed reviewers:
- dpapad@chromium.org

[commit-bot: I haz the power, 2016-10-03 23:35:42]

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

[commit-bot: I haz the power, 2016-10-03 23:35:43]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-10-03 23:42:06]

https://codereview.chromium.org/2390053002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2390053002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:13: top: -8px;
why can't we just write the code in a direction-agnostic way?  i.e. margin: 8px?

https://codereview.chromium.org/2390053002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:16: /* Not using
-webkit-margin-start because we mirror the control in rtl.
why not?  it'd be one less rule you have to duplicate or override

https://codereview.chromium.org/2390053002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:34: /*
-webkit-margin-start can be upset by the transform; manually control
wait, what?  is this a transform-origin issue?

[dschuyler, 2016-10-04 00:54:46]

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

[commit-bot: I haz the power, 2016-10-04 00:55:12]

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

[dschuyler, 2016-10-04 00:56:27]

I marked the review comments ack because we talked
in person.

The new changes are much nicer, thanks.

And the @apply() that I previously added can be removed.

https://codereview.chromium.org/2390053002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2390053002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:13: top: -8px;
On 2016/10/03 23:42:06, Dan Beam wrote:
> why can't we just write the code in a direction-agnostic way?  i.e. margin:
8px?

Acknowledged.

https://codereview.chromium.org/2390053002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:16: /* Not using
-webkit-margin-start because we mirror the control in rtl.
On 2016/10/03 23:42:06, Dan Beam wrote:
> why not?  it'd be one less rule you have to duplicate or override

Acknowledged.

https://codereview.chromium.org/2390053002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:34: /*
-webkit-margin-start can be upset by the transform; manually control
On 2016/10/03 23:42:06, Dan Beam wrote:
> wait, what?  is this a transform-origin issue?

Acknowledged.

[Dan Beam, 2016-10-04 01:01:01]

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/a11y_page/a11y_page.html (right):

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/a11y_page/a11y_page.html:33: <button
class="icon-arrow-forward" is="paper-icon-button-light">
while we're renaming this, why not "subpage-arrow" or "subpage-indicator"?

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
File
chrome/browser/resources/settings/default_browser_page/default_browser_page.html
(right):

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/default_browser_page/default_browser_page.html:23:
<button class="icon-arrow-forward" is="paper-icon-button-light"></button>
oh noes

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:9:
-webkit-margin-end: -8px;
can haz comment?

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:16: width: 36px;
can haz variables?

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:20: transform:
scaleX(-1); /* Flip on the X axis (aka mirror). */
arguable nit: \s\s in front of same-line comments

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_vars_css.html (right):

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_vars_css.html:36: 
nit: revert \n\n

[dschuyler, 2016-10-04 01:13:04]

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

[commit-bot: I haz the power, 2016-10-04 01:13:24]

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

[dschuyler, 2016-10-04 01:13:58]

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/a11y_page/a11y_page.html (right):

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/a11y_page/a11y_page.html:33: <button
class="icon-arrow-forward" is="paper-icon-button-light">
On 2016/10/04 01:01:00, Dan Beam wrote:
> while we're renaming this, why not "subpage-arrow" or "subpage-indicator"?

Done.

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
File
chrome/browser/resources/settings/default_browser_page/default_browser_page.html
(right):

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/default_browser_page/default_browser_page.html:23:
<button class="icon-arrow-forward" is="paper-icon-button-light"></button>
On 2016/10/04 01:01:00, Dan Beam wrote:
> oh noes

Done.

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:9:
-webkit-margin-end: -8px;
On 2016/10/04 01:01:00, Dan Beam wrote:
> can haz comment?

Done.

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:16: width: 36px;
On 2016/10/04 01:01:00, Dan Beam wrote:
> can haz variables?

Done.

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:20: transform:
scaleX(-1); /* Flip on the X axis (aka mirror). */
On 2016/10/04 01:01:00, Dan Beam wrote:
> arguable nit: \s\s in front of same-line comments

Done.

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_vars_css.html (right):

https://codereview.chromium.org/2390053002/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_vars_css.html:36: 
On 2016/10/04 01:01:00, Dan Beam wrote:
> nit: revert \n\n

Done.

[Dan Beam, 2016-10-04 02:08:40]

https://codereview.chromium.org/2390053002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/a11y_page/a11y_page.html (right):

https://codereview.chromium.org/2390053002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/a11y_page/a11y_page.html:33: <button
class="icon-subpage-arrow" is="paper-icon-button-light">
i meant class="subpage-arrow" (i.e. drop the "icon-")

https://codereview.chromium.org/2390053002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2390053002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:13: background-size:
20px;
should this 20px be var(--settings-icon-size)?

https://codereview.chromium.org/2390053002/diff/80001/third_party/polymer/v1_...
File
third_party/polymer/v1_0/components-chromium/paper-icon-button/paper-icon-button-light.html
(left):

https://codereview.chromium.org/2390053002/diff/80001/third_party/polymer/v1_...
third_party/polymer/v1_0/components-chromium/paper-icon-button/paper-icon-button-light.html:48:
@apply(--paper-icon-button-light-ripple);
revert this.

i may have accidentally done this by running reproduce.sh

[commit-bot: I haz the power, 2016-10-04 02:35:58]

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

[commit-bot: I haz the power, 2016-10-04 02:35:59]

Dry run: This issue passed the CQ dry run.

[dschuyler, 2016-10-04 18:36:57]

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

[commit-bot: I haz the power, 2016-10-04 18:37:20]

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

[dschuyler, 2016-10-04 18:38:45]

https://codereview.chromium.org/2390053002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/a11y_page/a11y_page.html (right):

https://codereview.chromium.org/2390053002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/a11y_page/a11y_page.html:33: <button
class="icon-subpage-arrow" is="paper-icon-button-light">
On 2016/10/04 02:08:39, Dan Beam wrote:
> i meant class="subpage-arrow" (i.e. drop the "icon-")

Done.

https://codereview.chromium.org/2390053002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2390053002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:13: background-size:
20px;
On 2016/10/04 02:08:39, Dan Beam wrote:
> should this 20px be var(--settings-icon-size)?

Done.

https://codereview.chromium.org/2390053002/diff/80001/third_party/polymer/v1_...
File
third_party/polymer/v1_0/components-chromium/paper-icon-button/paper-icon-button-light.html
(left):

https://codereview.chromium.org/2390053002/diff/80001/third_party/polymer/v1_...
third_party/polymer/v1_0/components-chromium/paper-icon-button/paper-icon-button-light.html:48:
@apply(--paper-icon-button-light-ripple);
On 2016/10/04 02:08:39, Dan Beam wrote:
> revert this.
> 
> i may have accidentally done this by running reproduce.sh

Done.

[Dan Beam, 2016-10-04 19:05:55]

Description was changed from

==========
[MD settings] use arrow-forward to switch direction in rtl

This CL flips the arrow-forward to left or right based on the rtl/ltr
direction setting and corrects the placement of the paper ripple under
the icon on rtl.

Minor changes: changes the old style name from .arrow-right to
.arrow-forward (since left or right changes in rtl/ltr); there are also
some css variables added to replace some constants.

BUG=651716
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD settings] use arrow-forward to switch direction in rtl

This CL flips the arrow-forward to left or right based on the rtl/ltr
direction setting and corrects the placement of the paper ripple under
the icon on rtl.

Minor changes: changes the old style name from .arrow-right to
.arrow-forward (since left or right changes in rtl/ltr); there are also
some css variables added to replace some constants.

BUG=651716,586579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-10-04 19:57:03]

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

[commit-bot: I haz the power, 2016-10-04 19:57:04]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-10-14 02:27:45]

lgtm

https://codereview.chromium.org/2390053002/diff/100001/chrome/browser/resourc...
File
chrome/browser/resources/settings/default_browser_page/default_browser_page.html
(right):

https://codereview.chromium.org/2390053002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/default_browser_page/default_browser_page.html:24:
</button>
you get to unwrap more things!  lucky you!

[dschuyler, 2016-10-17 20:59:11]

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

[commit-bot: I haz the power, 2016-10-17 20:59:53]

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

[dschuyler, 2016-10-17 21:02:54]

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

[commit-bot: I haz the power, 2016-10-17 21:03:14]

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

[dschuyler, 2016-10-17 21:05:10]

unwrapped lines

[Dan Beam, 2016-10-17 21:11:17]

lgtm

[dschuyler, 2016-10-17 21:15:38]

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

[commit-bot: I haz the power, 2016-10-17 21:16:23]

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

[dschuyler, 2016-10-17 21:16:36]

A small change to blend with the enableSiteSettings flag.

https://codereview.chromium.org/2390053002/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/settings/site_settings/site_list.html (right):

https://codereview.chromium.org/2390053002/diff/160001/chrome/browser/resourc...
chrome/browser/resources/settings/site_settings/site_list.html:91: </template>
Now that the site details are behind a flag, these arrows
need to be updated to use the same flag.

[Dan Beam, 2016-10-17 21:17:44]

lgtm

[commit-bot: I haz the power, 2016-10-18 00:33:03]

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

[commit-bot: I haz the power, 2016-10-18 00:33:04]

Dry run: This issue passed the CQ dry run.

[dschuyler, 2016-10-18 00:38:40]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2016-10-18 00:39:32]

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-10-18 00:44:33]

Description was changed from

==========
[MD settings] use arrow-forward to switch direction in rtl

This CL flips the arrow-forward to left or right based on the rtl/ltr
direction setting and corrects the placement of the paper ripple under
the icon on rtl.

Minor changes: changes the old style name from .arrow-right to
.arrow-forward (since left or right changes in rtl/ltr); there are also
some css variables added to replace some constants.

BUG=651716,586579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD settings] use arrow-forward to switch direction in rtl

This CL flips the arrow-forward to left or right based on the rtl/ltr
direction setting and corrects the placement of the paper ripple under
the icon on rtl.

Minor changes: changes the old style name from .arrow-right to
.arrow-forward (since left or right changes in rtl/ltr); there are also
some css variables added to replace some constants.

BUG=651716,586579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-10-18 00:44:34]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-10-18 00:48:30]

Description was changed from

==========
[MD settings] use arrow-forward to switch direction in rtl

This CL flips the arrow-forward to left or right based on the rtl/ltr
direction setting and corrects the placement of the paper ripple under
the icon on rtl.

Minor changes: changes the old style name from .arrow-right to
.arrow-forward (since left or right changes in rtl/ltr); there are also
some css variables added to replace some constants.

BUG=651716,586579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD settings] use arrow-forward to switch direction in rtl

This CL flips the arrow-forward to left or right based on the rtl/ltr
direction setting and corrects the placement of the paper ripple under
the icon on rtl.

Minor changes: changes the old style name from .arrow-right to
.arrow-forward (since left or right changes in rtl/ltr); there are also
some css variables added to replace some constants.

BUG=651716,586579
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/f76c6502e7f4a6d57fe57aed25a60ce3845dd82a
Cr-Commit-Position: refs/heads/master@{#425837}
==========

[commit-bot: I haz the power, 2016-10-18 00:48:31]

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