-
Notifications
You must be signed in to change notification settings - Fork 2.2k
docs(): authentication tutorial with ionic and rxjs compact for angularfire2 ^rc.10 #1721
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
Conversation
authentication with ionic and firebase
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.
@hiepxanh Thank you so much for sending in this PR!
I have a few comments around formatting, capitalization, and small typos. Nothing major.
Don't worry about the number of comments. We don't have a formatting guide for these kind of docs so this was bound to happen.
Don't hesitate to ask me if you need clarity from a comment of mine.
docs/ionic/authentication.md
Outdated
@@ -0,0 +1,94 @@ | |||
# STEP 1: install this plugin: |
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.
Use capitalization instead of uppercase for the word "step". I find that when something is placed in uppercase by itself too many times it can come off a bit aggressive.
docs/ionic/authentication.md
Outdated
@@ -0,0 +1,94 @@ | |||
# STEP 1: install this plugin: | |||
``` | |||
1) ionic cordova plugin add cordova-universal-links-plugin |
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.
Drop the numbers so it's copy and paste-able.
docs/ionic/authentication.md
Outdated
|
||
# STEP 2: Add Firebase to your Ionic | ||
|
||
STEP 2.1: [setup firebase to angular project] |
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.
Instead of sub numbering the steps just place the action in bold.
Setup Firebase to the Angular Project
docs/ionic/authentication.md
Outdated
|
||
# STEP 2: Add Firebase to your Ionic | ||
|
||
STEP 2.1: [setup firebase to angular project] |
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.
Link formatting is broken here
docs/ionic/authentication.md
Outdated
STEP 2.1: [setup firebase to angular project] | ||
(https://github.com/angular/angularfire2/blob/master/docs/install-and-setup.md) | ||
|
||
STEP 2.2: To set up an Android app, go to [Firebase Console](https://console.firebase.google.com/) then |
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.
Nitpick: Small typo "To set up in an Android app, go to..."
docs/ionic/authentication.md
Outdated
return this.oauthSignIn(new firebase.auth.GoogleAuthProvider()); | ||
} | ||
|
||
private oauthSignIn(provider: AuthProvider) { |
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.
The code formatting is off in this sample. Try pasting it into an editor to clean it up for you and then paste it back in here.
docs/ionic/authentication.md
Outdated
} | ||
``` | ||
|
||
# Problem 1: |
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 really like that you have a common problems section. Try using that title as the header:
Common problems
docs/ionic/authentication.md
Outdated
|
||
# Problem 1: | ||
|
||
if you got error when build code like this: |
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.
Try: "If you received an error like this during a build"
docs/ionic/authentication.md
Outdated
# Problem 1: | ||
|
||
if you got error when build code like this: | ||
`UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'manifest' of undefined` |
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.
Use a three backtick code block here:
UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'manifest' of undefined
docs/ionic/authentication.md
Outdated
if you got error when build code like this: | ||
`UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot read property 'manifest' of undefined` | ||
|
||
Please, using this fix from [issue](https://github.com/nordnet/cordova-universal-links-plugin/issues/134): |
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.
Try: "Use this fix from..."
Yes sir! thank you for showing my fault. I'm on my way |
No faults! We don't have clear guides on formats so there's going to be a bit of back and forth to get it into shape. You're doing great and we're really lucky to have docs contributed like this! |
docs/ionic/authentication.md
Outdated
STEP 3.1: In the Firebase console, open the **Dynamic Links** section at bottom left panel, setup by they instruction | ||
|
||
STEP 3.2: add this to config.xml at root level of project: | ||
``` |
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.
Add syntax highlighting perhaps?
```xml
```
docs/ionic/authentication.md
Outdated
|
||
# STEP 4: add login code: | ||
at login.service.ts add this function: | ||
``` |
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.
Syntax highlighting here as well
```ts
```
thank you @davideast for guiding me to present the article better
thank you @Chan4077 @davideast for guiding me to present the article better. �@davideast I was so pleased to receive your encouragement. Would you mind to help me to check my new update ? |
@davideast I already updated sir, can you check on my commit? |
Thanks @hiepxanh! |
you're welcome. I'm happy to help |
authentication with ionic and firebase
Checklist
yarn install
,yarn test
run successfully? yesDescription
Updating our documentation on Ionic Authentication. it work with google and facebook