Add FXIOS-10165 [Homepage] Add initial Top Sites Section #22621
Conversation
|
This PR has been automatically marked as stale. Please leave any comment to keep this PR opened. It will be closed automatically if no further update occurs in the next 7 days. Thank you for your contributions! |
github-actions
bot
added
the
stale
|
This pull request has conflicts when rebasing. Could you fix it @cyndichin? 🙏 |
github-actions
bot
removed
the
stale
cyndichin
force-pushed
the
cc/FXIOS-10165_add-top-sites-section
branch
from
58ed566 to
bf495f3
Compare
cyndichin
had a problem deploying
to
github-pages
GitHub Actions
Failure
| return HomepageState( | ||
| windowUUID: state.windowUUID, | ||
| headerState: HeaderState.reducer(state.headerState, action), | ||
| topSitesState: TopSitesState.reducer(state.topSitesState, action), | ||
| pocketState: PocketState.reducer(state.pocketState, action) | ||
| ) |
There was a problem hiding this comment.
will create a separate method for the default state in a future PR
| case .ProfileDidFinishSyncing, | ||
| .PrivateDataClearedHistory, | ||
| .FirefoxAccountChanged, | ||
| .TopSitesUpdated, | ||
| .DefaultSearchEngineUpdated: |
There was a problem hiding this comment.
copied over from adaptor, but would like to drill down to why we need to observe these cases. If anyone has context, happy to hear, otherwise will investigate these further once I dive deeper into the top sites logic.
cyndichin
force-pushed
the
cc/FXIOS-10165_add-top-sites-section
branch
from
bf495f3 to
76317f4
Compare
| import UIKit | ||
|
|
||
| /// The TopSite cell that appears for the homepage rebuild project. | ||
| class TopSiteCell: UICollectionViewCell, ReusableCell { |
There was a problem hiding this comment.
this file is a copy of TopSiteItemCell, but changed the name and update the configure method to using TopSiteState instead of TopSite
| import Shared | ||
|
|
||
| /// Top site UI class, used in the homepage top site section | ||
| final class TopSiteState: Hashable, Equatable { |
There was a problem hiding this comment.
this is a copy of TopSite but conformed to Hashable and Equatable
|
In case it helps, looks like this PR is failing several times due to this Unit test: HomepageViewControllerTests.test_viewDidLoad_setsUpThemingAndNotifications() |
yes thank you! its in draft so I've been committing + pushing as I make changes, but I should probably make sure I don't push too often since it does run the CI / CD process 😅 |
cyndichin
force-pushed
the
cc/FXIOS-10165_add-top-sites-section
branch
from
cfd9eda to
61a558f
Compare
Client.app: Coverage: 30.07
Generated by 🚫 Danger Swift against c339fad |
cyndichin
requested review from
thatswinnie,
OrlaM,
adudenamedruby and
MattLichtenstein
and removed request for
thatswinnie
|
This pull request has conflicts when rebasing. Could you fix it @cyndichin? 🙏 |
cyndichin
force-pushed
the
cc/FXIOS-10165_add-top-sites-section
branch
from
61a558f to
c339fad
Compare
cyndichin
had a problem deploying
to
github-pages
GitHub Actions
Failure
|
@tusharC95 @DanielDervishi for visibility |
📜 Tickets
Jira ticket
Github issue
💡 Description
Adds the foundation for setting up top sites section:
Includes updating diffable datasource from state and presenting google top site in the view.
Before adding middleware tests, waiting for this PR to be approved: #22643
Next PRs
To keep this PR small, I will create separate future PRs:
Include full logic for top sites (ordering)
Include navigation
Removing + Adding favicons will come with context menus
Update diffable datasource tests
📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120)Screenshots