Closed
Bug 1050404
Opened 10 years ago
Closed 10 years ago
[Collections] Update to use gaia-header
Categories
(Firefox OS Graveyard :: Gaia::Everything.me, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: wilsonpage, Assigned: wilsonpage)
References
Details
Attachments
(3 files)
No description provided.
Assignee | ||
Updated•10 years ago
|
Blocks: gaia-header
Assignee | ||
Comment 2•10 years ago
|
||
I'm using the 'dark/media' theme until I get confirmation back from visual on what theme should be used for collections app. NI hung for more info here.
Flags: needinfo?(hnguyen)
Comment 3•10 years ago
|
||
Comment on attachment 8469454 [details]
pull-request (master)
I haven't tested this or anything yet, but seems like it should be fine. Thanks!
Attachment #8469454 -
Flags: feedback?(kgrandon) → feedback+
Comment 4•10 years ago
|
||
Yes continue using the "media" theme for the dark applications. Also after speaking to Wilson, I think we should rename the "media theme" to "dark theme" to minimize confusion for any apps that want to use it.
Flags: needinfo?(hnguyen)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8469454 [details] pull-request (master) I have the commit stacked on blocker bug 1050801 purely for review purposes. Will rebase once that lands.
Attachment #8469454 -
Flags: review?(kgrandon)
Comment 6•10 years ago
|
||
Comment on attachment 8469454 [details]
pull-request (master)
I can't R+ this because it appears to break the wallpaper that is supposed to be seen behind the header. If this is a UX change, that's fine, but we have a bunch of wallpaper code in the app that we should remove.
Attachment #8469454 -
Flags: review?(kgrandon)
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #6) > Comment on attachment 8469454 [details] > pull-request (master) > > I can't R+ this because it appears to break the wallpaper that is supposed > to be seen behind the header. If this is a UX change, that's fine, but we > have a bunch of wallpaper code in the app that we should remove. kgrandon: see comment 4 where Hung states that we should use the 'dark'/'media' theme. This results in a solid grey header background.
Flags: needinfo?(hnguyen)
Comment 8•10 years ago
|
||
Sorry guys but can I get a screenshot of what's happening here? Thanks!
Flags: needinfo?(hnguyen)
Assignee | ||
Comment 9•10 years ago
|
||
Screenshot after with new header
Attachment #8470864 -
Flags: feedback?(hnguyen)
Comment 10•10 years ago
|
||
Comment on attachment 8470864 [details]
1015247.png
Looks good to me.
Attachment #8470864 -
Flags: feedback?(hnguyen) → feedback+
Assignee | ||
Updated•10 years ago
|
Attachment #8469454 -
Flags: review?(kgrandon)
Comment 11•10 years ago
|
||
The typical smart collections view is with a background image. Let's make sure we ui-review the proper image. The code does look good to me though, just want to make sure we are doing the right thing UX wise. Flagging both Hung and Patryk for ui-review, because I believe Patryk did quite a bit of work on smart collections in the past.
Attachment #8471158 -
Flags: ui-review?(padamczyk)
Attachment #8471158 -
Flags: ui-review?(hnguyen)
Updated•10 years ago
|
Component: Gaia → Gaia::Everything.me
Comment 12•10 years ago
|
||
If indeed there will no wallpaper behind the header, let's please remove this code: https://github.com/mozilla-b2g/gaia/blob/master/apps/collection/js/view_collection.js#L36 and close bug 1020957
Comment 13•10 years ago
|
||
Comment on attachment 8471158 [details]
Collections - New header.png
Loaded the patch and it looks good, one thing I would change is the header colour, on the flame it feels way too bright can we use #333333 instead for these collections? Thanks!
Attachment #8471158 -
Flags: ui-review?(padamczyk)
Attachment #8471158 -
Flags: ui-review?(hnguyen)
Attachment #8471158 -
Flags: ui-review+
Comment 14•10 years ago
|
||
Comment on attachment 8469454 [details]
pull-request (master)
Sounds good to me then. Thanks Wilson!
Attachment #8469454 -
Flags: review?(kgrandon) → review+
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8469454 [details] pull-request (master) LANDED https://github.com/mozilla-b2g/gaia/commit/dfaecc8154daa4edaa631934264e9848e213d091
Assignee | ||
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•