Closed Bug 1098544 Opened 10 years ago Closed 4 years ago

Add meta theme-color support to toolbar

Categories

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

Product:
Firefox for Android Graveyard
Component:
Theme and Visual Design
Platform:
All
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED INCOMPLETE

People

(Reporter: mcomella, Unassigned, Mentored)

References

Details

(Keywords: dev-doc-needed)

Attachments

(3 files, 4 obsolete files)

An example of theme installed from the AMO
(deleted), image/png
Details
theme-color.patch
(deleted), patch
Details | Diff | Splinter Review
Screenshot_20160815-143714.png
(deleted), image/png
Details 
Chrome has added support for theme colors in the toolbar [1], and it seems we suppport it on other platforms (e.g. bug 1013913 and bug 1043275). See also [2]. Let's do it on Android!

Not strictly lwt, but maybe we can build it into the system.

[1]: http://updates.html5rocks.com/2014/11/Support-for-theme-color-in-Chrome-39-for-Android
[2]: https://github.com/whatwg/meta-theme-color
Component: General → Theme and Visual Design
Attached patch theme-color.patch (obsolete) (deleted) — — Splinter Review 
This patch adds the theme-color to Tab.java.
I just saw a recent article about how Chrome does this:
http://www.androidpolice.com/2015/09/23/latest-chrome-dev-for-android-has-partial-support-for-theme-color-attribute-without-merging-tabs-and-apps/

I think this could be a nice small fix, and it could be one part of the "progressive apps" story.
Remember how we let LWT show through in our new Tablet's UI, Mike? I.e. we set the image over the entire Toolbar and then give the "tab" grey area transparency to show the image underneath? 

We could probably do the same thing here! As opposed to what we currently do for LWT.
Flags: needinfo?(michael.l.comella)
Instead of using bitmaps in our LWT, we could probably just use a ColorDrawable instead.

(In reply to Anthony Lam (:antlam) from comment #3)
> Remember how we let LWT show through in our new Tablet's UI, Mike? I.e. we
> set the image over the entire Toolbar and then give the "tab" grey area
> transparency to show the image underneath? 

But we'll need to work around ^, where we use a grey transparency, I think – I can't remember the details.

At the very least, we can start by making this mentorable. :)
Blocks: fennec-polish
Mentor: michael.l.comella, s.kaspari
Flags: needinfo?(michael.l.comella)
Blocks: progressive-apps
No longer blocks: customtabs
Sounds like we'll be using/building-on our LWT support to enable this feature. That sounds like the right approach. We don't need two ways of doing it.
Noticed that Chrome has started to do this, e.g. browse to facebook.com on your Chrome mobile browser. Nice UI "app-like" experience.
Ill take a look at this now
Assignee: nobody → dale
Dale, regarding your item in the front-end meeting, afaik, I have the most (rusty) context on how the LWT theme code works – let me know if you have questions! If you let me know if advance, I can refresh myself on the code and try to give you a rundown.
I recently applied a lightweight theme for fun (and for testing), and I noticed that we do apply the theme to the tabs tray. So if we only want to color the toolbar for the selected tab, we can't just re-purpose all our lightweight theme code as it stands. Something to look out for.
(In reply to :Margaret Leibovic from comment #10)
> I recently applied a lightweight theme for fun (and for testing), and I
> noticed that we do apply the theme to the tabs tray. So if we only want to
> color the toolbar for the selected tab, we can't just re-purpose all our
> lightweight theme code as it stands. Something to look out for.

Given the illusion we create of the tab moving off of the tab bar, I don't think it's straight-forward to make this animation look clean without the tabs tray being colored like the toolbar.

That being said, I don't feel there is a restriction that would prevent "theme-color" from affecting our tabs tray too! Do you have specific concerns about it?
(In reply to Michael Comella (:mcomella) from comment #11)
> (In reply to :Margaret Leibovic from comment #10)
> > I recently applied a lightweight theme for fun (and for testing), and I
> > noticed that we do apply the theme to the tabs tray. So if we only want to
> > color the toolbar for the selected tab, we can't just re-purpose all our
> > lightweight theme code as it stands. Something to look out for.
> 
> Given the illusion we create of the tab moving off of the tab bar, I don't
> think it's straight-forward to make this animation look clean without the
> tabs tray being colored like the toolbar.
> 
> That being said, I don't feel there is a restriction that would prevent
> "theme-color" from affecting our tabs tray too! Do you have specific
> concerns about it?

No specific concerns, just that it's extra complexity that will need to be accounted for.
I think that the meta "theme-color" support is nice, but I think that we may only need to customize the Tab and More buttons' background color and opacity, especially when a custom theme is applied from Mozilla Addons.
Deassigning since ill no longer be working on it, cheers
Assignee: dale → nobody
> That being said, I don't feel there is a restriction that would prevent "theme-color" 
> from affecting our tabs tray too! Do you have specific concerns about it?

The theme-color is something that is defined to be specific to the site (or more particularly to the current page). I think styling the entire browser (tab center) etc can lead to complexity and confusion, especially when the chrome colour is being used to give information to the user (private browsing mode). In both chrome and Firefox OS we only use the theme-color for components visible in the 'page viewing' mode specific to that page.

The android layout makes that a little more tricky, however the animation when viewing tabs pushes the url bar off the screen and has its own distinct area so I dont think there will be a problem with the url bar being for example green, and the tab tray maintaining its grey.
Going to retake a look at this
Assignee: nobody → dale
this addon already does this pretty well on desktop. Maybe it might help  
https://addons.mozilla.org/en-US/firefox/addon/vivaldifox/
Attached image Screenshot_20160701-173631.png (obsolete) (deleted) — 

    
    

    
  
So retaking a look at this, got a patch which handles the theme color however going to need a little direction around what the design should look like (and how to be implement that design) it doesnt seem like LWT will work straight off the bat

Who would be able to give design direction for this?

Cheers
Flags: needinfo?(bbermes)

    
    

    
  
I have seen the attached picture, but this patch will only work on Lollipop and above. Meanwhile, I suggest some parts which is fine to be colored.
1. The Tabs and More buttons can have a background color which depends on Theme color meta tag. If a custom theme is install, the buttons will change to a semi-transparent one. On tablet, the tabs may be colored as well.
2. The loading/progress bar can be colored as well, and leave the search/address bar colored white/black, so there will still a distinction between normal and private tabs.
3. Alternative of option #2. On the Tablet and mobile version, the search/address bar may be colored too, as long if there is no specific themes installed.
4. On Mobile, under the Tabs page, the selection color may be changed by the same meta tag.
5. When a user selects a text, the color of the selection "handles" and the selection options (such as Select all, Copy and Paste) can be colored with the meta tag.

    
    

    
  
(In reply to Dale Harvey (:daleharvey) from comment #19)
> So retaking a look at this, got a patch which handles the theme color
> however going to need a little direction around what the design should look
> like (and how to be implement that design) it doesnt seem like LWT will work
> straight off the bat
> 
> Who would be able to give design direction for this?
> 
> Cheers

I think we need to keep this simple, Anthony would you mind giving your blessing.

I don't think we should add theme/color choices to the more buttom, rather the actual tab and the URL background color, nothing else.
Flags: needinfo?(bbermes) → needinfo?(alam)

    
    

    
  
Im also for coloring the actual tab. 
It'd be nice if the url can be left as usual(black text on white bg) or white text on black bg

Best to leave grey more-tabs/3 dots as it is and let it blend with the notification bar

    
    

    
  
(In reply to bull500 from comment #22)
> Im also for coloring the actual tab. 
> It'd be nice if the url can be left as usual(black text on white bg) or
> white text on black bg
> 
> Best to leave grey more-tabs/3 dots as it is and let it blend with the
> notification bar
Ok. Let's make the color theming does not clash with themes installer from AMO.
Custom themes consists of an image. This image
This image is applied as the background for tab header (addressbar+buttons) and
on tab switcher page in mobile.
This image is also used as the background for Tabs in tablet version, similar to the Desktop version.
There should be some measures to avoid the theme color meta tag affecting the theme/image by overriding the position of the image (I.e. address bar and some Tabs.)

    
    

    
  
If the grey buttons are not colored, the meta tag may override the custom theme. (I don't know how to add attachments to this comment.)

    
    

    
  


  
The parts of the UI which can be changed by a theme should not be overwritten by theme-color meta theme.
Therefore I suggest the coloring is applied on buttons since they are semi-transparent when a theme is installed.
For Android Lollipop and above, the status bar color can also be changed.
For Android KitKat and some manufacturer-specific Android UIs, a custom patch/gradient status bar can be used to support the coloring as well.

    
    

    
  
(In reply to Barbara Bermes [:barbara] from comment #21)
> (In reply to Dale Harvey (:daleharvey) from comment #19)
> > So retaking a look at this, got a patch which handles the theme color
> > however going to need a little direction around what the design should look
> > like (and how to be implement that design) it doesnt seem like LWT will work
> > straight off the bat
> > 
> > Who would be able to give design direction for this?
> > 
> > Cheers
> 
> I think we need to keep this simple, Anthony would you mind giving your
> blessing.
> 
> I don't think we should add theme/color choices to the more buttom, rather
> the actual tab and the URL background color, nothing else.

Sure thing.

Looking at the screenshot in comment 18, it's a pretty good start. But could we also try applying the theme color to the tab itself? That is, where our UI is currently our Toolbar Grey, use the theme color instead. This, plus what you have with the status bar seems like it could be a good start to me.

Could I see a screenshot of this?
Flags: needinfo?(alam) → needinfo?(dale)

    
    

    
  

      
      
      
      

        Attached image
          
        Screenshot_20160711-171421.png (obsolete) (deleted)
        — 
      

    


  
I think its the background of the toolbar that needs to be coloured here, colouring the url bar gives us an awkward mark on the top left, or we could remove the curvature on the left?

Ill see how it looks when the background is coloured and not the url bar, but also need to consider how this will work with themes (which I didnt realise existed until mentioned above)

        Attachment #8767244 -
        Attachment is obsolete: true
Flags: needinfo?(dale) → needinfo?(alam)

    
    

    
  
Try color only the URL bar and leave the background to grey if its possible as well!

    
    

    
  

      
      
      
      

        Attached image
          
        Theme-color applied to background (obsolete) (deleted)
        — 
      

    


  
And here is the theme applied to the background

    
    

    
  
I like the screenshot in comment 29, it allows other themed tabs to still be applied.

    
    

    
  
(In reply to Dale Harvey (:daleharvey) from comment #29)
> Created attachment 8769749 [details]
> Theme-color applied to background
> 
> And here is the theme applied to the background

I thought about this too. But we need a reliable way to change the color of the two icons (overflow and tabs tray).

For instance, if the theme color is a really light grey, our icons will be difficult to see against it.

Dale, is there a dynamic way to say:

 - If, background color is light, use this color for the tabstray and overflow menu icon. 
 - If background color is dark, use this other color for the tabstray and overflow menu icon.

We're also proposing a new Toolbar design that will make supporting this UI change a lot easier so maybe we could hold off and do it together instead of trying to force it into this design? I'll chat with Barbara about this.
Flags: needinfo?(alam) → needinfo?(dale)

    
    

    
  
Okay. How about coloring in the tablet UI (which looks similar with the desktop UI)?

    
    

    
  
> Dale, is there a dynamic way to say:

> - If, background color is light, use this color for the tabstray and overflow menu icon. 
> - If background color is dark, use this other color for the tabstray and overflow menu icon.

Yup that is necessary and possible. 

> We're also proposing a new Toolbar design that will make supporting this UI 
> change a lot easier so maybe we could hold off and do it together instead of trying to 
> force it into this design?

Is there a bug open / some previews of that? something designed with the changeable theme-color in mind would likely be easier and I could take on some of the dev as it blocks my work.
Flags: needinfo?(dale)

    
    

    
  
So I assumed this to be much easier than it is, now the color from that background comes from http://searchfox.org/mozilla-central/source/mobile/android/base/resources/drawable/shaped_button.xml#17, if I change the referenced color in colors.xml and rebuild the background changes fine. However I am not overly familiar with how that color is actually used, and how to change it dynamically. 

The BrowserToolbar has 2 backgrounds shaped_button, I can find those with findViewById but I cannot find any way to set that color that it uses.

Right now I have 2 approaches, when we switch pages I can find anything that is supposed to change color and set it, or anything that is based on theme color could request what color it should be using and then we could do a redraw (probably the same as lightweightthemechanged event), it looks like the lwt already have the concept of dark / light button switches so if someone could point me to how to dynamically change the color of that shaped_button then I can hopefully put the rest together.

Any guidance on the above would be super helpful, thanks
Flags: needinfo?(s.kaspari)
Flags: needinfo?(michael.l.comella)

    
    

    
  
Its also worth asking are lwt something that is being considered / supported in the new toolbar design?
Flags: needinfo?(alam)

    
    

    
  
After loading shaped_button.xml it should be turned into a StateListDrawable[1]. Just looking at the documentation I do not see an easy way to just change a color.

However you could create a new one in code, with the color you want and then set it as background. Something like this:

> StateListDrawable background = new StateListDrawable();
> background.addState(new int[] { android.R.attr.state_pressed }, new ColorDrawable(0xFF0000FF));
> background.addState(new int[] { android.R.attr.state_focused }, new ColorDrawable(0xFF00FF00));
> background.addState(new int[] {}, new ColorDrawable(0xFFFF0000));

Does this help?

[1] https://developer.android.com/reference/android/graphics/drawable/StateListDrawable.html
Flags: needinfo?(s.kaspari)

    
    

    
  
There are two approaches I see:

1) Override the colors dynamically, as Sebastian mentions in comment 37.
 * You could restore the original StateListDrawable by caching the value returned by getBackground before you set the new background.
 + This is pretty simple to write the code for
 - Unless you need specify different behavior for different device types (e.g. `if (isSmallTablet...`)
 - It can be hard to track down where to update the colors (as opposed to being defined in XML) if you need to update them later

2) Declare the desired colors/states in XML with the generic glue code (which is how LWT works in the first place).
 + Easy to know where to update colors
 + Easy to define new colors for different configurations (by using the android resource system)
 - A lot of annoying boilerplate code to write

There are a few ways I see to do this off the top of my head:
 a) Re-use the existing LWT attributes like `state_dark` & `state_light` (e.g. [1] – I believe Android uses the first configuration that matches).
  * I believe you'd override onCreateDrawableState in a class that extends ShapedButton/whatever-class-the-menu-is to call `mergeDrawableStates(drawableState, STATE_LIGHT/DARK)` when you need the appropriate state
  + Less boilerplate than adding new attributes
  + Conceptually, based on name, `state_light` & `state_dark` make sense to use here
  - This likely creates a tight coupling between the base class & ShapedButton where if Themed* is changed to modify how state_light/dark works, the new code could break.

 b) Create new attributes a technique similar to the one used in Themed*, where it defines `state_/light/dark` (e.g. [2])
  * Ultimately, I believe you'd override onCreateDrawableState in ShapedButton and call `mergeDrawableStates(drawableState, YOUR_NEW_STATE` for the appropriate state
   + Decouples base class & ShapedButton
   + Decouples state_dark/light from these new attributes, which may not be mutually exclusive in the future
   - Annoying boilerplate (I recommend looking at the initial bug that implemented the LWT attrs [3])

I favor 2b for correctness and ease of updating the color values, particularly for different configurations, at a later point.

NI me if you have more questions on this!

[1]: https://dxr.mozilla.org/mozilla-central/rev/ffac2798999c5b84f1b4605a1280994bb665a406/mobile/android/base/resources/color-large-v11/tab_strip_item_bg.xml#28
[2]: https://dxr.mozilla.org/mozilla-central/rev/ffac2798999c5b84f1b4605a1280994bb665a406/mobile/android/base/resources/values/attrs.xml#109
[3]: https://hg.mozilla.org/mozilla-central/rev/65b79c7b4105
Flags: needinfo?(michael.l.comella)

    
    

    
  
(In reply to Dale Harvey (:daleharvey) from comment #35)
> Its also worth asking are lwt something that is being considered / supported
> in the new toolbar design?

Yes! definitely. It will all be considered together, from the get go, in the new design.

In the mean time, I think we should hold off on this for now.
Flags: needinfo?(alam)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        theme-color.patch (deleted)
        — 
          — Splinter Review
      

    


  

        Attachment #8655342 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached image
          
        Screenshot_20160815-143714.png (deleted)
        — 
      

    


  

        Attachment #8769739 -
        Attachment is obsolete: true

        Attachment #8769749 -
        Attachment is obsolete: true

    
    

    
  
So I have uploaded what I have so far, few technical issues (how to set the background on that corner on the top left). However the main issue is with themes, I made some experiments with trying to overlay the theme-colors semi transparently on top of the theme image and the results did not look great. I have a hard time seeing how ltw combined with theme-color is going to interact well and most definitely need a design spec to work towards.

Anthony has mentioned work on a new toolbar design that takes the theme color into consideration, so agree with him that we should deal with them at the same time (Anthony is there a bug # for the new toolbar design?).

When we can launch standalone web apps I can revive the statusbar only part of this patch since there will be not conflict with the themes, but best doing that at the same time.

    
    

    
  
Great! Is how can I apply this patch? Is it available in Nightly?

    
    

    
  
If you are building Firefox for Android yourself (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build) you can apply the patch, however its not currently in nightly and I doubt we will want to do so until we have the conflict with themes sorted out.

One possible solution is to just not do theme colors if lwt is enabled, do you think that is suitable Anthony or best to wait for the new toolbar?

    
    

    
  
(In reply to Dale Harvey (:daleharvey) from comment #42)
> Anthony has mentioned work on a new toolbar design that takes the theme
> color into consideration, so agree with him that we should deal with them at
> the same time (Anthony is there a bug # for the new toolbar design?).

I'm currently working on the design but there's no bug yet. I'll keep you posted Dale! Thanks!
Flags: needinfo?(alam)

    
    

    
  
Yo, just wondering since its been a about a month, is there anything more here / anything the Taipei team could help out with? Cheers
Flags: needinfo?(vchen)
Flags: needinfo?(alam)

    
    

    
  
Hey Dale! 

Designs are underway, but nothing to start development on yet unfortunately. I'll keep you guys posted!
Flags: needinfo?(alam)

    
    

    
  
Anthony, I was wondering how this was coming along, is there anything to track yet? Cheers
Flags: needinfo?(alam)

    
    

    
  
We talked about this in Kona and we're still working on it
Flags: needinfo?(alam)

    
    

    
  
Deassigning myself while blocked on design
Assignee: dale → nobody

    
    

    
  
Clearing needinfo
Flags: needinfo?(vchen)

    
    

    
  
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE

    
  
Product: Firefox for Android → Firefox for Android Graveyard

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: