-
Notifications
You must be signed in to change notification settings - Fork 21
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
Replace the method to show SFSafariViewController #45
Conversation
This reverts commit 584ef12.
Signed-off-by: Daiki Matsudate <[email protected]> # Conflicts: # MyLibrary/Sources/Safari/Safari.swift
HI @touyou , Thank you for improving SF Safari experience. See how it work and cherry-pick my commit to improve your pull request. |
This might be help for you to check diff. I have no plan to merge my PR. Please try to complete your work! |
@d-date Thanks for very helpful commit and advice! |
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.
Thank you!
What
I replace the method to show SFSafariViewController because I found some usability problem of current methods.
This is how the interaction has changed.
Before
480pB.mov
After
480pA.mov
Why
I found these problems:
How
I'm TCA beginner, so sorry for implementing it by UIApplication extension functions.
I replaced these view:
Schedule← Revert itAnd not replace Sponsors because it already uses
fullScreenCover
.According to this article,
fullScreenCover
still has usability problem, but it's not critical and I decided not to replace it.