Closed Bug 1208705 Opened 8 years ago Closed 8 years ago

Theme: Set accentColor on Android 5.0+


(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Not set


(firefox44 fixed)

Firefox 44
Tracking Status
firefox44 --- fixed


(Reporter: sebastian, Assigned: shubhamkjain, Mentored)



(Whiteboard: [lang=xml][good first bug])


(3 files, 1 obsolete file)

Currently we do not set an accentColor (for theming UI controls) in our theme. That's why checkboxes and headers have this cyan color in settings (See screenshot).
@Anthony: What's the best color to use here?
Flags: needinfo?(alam)
Mentor: s.kaspari
Whiteboard: [lang=xml][good first bug]
I'm here for my first bug. Can you please help me through resolving bugs in firefox?
Assignee: nobody → shubhamkjain
Any input from Anthony so far ?
Hey Shubham!

I've been thinking about this more. 

It seems like the best color here would be our Action Orange (#E66000) which is what seems to be there.
Flags: needinfo?(alam) → needinfo?(shubhamjindal18)
Flags: needinfo?(shubhamjindal18) → needinfo?(shubhamkjain)
Attached patch color_patch.diff (obsolete) — Splinter Review
Set the accent color to action_orange. @aAnthony I actually made the same choice. In fact the screenshot named color/fennec_orange that i attached is actually action_orange. i had accidentally named it incorrectly.
Flags: needinfo?(shubhamkjain)
Attachment #8666860 - Flags: review?(s.kaspari)
Comment on attachment 8666860 [details] [diff] [review]

Review of attachment 8666860 [details] [diff] [review]:

Nice. This is looking good!

The commit message is a bit misleading. This color does not relate to menus. It's used to tint UI controls in general (Android 5+). Maybe change it to something like: "Set colorAccent in v21+ theme.". And you can add the reviewer at the end of the message. So in this case the commit message could read something like this:

"Bug 1208705 - Set colorAccent in v21+ theme. r=sebastian"

Feel free to upload an updated patch. This does not require a new review.

I'll push the patch to try[1] for you. If all tests pass then you can add the "checkin-needed" [2] keyword to this bug in order to get your changes commited and pushed.

Attachment #8666860 - Flags: review?(s.kaspari) → review+
cool, Thank you! 
I'll upload an updated patch shortly.
Attachment #8666860 - Attachment is obsolete: true
Attachment #8667202 - Flags: review+
Keywords: checkin-needed
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Depends on: 1213223
Depends on: 1216503
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.