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 . 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 . We also hide this inside the data source . 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 . 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.  https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Home/TopSitesPanel.swift#L154  https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Home/TopSitesPanel.swift#L197  https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Home/TopSitesPanel.swift#L582  https://github.com/mozilla/firefox-ios/blob/master/Storage/SuggestedSites.swift#L29
Created attachment 8725084 [details] [review] PR
Comment on attachment 8725084 [details] [review] PR Good start - left some comments
Attachment #8725084 - Flags: feedback+
Comment on attachment 8725084 [details] [review] PR Awesome stuff - looks good. Thanks for revisions!
Attachment #8725084 - Flags: review?(sleroux) → review+
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
oops forgot/didn't see that, thanks!
status-fxios-v3.0: --- → fixed
Attachment #8725084 - Flags: ui-review?(randersen) → ui-review+
You need to log in before you can comment on or make changes to this bug.