-
Notifications
You must be signed in to change notification settings - Fork 297
Update Apple Quick Start Docs #560
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
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
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.
Agreeing with @abnegate here on retaining ViewModel logic
|
@Kilo-Loco I am closing this PR since there has been no activity here in over a year. If you would like to make any new changes, please create an issue first and submit a PR once that issue is assigned to you. For this PR, it may make a lot more sense to first discuss all opinions in an Issue before we jump to any changes since we have some differences of perspective, in case you'd like to open this up again. |
What does this PR do?
bodyof theContentViewIf 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/applein 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