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

Update Apple Quick Start Docs #560

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Kilo-Loco
Copy link
Contributor

What does this PR do?

  • Fixes 📚 Documentation: Apple Quick-Start Snippet Syntax Error #559 by creating an instance of an "Appwrite object" which can be referenced inside the body of the ContentView
  • Removes references to OAuth since the current steps don't implement OAuth and it is not an ideal flow for a newcomer to Appwrite
  • Updates the functionality of the final app to switch between screens based on session status.

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

Comment on lines -59 to -88
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)
}
```
Copy link
Member

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

Copy link
Contributor Author

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?

Copy link
Member

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bumping - @gewenyu99

Comment on lines -156 to +135
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()
Copy link
Member

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

Copy link
Contributor Author

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?

Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📚 Documentation: Apple Quick-Start Snippet Syntax Error
3 participants