-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Refactor FXIOS-10669, Subtask FXIOS-10820 [Sent from Firefox] Improve share "extension" type names and add documentation #23542
Conversation
Updated PR description. |
@@ -537,37 +541,6 @@ class BrowserCoordinator: BaseCoordinator, | |||
showShareSheet(with: url, title: nil) | |||
} | |||
|
|||
func showShareSheet(with url: URL?, title: String?) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This was just moved lower in the file)
Fixing 1 thing on the branch atm, getting infinite recursion from one of the renaming! 😆 |
Alright this has been fixed! There was a protocol with default implementation that was just calling itself since I didn't rename it and/or remove it properly. |
Client.app: Coverage: 31.51
Generated by 🚫 Danger Swift against cb01346 |
This pull request has conflicts when rebasing. Could you fix it @ih-codes? 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
Only thing I would change is the following in the PR description:
Renames ShareExtensionCoordinator -> ShareCoordinator
Renames ShareExtensionCoordinator -> ShareSheetCoordinator
Thanks @MattLichtenstein, done! I missed that when I had to write up the PR description a second time after the GitHub incident. 😭 |
… Update and add documentation. Update related unit tests and add a few test improvements.
… test naming to match.
…ests and mock naming.
7601e25
to
cb01346
Compare
📜 Tickets
Jira ticket
Github issue
Jira Subtask FXIOS-10820
💡 Description
This PR is part of the Sent from Firefox / Info Card Referral experiment work, which involves refactoring our old share sheet code.
This PR:
sharesheet
route to take aShareType
andShareMessage
instead of a URL and titleNote: The .sharesheet route was only just added for the Info Card Referral experiment here (https://github.com/mozilla-mobile/firefox-ios/pull/23094/files) and isn't in use yet. My next PR will hook things up again, I just didn't want too much clutter.
📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120
)