Closed Bug 1242629 Opened 5 years ago Closed 4 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

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]
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.
(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)
It looks like that the "add site" tile is gone in Nightly 55.0a1-2017-06-11 (or before).
Attachment #8877601 - Flags: review?(s.kaspari)
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.
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
https://hg.mozilla.org/mozilla-central/rev/db3196ee978a
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.