Closed Bug 1164287 Opened 5 years ago Closed 5 years ago

Remove build icon in Settings

Categories

(Firefox for Android :: Settings and Preferences, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 43
Tracking Status
firefox43 --- fixed

People

(Reporter: antlam, Assigned: sebastian)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

At first I thought it provided a nice context after the updates from inheriting the L theme, but it looks rather redundant now and seems to take away space from the top header label (especially considering localization).

Let's remove that icon from the top.
I previously did some work on the build icon size in bug 1132751 - investigate which parts of this should change or be removed.
Anthony, is this only for Android L?
Flags: needinfo?(alam)
(In reply to Michael Comella (:mcomella) from comment #2)
> Anthony, is this only for Android L?

I'm open to carrying out this decision to users on other versions too. I know we decided to keep it around but originally, but it does seem awfully unnecessary after giving it some time. :)

NI-ing Sebastian here since he brought it up in the other thread

Sebastian, I'd say we can go ahead with this!
Flags: needinfo?(alam) → needinfo?(s.kaspari)
Attached image up_button_on_android4.png (obsolete) —
Afaik we will still need to show the icon on Android 4.x devices (and maybe Honeycomb?) because the up button will be part of the app icon (see attached screenshot). Without that we would end up with a tiny up indicator that is almost impossible to hit. At least as long as we do not need some compatibility library (e.g. ActionBarCompat) that backports all the new stuff.

Nevertheless we should be good to go on Android 5+. On 2.3 there's no ActionBar at all, so no problem there.
Flags: needinfo?(s.kaspari)
Yeah, that's true and that's fine too. I forgot about that icon, heh.
This icon needs to go.
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
Attached image material-actionbar.png
I did not only remove the icon but changed the v21+ theme of settings and the sync activity to use the Material ActionBar style. What do you think?
Attachment #8617350 - Attachment is obsolete: true
Attachment #8644982 - Flags: feedback?(alam)
Comment on attachment 8644982 [details]
material-actionbar.png

LIKE.

Does this affect the action bar for text selection on web pages? 

I think this is great, moving in the Material direction! but how will this inconsistency feel? 

Can I see a build with this Sebastian?
Flags: needinfo?(s.kaspari)
Attachment #8644982 - Flags: feedback?(alam) → feedback+
(In reply to Anthony Lam (:antlam) from comment #10)
> Does this affect the action bar for text selection on web pages? 

No, I just changed it for settings and sync status to avoid breaking all kinds of things.

> I think this is great, moving in the Material direction! but how will this
> inconsistency feel? 

So this will only affect Android 5+ devices, so for them it should be more consistent now. Do we have any other ActionBars to fix? Right now I don't know of any but there probably are some.

> Can I see a build with this Sebastian?

Sure! I'll ping you on IRC.
Flags: needinfo?(s.kaspari)
saw it. tried it. love it. SHIP IT!
Comment on attachment 8645190 [details] [diff] [review]
1164287-settings-actionbar.patch

Review of attachment 8645190 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome
Attachment #8645190 - Flags: review?(mhaigh) → review+
url:        https://hg.mozilla.org/integration/fx-team/rev/f936a8642a500702d7e61da0a4ba8a9153b741af
changeset:  f936a8642a500702d7e61da0a4ba8a9153b741af
user:       Sebastian Kaspari <s.kaspari@gmail.com>
date:       Tue Aug 11 10:39:56 2015 +0200
description:
Bug 1164287 - Theme v21: Base ActionBar.FxAccountStatusActivity and ActionBar.GeckoPreferences on Material ActionBar. r=mhaigh
https://hg.mozilla.org/mozilla-central/rev/f936a8642a50
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
You need to log in before you can comment on or make changes to this bug.