[MOB-4150][MOB-4151] Hide NTP referral row, news section; fix shortcuts width and row cap#1060
Merged
falkorichter merged 10 commits intocopilot/add-background-to-ecosian-ntpfrom Feb 27, 2026
Conversation
e049bb0 to
aa8c55f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
docs/ntp-remove-referrals/README.md:3
- The document title says "Remove Referrals, News and Bookmarks Sections (MOB-4150)", but the changes describe hiding referrals (MOB-4150) and news (MOB-4151), plus library shortcuts (not the Firefox bookmarks section). Consider updating the title to reflect the correct tickets and the actual sections being gated so the doc remains a reliable reference.
# NTP: Remove Referrals, News and Bookmarks Sections (MOB-4150)
## Overview
firefox-ios/Client/Ecosia/Frontend/Home/EcosiaHomepageAdapter.swift:199
ToggleNTPNewsSectioncurrently only gates whether the.ecosiaNewssection is appended, butnewsViewModelstill subscribes inNTPNewsCellViewModel.initandrefreshData(...)is still called unconditionally. This means news can still be fetched/updated even when the section is hidden by default. Consider lazy/conditional initialization ofnewsViewModeland gating refresh/loading paths behind the same toggle to avoid background work and network usage when the feature is disabled.
private func shouldShowNews() -> Bool {
// Ecosia: Hidden by default (MOB-4150); re-enable via debug menu
guard ToggleNTPNewsSection.isEnabled else { return false }
return User.shared.showEcosiaNews
}
firefox-ios/Client/Ecosia/UI/NTP/Customization/CustomizableNTPSettingConfig.swift:16
- The comment references MOB-4150 for hiding the news setting, but the PR description ties news removal to MOB-4151. Updating the ticket reference here would avoid confusion when tracking why the case is excluded from
allCases.
// Ecosia: ecosiaNews hidden by default (MOB-4150); removed from the customization UI
case ecosiaNews
// Ecosia: manually provide allCases so ecosiaNews is excluded from the NTP customization row while
// the case itself remains for analytics and User.shared.showEcosiaNews persistence
static var allCases: [CustomizableNTPSettingConfig] { [.topSites, .climateImpact] }
Comment on lines
105
to
+109
| let minCardsConstant = TopSitesSectionLayoutProvider.UX.minCards | ||
| let maxCardsConstant = TopSitesSectionLayoutProvider.UX.maxCards | ||
| let tilesPerRowCount = numberOfTiles < minCardsConstant ? minCardsConstant : numberOfTiles | ||
|
|
||
| return tilesPerRowCount | ||
| // Ecosia: Respect maxCards cap so the row never exceeds 5 tiles (MOB-4150) | ||
| return min(tilesPerRowCount, maxCardsConstant) |
There was a problem hiding this comment.
This change caps numberOfTopSitesPerRow to 5, but there are existing unit tests (e.g. HomepageDimensionCalculatorTests) that currently assert higher values for iPad / landscape iPhone (7/8/10). Those tests will fail unless updated to reflect the new max-cards behavior (or made conditional if the cap is Ecosia-only).
2cc146a to
aa239c4
Compare
9d9f643 to
6afa5eb
Compare
6afa5eb to
88b0313
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes MOB-4150 — Remove referrals
Closes MOB-4151 — Remove news
Closes MOB-3999 - Update shortcuts to leverage Firefox implementation
Deactivates the NTP referral row and Ecosia news section by hiding them behind debug toggles (off by default). Also fixes layout inconsistencies: section widths are now consistent across all Ecosia NTP sections, and the shortcuts row is capped at 5 tiles to avoid a cramped layout.
What changed
NTPImpactCellViewModel.infoItemSectionsgates the second row (referral) behindToggleNTPReferralRow.isEnabled(UserDefaults"NTPShowReferralRow", defaultfalse)EcosiaHomepageAdapter.shouldShowNews()gates behindToggleNTPNewsSection.isEnabled(UserDefaults"NTPShowNewsSection", defaultfalse);.ecosiaNewsalso removed fromCustomizableNTPSettingConfig.allCasesso the user-facing NTP customization toggle disappearsDebugsettings section (tap version number 5× to reveal):Debug: Toggle - Show NTP Referral RowandDebug: Toggle - Show NTP News SectionAppSettingsTableViewController.generateSettings()was only building Firefox sections; now callsgetEcosiaSettingsSectionsShowingDebug()so Ecosia debug sections are actually showngetEcosiaSectionInsets(same as impact/referral/library/header), so all sections share the same horizontal margins on iPad and iPhone landscapeTopSitesSectionLayoutProvider.UX.maxCards = 5caps the computed tile count; previously the algorithm filled available width greedily and could produce 7+ icons per row on a wide iPadArchitecture
See
docs/ntp-remove-referrals/README.mdfor full architecture, file-by-file breakdown, and testing plan.Files changed
Client/Frontend/Settings/Main/AppSettingsTableViewController.swiftgenerateSettings()to Ecosia sectionsClient/Frontend/Home/Homepage/HomepageDimensionImplementation.swiftmaxCardscapClient/Frontend/Home/Homepage/Layout/TopSitesSectionLayoutProvider.swiftmaxCards = 5constantClient/Ecosia/UI/NTP/Impact/NTPImpactCellViewModel.swiftClient/Ecosia/Frontend/Home/EcosiaHomepageAdapter.swiftClient/Ecosia/UI/NTP/Customization/CustomizableNTPSettingConfig.swift.ecosiaNewsfromallCasesClient/Ecosia/Settings/EcosiaDebugSettings.swiftToggleNTPReferralRow,ToggleNTPNewsSectionClient/Ecosia/Extensions/AppSettingsTableViewController+Ecosia.swiftClient/Ecosia/Extensions/HomepageSectionLayoutProvider+Ecosia.swiftDeferred
Wallpaperalready decodestext-colorandlogo-text-colorfrom the JSON intoUIColor?properties. Wiring these intoNTPImpactRowView,NTPLogoCell, and the news section header (replacing the current hardcoded.white) was prototyped and reverted pending design sign-off on contrast/accessibility across all wallpapers. Follow-up ticket needed.Test plan
Debugsection appears (notDEBUG)Debug: Toggle - Show NTP Referral Row→ tap → referral row appears on NTPDebug: Toggle - Show NTP News Section→ tap → news section appears on NTP🤖 Generated with Claude Code