-
Notifications
You must be signed in to change notification settings - Fork 24
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
22 login page #37
22 login page #37
Conversation
Not sure why it's failing the circleci build with permission denied... |
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.
Good job getting this together. I have some comments and questions
Please review them
src/tests/containers/LogIn.test.js
Outdated
return item.prop('name') === 'password' | ||
}) | ||
|
||
it('should have a LogIn Header', () => { |
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.
Not sure how valuable is this test
src/tests/containers/LogIn.test.js
Outdated
expect(mainHeader).toHaveLength(1) | ||
}) | ||
|
||
it('should have a Form', () => { |
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.
Not sure how valuable is this test
src/tests/containers/LogIn.test.js
Outdated
expect(wrapper.state().password).toBe('password') | ||
}) | ||
|
||
it('should call handleLogIn and postLogInInfo when the form is submitted', async () => { |
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 would like to see the negative test
src/tests/containers/LogIn.test.js
Outdated
expect(wrapper.find('Form')).toHaveLength(1) | ||
}) | ||
|
||
it('should update the state when email input value changes', () => { |
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.
Not sure how valuable is this test
src/tests/containers/LogIn.test.js
Outdated
expect(wrapper.state().email).toBe('existing-user@example.com') | ||
}) | ||
|
||
it('should update the state when password input value changes', () => { |
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.
Not sure how valuable is this test
src/containers/LogIn.js
Outdated
iziToast.show({ | ||
theme: 'light', | ||
title: 'Success', | ||
message: 'Have a look around', |
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 text on the story is:
"Welcome back, Julie"
- returns an object, not an array like other components
* Get basic login to rails api set up * Add styling, izitoast, success, and failure protocol * Set up redux for Login, test * Update endpoints, move test files
* Get basic login to rails api set up * Add styling, izitoast, success, and failure protocol * Set up redux for Login, test * Update endpoints, move test files
Issue addressed
fixes #22
Screenshots (if appropriate):
Testing
have not added integration tests, but the coverage is looking good