1251688 - Support Deletion of Suggested Tiles

Status: RESOLVED FIXED

(Firefox for iOS :: Browser, defect)

Comment 1

[Stephan Leroux [:sleroux]]

Here's the skinny on TopSitesPanel and adding support for deleting suggested tiles:

1. The 'removal' code should partially work regardless of what kind of tile it is. The only reason we don't show the [x] for suggested tiles is because we explicitly hide them for suggested tiles [1].

2. The challenging part of removing suggested tiles is that in both the deletion logic and data source we factor in the SuggestedSites separately from the normal top sites. For example, when deleting, we add the number of suggested sites to the data source and use that for determining which cell to delete [2]. We also hide this inside the data source [3]. 

3. Another issue is that we don't actually store any state for a suggested tile anywhere. They are all statically provided from SuggestedSites.swift [4]. We'll need to add a way of remembering when a user deletes one.

Some ideas:

* I think the first step to bringing some sanity into TopSitesPanel is to rid the notion of appending SuggestedSites to the data source every time we use it and bring a uniform view of the sites that we display. This way the removal logic can be somewhat agnostic of what kind of site we're deleting. This might look like building a wrapping cursor object that delegates to two child cursors for top sites/suggested sites so from the outside it looks like a single cursor.

* Along with the cursor abstraction, we could also delegate the persistence/deletion to this object as well so when we tell the object to delete a tile, the TopSitesPanel doesn't care how we delete it - just that it gets deleted. Behind the scenes we can implement the SQL deletion for TopSites and something else for suggested sites.

Overall I think we'll need to build up some abstractions on top of what we have now to make it easier to manage deleting both types of cells while keeping our code clean.

[1] https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Home/TopSitesPanel.swift#L154
[2] https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Home/TopSitesPanel.swift#L197
[3] https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Home/TopSitesPanel.swift#L582
[4] https://github.com/mozilla/firefox-ios/blob/master/Storage/SuggestedSites.swift#L29

Comment 2

[Bryan Munar (:bkmunar)]

Attachment: PR

Comment 3

[Stephan Leroux [:sleroux]]

Comment on attachment 8725084 [details] [review]
PR

Good start - left some comments

Comment 4

[Stephan Leroux [:sleroux]]

Comment on attachment 8725084 [details] [review]
PR

Awesome stuff - looks good. Thanks for revisions!

Comment 5

[Richard Newman [:rnewman]]

Re-read this:

https://mail.mozilla.org/pipermail/mobile-firefox-dev/2016-March/001791.html

Comment 6

[Bryan Munar (:bkmunar)]

oops forgot/didn't see that, thanks!

Comment 7

[Bryan Munar (:bkmunar)]

https://github.com/mozilla/firefox-ios/commit/5e55b1057c1dc2b8ce450aa8ab9fdca895b0f461

Comment 8

[Stephan Leroux [:sleroux]]

v3.x 0cfcf23