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

Add navigation bar to the avatar picker #358

Merged
merged 3 commits into from
Aug 22, 2024
Merged

Add navigation bar to the avatar picker #358

merged 3 commits into from
Aug 22, 2024

Conversation

pinarol
Copy link
Contributor

@pinarol pinarol commented Aug 20, 2024

Closes #343

Description

Adds navigation bar. I used NavigationView for now since it is supported in iOS 15. NavigationStack is supported only for iOS 16+.

  • Adds "Done" button to close the picker
  • Gravatar icon opens Safari with profile url

Updates the "Discover Your Gravatar Card" to "View profile" based on designs. Ties the action to open the profile in Safari.

Refactors some of the paddings based on the designs.

Testing Steps

  • Play around with the SwiftUI demo app.
@pinarol pinarol self-assigned this Aug 20, 2024
@pinarol pinarol marked this pull request as ready for review August 20, 2024 15:21
@pinarol pinarol requested a review from etoledom August 20, 2024 15:21
@wpmobilebot
Copy link

wpmobilebot commented Aug 20, 2024

Gravatar UIKit Prototype Build📲 You can test the changes from this Pull Request in Gravatar UIKit Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar UIKit Prototype Build Gravatar UIKit Prototype Build
Build Number1118
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-uikit.prototype-build
Commit4a4dd0b
App Center BuildGravatar SDK Demo - UIKit #54
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.
@wpmobilebot
Copy link

wpmobilebot commented Aug 20, 2024

Gravatar SwiftUI Prototype Build📲 You can test the changes from this Pull Request in Gravatar SwiftUI Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar SwiftUI Prototype Build Gravatar SwiftUI Prototype Build
Build Number1118
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-swiftui.prototype-build
Commit4a4dd0b
App Center BuildGravatar SDK Demo - SwiftUI #53
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Looks good to me!

:shipit:

There's one detail which troubles me a bit, but I don't think there's much we can do without making things overly complicated:

SFSafariViewController has a custom modal presentation animation which emulates a navigation push. It's the standard animation in which the in-app browser is presented.

CleanShot.2024-08-22.at.16.43.06.mp4

SwiftUI doesn't seem to be compatible with these custom presentation animations, and we lose the standard SFSafariViewController presentation.

I saw that most resources online mentions using .sheet presentation, which is the easiest. I tested using NavigationLink but it won't work either.
The closest is to use .fullScreenCover() instead of .sheet(), so it presents full screen.

The complex option I saw is to present SFSafariView in UIKit side from UIApplicationDelegate or UIWindow, and call it from SwiftUI using a global reference. But we also don't have the UIKit context in a pure swiftUI app. ¯_(ツ)_/¯

Either way, lets go along with this one. Hopefully Apple will do something about this at some point.

Either way, I'm fine how it is now

@pinarol
Copy link
Contributor Author

pinarol commented Aug 22, 2024

I liked "fullScreenCover" better. 👍 For now it seems like the best we can get out of SwiftUI about this.

@pinarol pinarol merged commit 675ad1f into trunk Aug 22, 2024
9 checks passed
@pinarol pinarol deleted the wppinar/add-navbar branch August 22, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants