Closed
Bug 1071226
Opened 10 years ago
Closed 10 years ago
Refine private browsing new tablet browser toolbar
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 35
People
(Reporter: mcomella, Assigned: mcomella)
References
Details
Attachments
(4 files, 5 obsolete files)
Primarily, create and use a private browsing menu icon, then make sure everything else looks okay. NI :antlam for the icon.
Flags: needinfo?(alam)
Assignee | ||
Comment 2•10 years ago
|
||
After speaking with Anthony, we decided to use the same menu asset for private browsing and not.
Attachment #8495605 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
Anthony, what do you think? I think the tabs panel font color may also need to be adjusted.
Attachment #8495607 -
Flags: feedback?(alam)
Assignee | ||
Comment 4•10 years ago
|
||
This is outside the scope of the toolbar but we had misunderstanding and I have the assets, so I updated these buttons. Note that these assets *will* land on old tablet.
Attachment #8495626 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 5•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8495607 -
Attachment description: Same menu icon in pb → Part 1: Same menu icon in pb
Comment 7•10 years ago
|
||
Comment on attachment 8495626 [details] [diff] [review] Part 2: Update tabs panel selector icons Review of attachment 8495626 [details] [diff] [review]: ----------------------------------------------------------------- It's more like adding new tabs panel icons, no? What happened to the old icons? Maybe this needs to be prefixed with new_tablet?
Comment 8•10 years ago
|
||
Comment on attachment 8495605 [details] [diff] [review] Part 1: Use same menu asset for private browsing mode Review of attachment 8495605 [details] [diff] [review]: ----------------------------------------------------------------- Less resources = win.
Attachment #8495605 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #7) > It's more like adding new tabs panel icons, no? Yes, these are tabs panel icons - see comment 4 for the motivation. > What happened to the old icons? Maybe this needs to be prefixed with new_tablet? We just used the phone icons. I don't think we should prefix with new_tablet because then we need to have a separate branch in the TabsPanel.initialize method for minimal gains though we could if you really don't want these showing up on old tablet.
Comment 10•10 years ago
|
||
Comment on attachment 8495607 [details]
Part 1: Same menu icon in pb
+ for the 3 dot not being an awkward gradient purple.
But yes, now the tabs icon needs to change colors to match
Attachment #8495607 -
Flags: feedback?(alam) → feedback+
Comment 11•10 years ago
|
||
Splitted bug 1073614 off from this. Talked on IRC, let's reuse the hat tab and the mask icon from the current tree. I'm going to file a follow up to update those for mobile and tablet.
Assignee | ||
Updated•10 years ago
|
Attachment #8495626 -
Attachment is obsolete: true
Attachment #8495626 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8495627 -
Attachment is obsolete: true
Assignee | ||
Comment 12•10 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #10) > But yes, now the tabs icon needs to change colors to match I think that's because the assets from bug 1072466 hadn't landed yet - this screenshot includes them.
Attachment #8495607 -
Attachment is obsolete: true
Attachment #8496143 -
Flags: feedback?(alam)
Comment 13•10 years ago
|
||
Comment on attachment 8496143 [details]
Part 1: Private browsing
Nice!
You brought up a good point, I wonder if the '1' should be in a diff color. What colour is it right now? (We don't want to hide it too much)
Attachment #8496143 -
Flags: feedback?(alam) → feedback+
Flags: needinfo?(michael.l.comella)
Assignee | ||
Comment 14•10 years ago
|
||
:antlam gave me these colors on irc: * #777777 for selected and background tabs * #f5f5f5 for private selected tabs and tabs panel button.
Attachment #8496288 -
Flags: review?(lucasr.at.mozilla)
Comment 15•10 years ago
|
||
Comment on attachment 8496288 [details] [diff] [review] Part 2: Update text color in private browsing and tab strip Review of attachment 8496288 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/resources/color-large-v11/new_tablet_tab_strip_item_title.xml @@ +3,5 @@ > xmlns:gecko="http://schemas.android.com/apk/res-auto"> > > <item android:state_checked="true" > gecko:state_private="true" > + android:color="@color/new_tablet_private_browsing_chrome_text"/> Where's new_tablet_private_browsing_chrome_text defined? Does it actually need to be a new color resource?
Attachment #8496288 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #15) > Where's new_tablet_private_browsing_chrome_text defined? Does it actually > need to be a new color resource? Forgot to `hg add` (again).
Attachment #8496900 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8496288 -
Attachment is obsolete: true
Comment 17•10 years ago
|
||
Comment on attachment 8496900 [details] [diff] [review] Part 2: Update text color in private browsing and tab strip Review of attachment 8496900 [details] [diff] [review]: ----------------------------------------------------------------- This looks ok but creating a new color just for private mode on tablets looks a bit smelly. I'd expect us to be able to use one of the text_color_*_inverse colors here... ::: mobile/android/base/resources/values-large-v11/colors.xml @@ +3,5 @@ > + - License, v. 2.0. If a copy of the MPL was not distributed with this > + - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > + > +<resources> > + <color name="new_tablet_private_browsing_chrome_text">#F5F5F5</color> Can't we just use one of the existing text_color_* colors here? Just pick one that is close enough to F5F5F5, or maybe just change one of the existing text colors?
Attachment #8496900 -
Flags: review?(lucasr.at.mozilla) → feedback+
Assignee | ||
Comment 18•10 years ago
|
||
Done: text_color_primary_inverse is unused elsewhere and I doubt we ever want to use #FFF anyway.
Attachment #8496908 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8496900 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(michael.l.comella)
Comment 19•10 years ago
|
||
Comment on attachment 8496908 [details] [diff] [review] Part 2: Update text color in private browsing and tab strip Review of attachment 8496908 [details] [diff] [review]: ----------------------------------------------------------------- Great, thanks.
Attachment #8496908 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 20•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/fa408884a4e0 https://hg.mozilla.org/integration/fx-team/rev/06a3ad8c7d2c
Assignee | ||
Comment 21•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/c6a2ba054ff8 https://hg.mozilla.org/integration/fx-team/rev/85ba68c632ef
Comment 23•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c6a2ba054ff8 https://hg.mozilla.org/mozilla-central/rev/85ba68c632ef
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
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
•