MD Settings: Fix side navigation bar background color for RTL.

MD Settings: Fix side navigation bar background color for RTL.

BUG=593989
Committed: https://crrev.com/d22fae34f47ce95aee43167fef1eb3f6cb231a0a
Cr-Commit-Position: refs/heads/master@{#382698}

[dpapad, 2016-03-22 20:56:54]

Description was changed from

==========
MD Settings: Fix navigation bar background color for RTL.

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

to

==========
MD Settings: Fix side navigation bar background color for RTL.

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

[dpapad, 2016-03-22 20:58:13]

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

[dpapad, 2016-03-22 20:58:14]

Before/after http://imgur.com/a/NXqc3.

@Dave: Do you know of an appropriate bug to attach to this CL? Did not find a
side-bar specific bug in the tracker spreadsheet.

[dschuyler, 2016-03-22 21:05:50]

On 2016/03/22 20:58:14, dpapad wrote:
> Before/after http://imgur.com/a/NXqc3.
> 
> @Dave: Do you know of an appropriate bug to attach to this CL? Did not find a
> side-bar specific bug in the tracker spreadsheet.

  Issue 593989: Side nav menu

[dpapad, 2016-03-22 21:11:07]

Description was changed from

==========
MD Settings: Fix side navigation bar background color for RTL.

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

to

==========
MD Settings: Fix side navigation bar background color for RTL.

BUG=593989
==========

[dpapad, 2016-03-22 21:11:45]

On 2016/03/22 at 21:05:50, dschuyler wrote:
> On 2016/03/22 20:58:14, dpapad wrote:
> > Before/after http://imgur.com/a/NXqc3.
> > 
> > @Dave: Do you know of an appropriate bug to attach to this CL? Did not find
a
> > side-bar specific bug in the tracker spreadsheet.
> 
>   Issue 593989: Side nav menu

Attached, thanks. FYI, I am not working on side-nav. I was just testing some RTL
and happened to come across this easy and small fix.

[dschuyler, 2016-03-22 21:13:10]

lgtm

[dpapad, 2016-03-22 21:15:36]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-03-22 21:15:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1828493002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1828493002/1

[commit-bot: I haz the power, 2016-03-22 22:02:59]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-22 22:05:24]

Description was changed from

==========
MD Settings: Fix side navigation bar background color for RTL.

BUG=593989
==========

to

==========
MD Settings: Fix side navigation bar background color for RTL.

BUG=593989

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

[commit-bot: I haz the power, 2016-03-22 22:05:25]

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

[michaelpg, 2016-03-23 00:18:32]

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

https://codereview.chromium.org/1828493002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_ui/settings_ui.html:50:
background-color: var(--settings-background-color);
it would be nice if we didn't have to duplicate this

I guess we can't comma-separate the mixins like we can with toplevel CSS rules,
but wdyt about creating a separate common mixin to @apply in each of these
mixins?

or would that be terrible in itself?

[dpapad, 2016-03-23 00:40:18]

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

https://codereview.chromium.org/1828493002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_ui/settings_ui.html:50:
background-color: var(--settings-background-color);
On 2016/03/23 at 00:18:32, michaelpg wrote:
> it would be nice if we didn't have to duplicate this
> 
> I guess we can't comma-separate the mixins like we can with toplevel CSS
rules, but wdyt about creating a separate common mixin to @apply in each of
these mixins?
> 
> or would that be terrible in itself?

I am not very familiar with CSS mixins. Can you use @apply within a mixin? Also,
if I understand your suggestion it would look something as follows,

--side-nav-background: {
  background-color: var(--settings-background-color);
}
--paper-drawer-panel-left-drawer-container: {
  @apply(--side-nav-background);
}
--paper-drawer-panel-right-drawer-container: {
  @apply(--side-nav-background);
}

So now instead of repeating the CSS variable twice, we have an extra level of
indirection, so we repeat the new mixin twice. Is that any better (or am I
misunderstanding the suggestion)?