Closes #1397: Add custom tab toolbar color support.. #1654
Conversation
| /** | ||
| * Initializes and resets the Toolbar for a Custom Tab based on the CustomTabConfig. | ||
| */ | ||
| class CustomTabToolbarFeature( |
There was a problem hiding this comment.
nit: should be CustomTabs.
Will fill in the follow up.
There was a problem hiding this comment.
I can only do one rain dance a day to get a green build. ;)
There was a problem hiding this comment.
You know what, I'm going to do this now because I want to add it to the changelog as well. I'd rather not change class names after.
jonalmeida
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1654 +/- ##
============================================
+ Coverage 82.41% 82.46% +0.05%
- Complexity 1792 1802 +10
============================================
Files 219 220 +1
Lines 7095 7113 +18
Branches 1223 1224 +1
============================================
+ Hits 5847 5866 +19
Misses 662 662
+ Partials 586 585 -1
Continue to review full report at Codecov.
|
| class CustomTabToolbarFeature( | ||
| private val sessionManager: SessionManager, | ||
| private val toolbar: BrowserToolbar, | ||
| private val sessionId: String? = null |
There was a problem hiding this comment.
In theory a custom tab should always have a session id. But I guess it's easier to implement that way? :)
There was a problem hiding this comment.
It should! Although we tend to get the sessionId from the Bundle which makes the ID optional. So yeah, it's easier this way. :)
| // Magic weighting taken from a stackoverflow post, supposedly related to how | ||
| // humans perceive color. | ||
| return (0.299 * red + 0.587 * green + 0.114 * blue).toInt() | ||
| } |
There was a problem hiding this comment.
I wonder what a good pattern is.. I usually move those helpers outside the class as private methods inside the file (since they are somewhat unrelated to the class).
| /** | ||
| * Initializes and resets the Toolbar for a Custom Tab based on the CustomTabConfig. | ||
| */ | ||
| class CustomTabToolbarFeature( |
jonalmeida
changed the title
I ended up finishing most of the requirements for a custom tabs toolbar in mozilla-mobile/reference-browser#209 all at once. This PR only contains the minimum required for setting up the feature and toolbar background color, the rest will come through in a follow up PR when I'm done writing tests for them.