Skip to content

Login functionality#97

Merged
khoadnguyen merged 9 commits intodevfrom
login-functionality
Oct 9, 2018
Merged

Login functionality#97
khoadnguyen merged 9 commits intodevfrom
login-functionality

Conversation

@awstin17
Copy link
Copy Markdown
Collaborator

@awstin17 awstin17 commented Oct 7, 2018

Added code to login to app, added icons, added validators

@awstin17 awstin17 added the code review This PR is ready for code review label Oct 7, 2018
@awstin17 awstin17 added this to the Week 4 milestone Oct 7, 2018
@awstin17 awstin17 self-assigned this Oct 7, 2018
@awstin17 awstin17 requested a review from khoadnguyen October 7, 2018 05:33
font-size: 25px;
}

#email:before {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, you got it working, can you explain to the rest of the class how this works and what you used?

private loginCreds : FormGroup;

constructor(public navCtrl: NavController, public navParams: NavParams, public _userService: UserProvider, private formBuilder: FormBuilder) {
this.loginCreds = this.formBuilder.group({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like that this is cleaner, querying the value object. What does it return?

constructor(public navCtrl: NavController, public navParams: NavParams) {
private loginCreds : FormGroup;

constructor(public navCtrl: NavController, public navParams: NavParams, public _userService: UserProvider, private formBuilder: FormBuilder) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Coordinate with @digitalkyopo on naming conventions for _user/_userService. I think you and him named it different things.

}

toDashboard() {
this.navCtrl.setRoot(DashboardPage);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, you used this correctly. During code review, lets remind people diff b/w setRoot and push

@khoadnguyen khoadnguyen merged commit b8a61b5 into dev Oct 9, 2018
@khoadnguyen khoadnguyen deleted the login-functionality branch October 31, 2018 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code review This PR is ready for code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants