Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MOB-3113] Add Ecosia Framework, remove Core and update tests #835

Conversation

d4r1091
Copy link
Member

@d4r1091 d4r1091 commented Jan 31, 2025

MOB-3113

Context

We want to pick up the work done here #816 and progress by adding the Ecosia Framework as part of the recent Firefox Upgrade.
Below is the reference message highlighting what was done as part of the Ecosia Framework.

#816 (comment)

Moreover, we wanted to add the Ecosia Framework and some fixes to Unit and Snapshot Tests.

🚧 🚧 🚧
There are still a few things to check after having this PR merged (luckily after #834)

Take this first Draft step as an overall review.
🚧 🚧 🚧

Other

There may be some changes that still need to be reverted after aligning with the latest upgrade branch.

Before merging

Checklist

  • I performed some relevant testing on a real device and/or simulator
  • I updated only the english localization source files if needed
  • I added the // Ecosia: helper comments where needed
  • I made sure that any change to the Analytics events included in PR won't alter current analytics (e.g. new users, upgrading users)

thatswinnie and others added 30 commits October 21, 2024 09:30
…e after opening website with QR scan (mozilla-mobile#22616)

* Set editing to fals when QR code reader is shown

* Clean up code that is already in LocationTextField

* Revert: Set editing to fals when QR code reader is shown

* Dispatch cancelEdit action before showing QR code reader to end edit mode
…ance (mozilla-mobile#22528)

* Add waitAndTap()

* Update BaseTestCase.swift

* Added guard for better readability and early exit

* Remove unnecessary timeout
…ute in any locale (mozilla-mobile#22658)

Update url validation test to execute in any locale
…-mobile#22664)

* Bugfix Bookmarks cell not setting background blur visual effect when wallpaper is enabled

---------

Co-authored-by: Filippo <[email protected]>
* Now the active tab is in the bottom. Scroll up to see the "Homepage".

* iPad specific code

* Do not need to check tab tray has been expanded
…orm submission (mozilla-mobile#22637)

* completed ticket

* Updated logic

* updated function name

* Addressed Comment
…la-mobile#22662)

Add ToDo comments per 10/18 incident reverts and related FXIOS-10334 ticket
…1021163120 (mozilla-mobile#22675)

Auto update SPM with latest rust-component release 133.0.20241021163120

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…ab (mozilla-mobile#22596)

* Refactor to make tab tray scrolling behaviour more granular. Allows us to scroll to a restored closed tab (with or without animation).

* Refactor to move the scroll behavior into its own separate middleware action.

* Only scroll to a cell when it's not already fully in view. Check that an indexPath is valid before scrolling to avoid potential crashes.

* Bugfix for reducer returning previous state without clearing transient properties.

* Add logic to TabsPanelState createTabScrollBehavior logic for determining the correct scroll destination tab index from a scroll action.

* Add check in TabsDisplayView to only respond to TabsPanelState updates for the assigned panelType.

* Add unit tests for TabPanelState createTabScrollBehavior method.

* Refactoring and documentation updates.
…redux bug and move tab close toast to the TabTrayState (mozilla-mobile#22625)

* Bugfix for redux bug with TabTrayState passing previous state instead of initializing new state on global unrelated state updates. 

* Removed hideUndoToast action as it is no longer necessary thanks to bugfix. 

* Refactor the toast display logic up a level from TabDisplayPanel to TabTrayViewController to prevent glitches.
…ts for Unified Search (mozilla-mobile#22655)

* Preliminary collection of strings and accessibility labels for the Unified Search project work.
Localize [v133] String import 2024-10-21

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* be a team player and rename things according to spec

* be a team player and add zoom functionality

* ok but how did this get in in the first place?

* fix test

* spaces are places too
* Fix build errors

* Fix Swiftlint warnings
…1022050402 (mozilla-mobile#22696)

Auto update SPM with latest rust-component release 133.0.20241022050402

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
… and stub SearchEngineSelectionViewController (mozilla-mobile#22702)

* Add the SearchEngineSelectionCoordinator. Stub in the SearchEngineSelectionViewController.

* Navigate to show the search engine selection from the toolbar's didTap delegate method.

* Add basic theming to the SearchEngineSelectionViewController stub. 

* Add navigation to open settings from the search engine selection bottom sheet.

* Add unit tests for the SearchEngineSelectionViewController. Add additional BrowserCoordinator and SearchEngineSelectionCoordinator tests.
… QR Code should be hidden (mozilla-mobile#22695)

* Hide qr code button when user starts typing

* Remove unused code

* Reset search when user types over highlighted text

* Dispatch canceling edit mode only once

* Save search term when starting to edit

* Use default values for reducer init

* Add showQRPageAction to state

* Add comment

* Rename redux actions

* Set state didStartTyping when the user types in address toolbar
Copy link
Collaborator

@lucaschifino lucaschifino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work! Can imagine the effort the project file changes required 🙈 Even github is struggling to show all changes 😅

Just seeing the 🟢 is already awesome ❤️

Looked over it quickly and looks good to me from what I had already reviewed on #816 👏

Will have a more careful look once it's ready to review 👀

.swiftlint.yml Outdated
@@ -164,7 +164,10 @@ excluded: # paths to ignore during linting. Takes precedence over `included`.
- focus-ios/
# Ecosia: Exclude Swift Packages
- SourcePackages
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can be removed in favor of the line below. Wonder why it did not complain before 🤔

Comment on lines 158 to 162
public static func defaultSites() -> [Site] {
/* Ecosia: Simpli return Ecosia sites
let locale = Locale.current
let defaultSites = sites[locale.identifier] ?? sites["default"]
return defaultSites?.map { data in
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While reviewing leftover files, I realised that this defaultSites function is equivalent to what we used to do in SuggestedSitesCursor (these lines), which is then used in TopSitesProvider.

I don't fully understand what's the difference from that to simply returning the sites here, but did you test whether they result the same?

Btw, will remove the leftover files (DefaultSuggestedSites and SuggestedSite) on the old Storage folder since you've handled reintroducing their changes here 🙏

@d4r1091 d4r1091 force-pushed the mob-3113-firefox-upgrade-133-with-ecosia-framework branch from f346738 to c0b7c37 Compare February 4, 2025 20:28
@d4r1091 d4r1091 closed this Feb 4, 2025
@d4r1091 d4r1091 force-pushed the mob-3113-firefox-upgrade-133-with-ecosia-framework branch from c0b7c37 to 970f5b4 Compare February 4, 2025 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.