1214942 - Toolbar ActionBar icon colors are the wrong color (white) on 5+.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I remember using controlColor (or similar) to change the color of the controls in the preferences panel to white – perhaps this is related?

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Screenshot

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: floating-toolbar.png

Aren't white icons correct after we switched to a .DarkActionBar theme? But the ActionBar should be dark? I'm a bit loosing track here. :)

On the positive side: On Android 6 we now have a floating toolbar (See screenshot).

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Sebastian Kaspari (:sebastian) from comment #2)
> Aren't white icons correct after we switched to a .DarkActionBar theme? But
> the ActionBar should be dark? I'm a bit loosing track here. :)

Perhaps but we need to override the icon color with toolbar icon grey.

I wonder why we choose dark action bar for the main activity, rather than light. I don't think we use the system action bar in any other places (and as such, it probably doesn't matter if we use dark or light).

> On the positive side: On Android 6 we now have a floating toolbar (See
> screenshot).

AppCompat is win! :D

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

The commit I made to change the action bar style is here:
  http://hg.mozilla.org/mozilla-central/rev/de6a56d4bd23

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Removing the "android:" prefix from
  <item name="android:actionBarTheme">@style/ActionBarTheme</item>

Changes the colors of the icons but changing the attached theme appears to do nothing – for some reason, this attribute needs the prefix.

I managed to change the color of the icons (colorControlNormal) with exception to the back button (which is still white), the background (actionModeBackground), the line at the bottom of the screen (colorControlActivated), and the icon pressed color (colorControlHighlight), but I'm pretty much flailing trying to change the color of the back button and the text.

Sebastian, do you know what's going on here? I'll post my WIP.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1214942 - Change action bar colors on 5+. r=sebastian

Bug 1214942 - (WIP) Change action bar colors on 5+.

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I managed to change the text color – was it `android:textColor`? `textColor`? `colorPrimary`? `android:colorPrimary`? `actionMenuTextColor`? How about in a `titleTextStyle`?

No! It was `android:textColorPrimary`. -_- Still working on that back button color though.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

For the back button, http://stackoverflow.com/a/27517878 says colorControlNormal but that doesn't seem to work.

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8676484 [details]
MozReview Request: Bug 1214942 - Change action bar colors on 5+. r=sebastian

Bug 1214942 - (WIP) Change action bar colors on 5+.

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Michael Comella (:mcomella) from comment #3)
> I wonder why we choose dark action bar for the main activity, rather than
> light. I don't think we use the system action bar in any other places (and
> as such, it probably doesn't matter if we use dark or light).

I did that! Because we actually use a dark ActionBar in the Settings and Sync Activities (Bug 1164287).


(In reply to Michael Comella (:mcomella) from comment #5)
> Sebastian, do you know what's going on here? I'll post my WIP.

Are you trying to go back to the light version? I think I tried this as well in bug 1195287. After several problems (I don't remember if it was the up button) I settled with a patch for a dark version that matches the toolbar we use in Settings/Sync. Resulting in something like this (Imagine white icons, this is a WIP screenshot): https://bug1195287.bmoattachments.org/attachment.cgi?id=8654076

If I remember correctly the Material theme always used/assumed a dark Contextual Action Bar no matter if a .DarkActionBar theme was used or not.

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Main activity, no action bar theme

I moved the action bar style/theme to the gecko preferences and removed it from the main activity theme to get the attached screenshot.

However, whenever I add an actionBarTheme to the main activity theme, the back arrow turns white and I can't figure out which setting changes that (perhaps it's related to bug 1220309?). I assume we're overwriting the system style which sets the button to black but the default is white.

If I can't figure it out soon, it's probably not worth the effort (this looks alright as is!) and we can fix it when we unify the action bar style entirely.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Michael Comella (:mcomella) from comment #11)
> However, whenever I add an actionBarTheme to the main activity theme, the
> back arrow turns white and I can't figure out which setting changes that
> (perhaps it's related to bug 1220309?).

It's related to bug 1220309 – I extended AppCompatActivity in GeckoApp and removed the `android:` attributes from `actionBar*` and the back arrow color is changed to the color of colorControlNormal (the attribute I expected it to be).

Guess we're rolling with this.

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8676484 [details]
MozReview Request: Bug 1214942 - Change action bar colors on 5+. r=sebastian

Bug 1214942 - Change action bar colors on 5+. r=sebastian

The Preferences Activity looks as desired.

We can't get the main activity to have the desired looks because bug 1220309
prevents us from changing the Action Bar back button color from white so we
went with the default style which looks *alright*.

Comment 14

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8676484 [details]
MozReview Request: Bug 1214942 - Change action bar colors on 5+. r=sebastian

https://reviewboard.mozilla.org/r/22729/#review21915

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/d750bdaa6ffa78e97b3d08e9fcd5602bb5415a43
Bug 1214942 - Change action bar colors on 5+. r=sebastian

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d750bdaa6ffa