-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Remove FXIOS-9088 - Remove Pocket Share Sheet Experiment #20173
Conversation
} | ||
|
||
/// Exclude 'Add to Reading List' which currently uses Safari. If share sheet changes are enabled exclude | ||
/// Copy from system to provide custom activity |
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.
@adudenamedruby this comment was not clear to me so I removed it lol, but I think the logic below is right?
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 comment is not clear to me either. Did I write it? lololol Logic looks fine to me, though, yeah.
@adudenamedruby tagged you on this one because you're the Nimbus KingTM and this is my first PR touching feature flags. I feel like there's a file I need to remove somewhere that I'm forgetting about but not sure... |
Client.app: Coverage: 31.23
Generated by 🚫 Danger Swift against 705fcb8 |
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.
Halmost there. Just more delets needed. Moar. MOAR!
} | ||
|
||
/// Exclude 'Add to Reading List' which currently uses Safari. If share sheet changes are enabled exclude | ||
/// Copy from system to provide custom activity |
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 comment is not clear to me either. Did I write it? lololol Logic looks fine to me, though, yeah.
dd4060a
to
8797dde
Compare
Ready for another look! |
@adudenamedruby Also please see recent commit where I just took out the share button from the toolbar completely - double checked today and we weren't using it outside of this experiment so thought I would just remove it all. |
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.
LGTM!
278380c
to
705fcb8
Compare
@Mergifyio backport release/v126 |
@dataports looks like a backport was not created for some reason. Not sure why, your comment was correct. |
@Mergifyio backport release/v126 |
✅ Backports have been created
|
(cherry picked from commit a6329c8)
📜 Tickets
Jira ticket
Github issue
💡 Description
Removes Pocket share sheet experiment which effected the context menu options when long pressing items in the the tab tray, and whether the share to device menu appears in the Settings menu.
Removed code for enabling the share button in the toolbar.
📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120
)