Closed
Bug 1317933
Opened 8 years ago
Closed 8 years ago
Custom Tab: Switch theme based on background color?
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox53 fixed)
RESOLVED
FIXED
Firefox 53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: sebastian, Assigned: mail)
References
Details
Attachments
(3 files, 1 obsolete file)
In bug 1316869 we added code to adapt the text color based on the background color defined by the third-party app. However the back/up icon is still white on light backgrounds. We might need to switch the whole theme of the activity (light/dark).
This seems to work:
private void setThemeFromToolbarColor() {
if (toolbarColor == NO_COLOR) {
return;
}
if (ColorUtil.getReadableTextColor(toolbarColor) == Color.BLACK) {
setTheme(android.R.style.Theme_Material_Light_NoActionBar);
} else {
setTheme(android.R.style.Theme_Material_NoActionBar);
}
}
Reporter | ||
Comment 2•8 years ago
|
||
Sounds good for now. Do you want to upload a patch for that?
Reporter | ||
Comment 6•8 years ago
|
||
Comment on attachment 8820649 [details] [diff] [review]
1317933-friedger.patch
Review of attachment 8820649 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
@@ +94,5 @@
> +
> + if (ColorUtil.getReadableTextColor(toolbarColor) == Color.BLACK) {
> + setTheme(android.R.style.Theme_Material_Light_NoActionBar);
> + } else {
> + setTheme(android.R.style.Theme_Material_NoActionBar);
I guess we would need an appcompat theme here to support 4+?
Attachment #8820649 -
Flags: feedback+
Now with appcompat theme
Attachment #8820649 -
Attachment is obsolete: true
Attachment #8826955 -
Flags: review?(s.kaspari)
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → mail
Status: NEW → ASSIGNED
Reporter | ||
Comment 8•8 years ago
|
||
Comment on attachment 8826955 [details] [diff] [review]
1317933-friedger_2.patch
Review of attachment 8826955 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM. Thank you, Friedger!
::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java
@@ +94,5 @@
> + if (toolbarColor == NO_COLOR) {
> + return;
> + }
> +
> + if (ColorUtil.getReadableTextColor(toolbarColor) == Color.BLACK) {
Technically this utility method was only used here, so it could return something different than a color now (a boolean? a theme?). However we can file a follow-up bug for that if anyone wants to make this nicer.
Attachment #8826955 -
Flags: review?(s.kaspari) → review+
Reporter | ||
Comment 9•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/721b8d0c2401e20cb48a82e7eb7ccc8e96e98e35
Bug 1317933 - CustomTabsActivity: Set theme based on toolbar color. r=sebastian
Comment 10•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•