-
Notifications
You must be signed in to change notification settings - Fork 230
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
Update Apple Quick Start Docs #560
base: main
Are you sure you want to change the base?
Update Apple Quick Start Docs #560
Conversation
In order to allow creating OAuth sessions, the following URL scheme must be added to your **Info.plist** file. | ||
|
||
```xml | ||
<key>CFBundleURLTypes</key> | ||
<array> | ||
<dict> | ||
<key>CFBundleTypeRole</key> | ||
<string>Editor</string> | ||
<key>CFBundleURLName</key> | ||
<string>io.appwrite</string> | ||
<key>CFBundleURLSchemes</key> | ||
<array> | ||
<string>appwrite-callback-[PROJECT_ID]</string> | ||
</array> | ||
</dict> | ||
</array> | ||
``` | ||
|
||
If you're using UIKit as opposed to SwiftUI, you will also need to add the following to your **SceneDelegate.swift** file. | ||
|
||
```swift | ||
func scene(_ scene: UIScene, openURLContexts URLContexts: Set<UIOpenURLContext>) { | ||
guard let url = URLContexts.first?.url, | ||
url.absoluteString.contains("appwrite-callback") else { | ||
return | ||
} | ||
|
||
WebAuthComponent.handleIncomingCookie(from: url) | ||
} | ||
``` |
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.
Let's keep this here
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.
I thought it might be a good idea to remove this since it adds unnecessary steps to get the user up and running with the createEmailSession
flow. Wouldn't leaving the code there cause some confusion for newcomers to Appwrite?
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.
I agree it's unnecessary if you only want to create an email/password flow, but I think it's important to have it here for users who do want to use OAuth; this information is otherwise a bit harder to find. What do you think @gewenyu99?
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.
Bumping - @gewenyu99
class ViewModel: ObservableObject { | ||
@Published var email: String = "" | ||
@Published var password: String = "" | ||
} | ||
|
||
struct ContentView: View { | ||
@ObservedObject var viewModel = ViewModel() | ||
@State var email: String = "" | ||
@State var password: String = "" | ||
@State var session: Session? | ||
|
||
let appwrite = Appwrite() |
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.
Let's keep the ViewModel
, but move the Appwrite
instance into it. We can also add proxy methods for onRegister
and onLogin
in the view model to call the appwrite functions
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.
I can go either way on ViewModel
but I am recommending that we remove it since view models are a debatable topic in the SwiftUI community. I also feel like it adds unnecessary complexity to the docs and would be better suited for tutorials instead. What do you think?
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.
It's good practice to separate the data from the view in any framework, to promote modularity and improve testability. I agree it adds a bit of complexity but we need to strike a balance between simplicity and best practice. In this case, I think it's better to leave it.
Although we could name it better like LoginCredentials
or similar instead of just ViewModel
to make it easier to reason about
What does this PR do?
body
of theContentView
If this PR is accepted, similar changes need to be made to Android and Flutter to maintain parity between the mobile platform quick start pages. I believe these changes will shorten the path to a newcomer becoming familiar with Appwrite and provides a functioning app as a result to following the steps listed.
Test Plan
Visit
docs/quick-starts/apple
in the project or in browser to see changes.Related PRs and Issues
More Issues and PRs will be created as a result of this PR being merged.
Have you read the [Contributing Guidelines on issues]
Yes