[MOB-3164] Clean Upgrade leftovers

[lucaschifino]

MOB-3164

Context

Due to the folder change on the upgrade, there were some leftover files with Ecosia changes.

Approach

Review leftovers and either:

1. Delete the file if not relevant
2. Apply Ecosia changes to new files and delete the leftover
3. Create follow-up tickets for specific flows (see this ticket comment)

Other

Before merging

• ⏳ Wait for [MOB-3113] Add Ecosia Framework, remove Core and update tests #837 to be merged before so this doesn't introduce more conflicts

Checklist

• I performed some relevant testing on a real device and/or simulator
• I added the // Ecosia: helper comments where needed

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit cbf8305)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Hardcoded Theme Color The addition of UX.hardcodedDarkTextColor introduces a hardcoded theme color. Ensure this aligns with the dynamic theming approach and does not conflict with other theme settings. static let hardcodedDarkTextColor = EcosiaDarkTheme().colors.ecosia.textPrimary Incomplete Commented Code The commented-out code block in applySelectedStyle should either be removed or clarified if it is intended for future use. This could lead to confusion or maintenance issues. /* Update theme favicon.tintColor = colors.textPrimary titleText.textColor = colors.textPrimary closeButton.tintColor = colors.textPrimary let isToolbarRefactorEnabled = featureFlags.isFeatureEnabled(.toolbarRefactor, checking: .buildOnly) let backgroundColor = isToolbarRefactorEnabled ? colors.actionTabActive : colors.layer2 cellBackground.backgroundColor = backgroundColor cellBackground.layer.shadowColor = colors.shadowDefault.cgColor cellBackground.isHidden = false */ cellBackground.backgroundColor = colors.ecosia.buttonBackgroundPrimary backgroundView?.backgroundColor = colors.ecosia.ntpCellBackground let tint = isSelectedTab ? colors.ecosia.textInversePrimary : colors.ecosia.textPrimary titleText.textColor = tint closeButton.tintColor = tint favicon.tintColor = tint cellBackground.isHidden = !isSelectedTab Disabled UI Functionality The removal of UI functionality for default search engine selection should be validated to ensure it aligns with the intended user experience and product requirements. /* Ecosia: Remove UI that let User indicate a detail scren cell.editingAccessoryType = .disclosureIndicator */ cell.accessibilityLabel = .Settings.Search.AccessibilityLabels.DefaultSearchEngine cell.accessibilityValue = engine.shortName cell.textLabel?.text = engine.shortName cell.imageView?.image = engine.image.createScaled(IconSize) cell.imageView?.layer.cornerRadius = 4 cell.imageView?.layer.masksToBounds = true cell.applyTheme(theme: themeManager.getCurrentTheme(for: windowUUID)) case .alternateEngines: // The default engine is not an alternate search engine. let index = indexPath.item + 1 if index < model.orderedEngines.count { engine = model.orderedEngines[index] cell.showsReorderControl = true let toggle = ThemedSwitch() toggle.applyTheme(theme: themeManager.getCurrentTheme(for: windowUUID)) // This is an easy way to get from the toggle control to the corresponding index. toggle.tag = index toggle.addTarget(self, action: #selector(didToggleEngine), for: .valueChanged) toggle.isOn = model.isEngineEnabled(engine) cell.editingAccessoryView = toggle cell.textLabel?.text = engine.shortName cell.textLabel?.adjustsFontSizeToFitWidth = true cell.textLabel?.minimumScaleFactor = 0.5 cell.textLabel?.numberOfLines = 0 cell.imageView?.image = engine.image.createScaled(IconSize) cell.imageView?.layer.cornerRadius = 4 cell.imageView?.layer.masksToBounds = true cell.selectionStyle = .none cell.applyTheme(theme: themeManager.getCurrentTheme(for: windowUUID)) } else { cell.editingAccessoryType = .disclosureIndicator cell.accessibilityLabel = .SettingsAddCustomEngineTitle cell.accessibilityIdentifier = AccessibilityIdentifiers.Settings.Search.customEngineViewButton cell.textLabel?.text = .SettingsAddCustomEngine cell.applyTheme(theme: themeManager.getCurrentTheme(for: windowUUID)) } case .searchEnginesSuggestions: switch indexPath.item { case SearchSuggestItem.defaultSuggestions.rawValue: buildSettingWith( prefKey: PrefsKeys.SearchSettings.showSearchSuggestions, defaultValue: model.shouldShowSearchSuggestions, titleText: String.localizedStringWithFormat( .Settings.Search.ShowSearchSuggestions ), cell: cell, selector: #selector(didToggleSearchSuggestions) ) case SearchSuggestItem.privateSuggestions.rawValue: if featureFlags.isFeatureEnabled(.feltPrivacySimplifiedUI, checking: .buildOnly), !featureFlags.isFeatureEnabled(.firefoxSuggestFeature, checking: .buildAndUser) { buildSettingWith( prefKey: PrefsKeys.SearchSettings.showPrivateModeSearchSuggestions, defaultValue: model.shouldShowPrivateModeSearchSuggestions, titleText: String.localizedStringWithFormat( .Settings.Search.PrivateSessionSetting ), statusText: String.localizedStringWithFormat( .Settings.Search.PrivateSessionDescription ), cell: cell, selector: #selector(didToggleShowSearchSuggestionsInPrivateMode) ) } default: break } case .firefoxSuggestSettings: switch indexPath.item { case FirefoxSuggestItem.browsingHistory.rawValue: buildSettingWith( prefKey: PrefsKeys.SearchSettings.showFirefoxBrowsingHistorySuggestions, defaultValue: model.shouldShowBrowsingHistorySuggestions, titleText: String.localizedStringWithFormat( .Settings.Search.Suggest.SearchBrowsingHistory ), cell: cell, selector: #selector(didToggleBrowsingHistorySuggestions) ) case FirefoxSuggestItem.bookmarks.rawValue: buildSettingWith( prefKey: PrefsKeys.SearchSettings.showFirefoxBookmarksSuggestions, defaultValue: model.shouldShowBookmarksSuggestions, titleText: String.localizedStringWithFormat( .Settings.Search.Suggest.SearchBookmarks ), cell: cell, selector: #selector(didToggleBookmarksSuggestions) ) case FirefoxSuggestItem.syncedTabs.rawValue: buildSettingWith( prefKey: PrefsKeys.SearchSettings.showFirefoxSyncedTabsSuggestions, defaultValue: model.shouldShowSyncedTabsSuggestions, titleText: String.localizedStringWithFormat( .Settings.Search.Suggest.SearchSyncedTabs ), cell: cell, selector: #selector(didToggleSyncedTabsSuggestions) ) case FirefoxSuggestItem.nonSponsored.rawValue: if featureFlags.isFeatureEnabled(.firefoxSuggestFeature, checking: .buildAndUser) { buildSettingWith( prefKey: PrefsKeys.SearchSettings.showFirefoxNonSponsoredSuggestions, defaultValue: model.shouldShowFirefoxSuggestions, titleText: String.localizedStringWithFormat( .Settings.Search.Suggest.ShowNonSponsoredSuggestionsTitle ), statusText: String.localizedStringWithFormat( .Settings.Search.Suggest.ShowNonSponsoredSuggestionsDescription, AppName.shortName.rawValue ), cell: cell, selector: #selector(didToggleEnableNonSponsoredSuggestions) ) } case FirefoxSuggestItem.sponsored.rawValue: if featureFlags.isFeatureEnabled(.firefoxSuggestFeature, checking: .buildAndUser) { buildSettingWith( prefKey: PrefsKeys.SearchSettings.showFirefoxSponsoredSuggestions, defaultValue: model.shouldShowSponsoredSuggestions, titleText: String.localizedStringWithFormat( .Settings.Search.Suggest.ShowSponsoredSuggestionsTitle ), statusText: String.localizedStringWithFormat( .Settings.Search.Suggest.ShowSponsoredSuggestionsDescription, AppName.shortName.rawValue ), cell: cell, selector: #selector(didToggleEnableSponsoredSuggestions) ) } // case FirefoxSuggestItem.privateSuggestions.rawValue: // buildSettingWith( // prefKey: PrefsKeys.SearchSettings.showPrivateModeFirefoxSuggestions, // defaultValue: model.shouldShowPrivateModeFirefoxSuggestions, // titleText: String.localizedStringWithFormat( // .Settings.Search.PrivateSessionSetting // ), // statusText: String.localizedStringWithFormat( // .Settings.Search.Suggest.PrivateSessionDescription // ), // cell: cell, // selector: #selector(didToggleShowFirefoxSuggestionsInPrivateMode) // ) // cell.isHidden = shouldHidePrivateModeFirefoxSuggestSetting case FirefoxSuggestItem.suggestionLearnMore.rawValue: cell.accessibilityLabel = String.localizedStringWithFormat( .Settings.Search.AccessibilityLabels.LearnAboutSuggestions, AppName.shortName.rawValue ) cell.textLabel?.text = String.localizedStringWithFormat( .Settings.Search.Suggest.LearnAboutSuggestions, AppName.shortName.rawValue ) cell.imageView?.layer.cornerRadius = 4 cell.imageView?.layer.masksToBounds = true cell.selectionStyle = .none cell.applyTheme(theme: themeManager.getCurrentTheme(for: windowUUID)) default: break } } // So that the separator line goes all the way to the left edge. cell.separatorInset = .zero return cell } override func numberOfSections(in tableView: UITableView) -> Int { sectionsToDisplay = [ .defaultEngine, .alternateEngines, .searchEnginesSuggestions, .firefoxSuggestSettings ] return sectionsToDisplay.count } override func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { let section = Section(rawValue: sectionsToDisplay[section].rawValue) ?? .defaultEngine switch section { case .defaultEngine: return 1 case .alternateEngines: // The first engine -- the default engine -- is not shown in the alternate search engines list. // But the option to add a Search Engine is. return model.orderedEngines.count case .searchEnginesSuggestions: return featureFlags.isFeatureEnabled(.feltPrivacySimplifiedUI, checking: .buildOnly) && !featureFlags.isFeatureEnabled(.firefoxSuggestFeature, checking: .buildAndUser) ? SearchSuggestItem.allCases.count : 1 case .firefoxSuggestSettings: return featureFlags.isFeatureEnabled(.firefoxSuggestFeature, checking: .buildAndUser) ? FirefoxSuggestItem.allCases.count : 3 } } // MARK: - UITableViewDelegate override func tableView(_ tableView: UITableView, willSelectRowAt indexPath: IndexPath) -> IndexPath? { let section = Section(rawValue: sectionsToDisplay[indexPath.section].rawValue) ?? .defaultEngine switch section { case .defaultEngine: guard indexPath.item == 0 else { return nil } /* Ecosia: Hide possibility to show Default Engine selection screen let searchEnginePicker = SearchEnginePicker(windowUUID: windowUUID) // Order alphabetically, so that picker is always consistently ordered. // Every engine is a valid choice for the default engine, even the current default engine. searchEnginePicker.engines = model.orderedEngines.sorted { e, f in e.shortName < f.shortName } searchEnginePicker.delegate = self searchEnginePicker.selectedSearchEngineName = model.defaultEngine?.shortName navigationController?.pushViewController(searchEnginePicker, animated: true) */

[lucaschifino]

Ecosia changes in this test handled in #837

[lucaschifino]

Ecosia changes in this file handled in #837

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to cbf8305
Explore these optional code suggestions:

Category	Suggestion	Score
General	Ensure theme compatibility for text color Ensure that UX.hardcodedDarkTextColor is compatible with all themes, especially if the EcosiaDarkTheme is not applied universally, to avoid potential visual inconsistencies. firefox-ios/Client/Ecosia/UI/MultiplyImpact/MultiplyImpact.swift [18] -static let hardcodedDarkTextColor = EcosiaDarkTheme().colors.ecosia.textPrimary +static let hardcodedDarkTextColor = ThemeManager.shared.currentTheme.colors.ecosia.textPrimary Suggestion importance[1-10]: 8 Why: The suggestion improves theme compatibility by dynamically fetching the current theme's text color instead of hardcoding it to a specific theme. This change reduces the risk of visual inconsistencies across different themes.	8
	Handle table row deselection safely Ensure that deselecting the table view row when hiding the default engine selection screen does not interfere with user interaction or cause unexpected behavior. firefox-ios/Client/Frontend/Settings/SearchSettingsTableViewController.swift [382] -tableView.deselectRow(at: indexPath, animated: false) +if let selectedRow = tableView.indexPathForSelectedRow { + tableView.deselectRow(at: selectedRow, animated: true) +} Suggestion importance[1-10]: 7 Why: The suggestion improves the safety of table row deselection by checking if a row is selected before attempting to deselect it. This prevents potential issues with user interaction or unexpected behavior.	7
	Remove unnecessary commented-out code Remove or refactor the commented-out code block to avoid confusion and maintain code clarity. firefox-ios/Client/Frontend/Browser/TopTabCell.swift [121-125] -/* Update theme -favicon.tintColor = colors.textPrimary -titleText.textColor = colors.textPrimary -closeButton.tintColor = colors.textPrimary +favicon.tintColor = colors.ecosia.textPrimary +titleText.textColor = colors.ecosia.textPrimary +closeButton.tintColor = colors.ecosia.textPrimary Suggestion importance[1-10]: 6 Why: Removing the commented-out code improves code clarity and maintainability. However, the suggestion does not address whether the commented-out code is still relevant or could be refactored elsewhere.	6
Possible issue	Prevent runtime crashes from theme initialization Validate that EcosiaLightTheme is always available and initialized properly to prevent runtime crashes when setting button colors. firefox-ios/Client/Ecosia/UI/Onboarding/Welcome.swift [151] -cta.backgroundColor = EcosiaLightTheme().colors.ecosia.buttonBackgroundSecondary +cta.backgroundColor = ThemeManager.shared.currentTheme.colors.ecosia.buttonBackgroundSecondary Suggestion importance[1-10]: 7 Why: The suggestion ensures that the theme is dynamically fetched from the ThemeManager, which is safer and prevents potential runtime crashes if the EcosiaLightTheme is not properly initialized. However, it assumes the ThemeManager is correctly implemented and accessible.	7

---

Previous suggestions

Suggestions up to commit cbf8305

Category	Suggestion	Score
General	Avoid hardcoding theme-specific colors Ensure that UX.hardcodedDarkTextColor is compatible with all themes, as hardcoding a color might lead to inconsistencies in dynamic theme changes. firefox-ios/Client/Ecosia/UI/MultiplyImpact/MultiplyImpact.swift [18] -static let hardcodedDarkTextColor = EcosiaDarkTheme().colors.ecosia.textPrimary +static let hardcodedDarkTextColor = ThemeManager.shared.currentTheme.colors.ecosia.textPrimary Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential issue with hardcoding a theme-specific color, which could lead to inconsistencies when themes change dynamically. Using the current theme from a shared ThemeManager is a more robust solution.	8
	Ensure sufficient contrast for accessibility Verify that EcosiaLightTheme().colors.ecosia.buttonBackgroundSecondary and EcosiaLightTheme().colors.ecosia.textPrimary are accessible and provide sufficient contrast for readability. firefox-ios/Client/Ecosia/UI/Onboarding/Welcome.swift [151-155] cta.backgroundColor = EcosiaLightTheme().colors.ecosia.buttonBackgroundSecondary cta.setTitleColor(EcosiaLightTheme().colors.ecosia.textPrimary, for: .normal) +assert(AccessibilityChecker.isContrastSufficient(backgroundColor: cta.backgroundColor, textColor: cta.titleColor(for: .normal))) Suggestion importance[1-10]: 7 Why: The suggestion adds a check for sufficient contrast, which is important for accessibility compliance. However, the proposed code introduces an assertion that might not be suitable for production environments without proper handling.	7
	Remove commented-out code for clarity Remove or refactor the commented-out code block to avoid confusion and maintain code clarity. firefox-ios/Client/Frontend/Browser/TopTabCell.swift [121-125] -/* Update theme -favicon.tintColor = colors.textPrimary -titleText.textColor = colors.textPrimary -closeButton.tintColor = colors.textPrimary -*/ +// Removed unused code block for clarity Suggestion importance[1-10]: 6 Why: Removing commented-out code improves code clarity and maintainability. However, the suggestion does not provide a clear plan for handling the removed functionality, which slightly reduces its impact.	6
	Validate intentional hiding of functionality Ensure that the commented-out code for hiding the default engine selection screen is intentional and does not affect user functionality unexpectedly. firefox-ios/Client/Frontend/Settings/SearchSettingsTableViewController.swift [373-381] -/* Ecosia: Hide possibility to show Default Engine selection screen -let searchEnginePicker = SearchEnginePicker(windowUUID: windowUUID) -navigationController?.pushViewController(searchEnginePicker, animated: true) -*/ +// Ensure this change aligns with product requirements and does not impact user experience negatively Suggestion importance[1-10]: 5 Why: The suggestion highlights the need to ensure that the commented-out code aligns with product requirements. While it is a valid concern, it does not provide a concrete solution or improvement to the code itself.	5

[lucaschifino]

Ecosia changes in this file handled in #835

(see this review comment)

[github-actions]

Persistent review updated to latest commit cbf8305

[d4r1091]

Great cleanup! Always satisfying to see a lot of deleted rows! 🙌 .
Nothing major at all, just a 🤏 update on a comment

[d4r1091]

Let's gooo! 🚀