Closed
Bug 1242629
Opened 9 years ago
Closed 7 years ago
Use our color palette for top sites "add a site" tiles
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox56 fixed)
RESOLVED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: antlam, Assigned: mail, Mentored)
References
Details
(Whiteboard: [lang=java][good next bug])
Attachments
(1 file)
Leftover from: https://bugzilla.mozilla.org/show_bug.cgi?id=1216987#c16
Let's clean this up too:
- top sites default (background fill color) -> "about page header grey" (#F5F5F5)
- top sites border -> 1dp divider grey (#D7D9DB)
To be more specific, this is the "Add a site" tile.
Mentor: michael.l.comella, s.kaspari
Summary: Use our color palette for top sites tiles → Use our color palette for top sites "add a site" tiles
Whiteboard: [lang=java][good next bug]
Reporter | ||
Comment 2•9 years ago
|
||
Malayaleecoder mentions they're interested in working on this!
Flags: needinfo?(malayaleecoder)
Comment 3•9 years ago
|
||
Hello Anthony!
Can you get me started by suggesting the files to work on? Let me also confirm what I am supposed to do to get this bug fixed, add a border to "Add a site" tile and also change its background fill color.
Do correct me if I am wrong!
Flags: needinfo?(malayaleecoder) → needinfo?(alam)
I don't know the specific files off the top of my head but the general outline is we have a `TopSitesPanel` fragment that houses the gridView (`TopSitesGridView`) you'd be updated.
So I'd start looking in TopSitesPanel.java & TopSitesGridView.java.
If you need more specifics on which files to touch, let me know and I can dig in further.
Reporter | ||
Comment 5•9 years ago
|
||
(In reply to malayaleecoder from comment #3)
> Hello Anthony!
>
> Can you get me started by suggesting the files to work on? Let me also
> confirm what I am supposed to do to get this bug fixed, add a border to "Add
> a site" tile and also change its background fill color.
> Do correct me if I am wrong!
I think that's correct :) I'll let mcomella take it from here
Flags: needinfo?(alam) → needinfo?(malayaleecoder)
Comment 6•9 years ago
|
||
Ok, Thanks Anthony!
I will go through TopSitesPanel.java and TopSitesGridView.java and check what I can find and will revert back ASAP.
Flags: needinfo?(malayaleecoder)
Comment 7•9 years ago
|
||
Hello Michael,
I have roughly gone through TopSitesPanel.java and TopSitesGridView.java.
I am still not able to find the correct method.
Any suggestions regarding this?
Thank You.
Flags: needinfo?(michael.l.comella)
It looks like we set the "Add site" tile in TopSitesGridItemView.blankOut [1].
So you have some idea of how I found that, my approach to find this was to open TopSitesGridView and start trying to figure out where the top sites tiles are populated. Typically, this is from an Adapter, however, I found a shortcut with the `dummyChildView` member variable, which is of type `TopSitesGridItemView`. I opened it up and tried to identify where the default state might be, which led me to the `TopSitesGridItemView.updateType` method, where I saw TopSites.TYPE_PINNED. I went to the item definition, found TopSites.TYPE_BLANK, where I "found usages" in Android Studio, which led me to the `blankOut` method.
[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/TopSitesGridItemView.java#136
Flags: needinfo?(michael.l.comella)
It looks like that the "add site" tile is gone in Nightly 55.0a1-2017-06-11 (or before).
Comment hidden (mozreview-request) |
Attachment #8877601 -
Flags: review?(s.kaspari)
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8877601 [details]
Bug 1242629 - Use our color palette for top sites "add a site" tiles
https://reviewboard.mozilla.org/r/149072/#review160742
Attachment #8877601 -
Flags: review?(s.kaspari) → review+
Updated•7 years ago
|
Assignee: nobody → mail
Status: NEW → ASSIGNED
Comment 12•7 years ago
|
||
Sorry this took so long, Friedger. This looks good to me. I'll land the patch.
Comment 13•7 years ago
|
||
Pushed by s.kaspari@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/db3196ee978a
Use our color palette for top sites "add a site" tiles r=sebastian
Comment 14•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
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
•