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

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

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)

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

To be more specific, this is the "Add a site" tile.

Comment 2

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Malayaleecoder mentions they're interested in working on this!

Comment 3

[Vishnu (: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!

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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.

Comment 5

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(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

Comment 6

[Vishnu (:malayaleecoder)]

Ok, Thanks Anthony!
I will go through TopSitesPanel.java and TopSitesGridView.java and check what I can find and will revert back ASAP.

Comment 7

[Vishnu (: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.

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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

Comment 9

[friedger]

It looks like that the "add site" tile is gone in Nightly 55.0a1-2017-06-11 (or before).

Comment 10

[friedger]

Attachment: Bug 1242629 - Use our color palette for top sites "add a site" tiles

add flag for default border

Review commit: https://reviewboard.mozilla.org/r/149072/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/149072/

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

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

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

Sorry this took so long, Friedger. This looks good to me. I'll land the patch.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db3196ee978a
Use our color palette for top sites "add a site" tiles r=sebastian

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/db3196ee978a