Use our color palette for top sites "add a site" tiles

RESOLVED FIXED in Firefox 56

Status

()

Firefox for Android
General
RESOLVED FIXED
2 years ago
10 months ago

People

(Reporter: antlam, Assigned: friedger, Mentored)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 56
All
Android
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [lang=java][good next bug])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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

2 years ago
Malayaleecoder mentions they're interested in working on this!
Flags: needinfo?(malayaleecoder)
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

2 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)
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)
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)
(Assignee)

Comment 9

11 months ago
It looks like that the "add site" tile is gone in Nightly 55.0a1-2017-06-11 (or before).
Comment hidden (mozreview-request)
(Assignee)

Updated

10 months ago
Attachment #8877601 - Flags: review?(s.kaspari)

Comment 11

10 months 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+
Assignee: nobody → mail
Status: NEW → ASSIGNED
Sorry this took so long, Friedger. This looks good to me. I'll land the patch.

Comment 13

10 months 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

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/db3196ee978a
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.