Login in page #468
Login in page #468
Conversation
test/FormProviderFixture.jsx
Outdated
| const FormProviderFixture = ({ children }) => { | ||
| const methods = useForm() | ||
|
|
||
| return <FormProvider {...methods}>{children} </FormProvider> |
There was a problem hiding this comment.
nit
| return <FormProvider {...methods}>{children} </FormProvider> | |
| return <FormProvider {...methods}>{children}</FormProvider> |
views/components/Hero/Hero.css
Outdated
| @@ -0,0 +1,20 @@ | |||
| .HeroCard_container { | |||
There was a problem hiding this comment.
IMO the block should be the component name for consistency throughout the app, so either Hero or change the name of the component to HeroCard.
| @@ -1,23 +1,23 @@ | |||
| import React, { useState, useContext } from 'react' | |||
There was a problem hiding this comment.
should these files be nested in a SignInPage directory like the other component directories?
views/layouts/HeroLayout.css
Outdated
| .HeroLayout_container { | ||
| display: flex; | ||
| flex-direction: column; | ||
| height: 897px; |
There was a problem hiding this comment.
why are we specifying a pixel height? Should the layout take up at least the screen height?
views/layouts/HeroLayout.css
Outdated
| align-items: center; | ||
| justify-content: space-around; | ||
| flex-direction: row; | ||
| height: 100%; |
There was a problem hiding this comment.
should this be a minimum height?
| align-items: center; | ||
| } | ||
| .footer_actions { | ||
| width: 1018px; |
There was a problem hiding this comment.
we shouldn't need this.
| const { container } = wrapper | ||
|
|
||
| const Footer = container.firstChild | ||
|
|
||
| expect(Footer).toBeInTheDocument() |
There was a problem hiding this comment.
let's use the screen import instead of defining a wrapper. Also, do we need this test if we have the others?
| test('renders the form', () => { | ||
| const { container } = wrapper | ||
| const form = container.firstChild | ||
|
|
||
| expect(form).toBeInTheDocument() | ||
| }) |
There was a problem hiding this comment.
same comment re: using screen and this test may not be valuable given the others.
views/forms/LoginForm/LoginForm.css
Outdated
| @@ -0,0 +1,7 @@ | |||
| .SignInButton_container { | |||
There was a problem hiding this comment.
consistent block name (LoginForm_submitBtnContainer)
There was a problem hiding this comment.
the block should be the same for all elements in a component. If we extract elements to a new component, they become elements with a block name of the new component name.
views/pages/SignInPage.css
Outdated
| @@ -0,0 +1,29 @@ | |||
| .LoginForm_container { | |||
There was a problem hiding this comment.
consistent block names
Elkrival
force-pushed
the
login-in-page
branch
from
5a47080 to
11a691a
Compare
| width: 565px; | ||
| height: 270px; | ||
| } | ||
| .HeroCard_image { | ||
| height: 106px; | ||
| width: 476px; |
There was a problem hiding this comment.
can we remove these heights and widths? Maybe we need to regroup on this. If these are meant to be components that can in theory be used elsewhere in app then this could really cause us headaches.
There was a problem hiding this comment.
It's not used anywhere else besides this layout. The right container of this layout is the different forms, but otherwise this stays the same.
Elkrival
force-pushed
the
login-in-page
branch
from
3ecc044 to
ae0b517
Compare
Elkrival
force-pushed
the
428-update-material-ui-version-to-the-latest-stable-version
branch
2 times, most recently
from
5dc9d5a to
b125c60
Compare
Elkrival
force-pushed
the
login-in-page
branch
from
ae0b517 to
9930666
Compare
Elkrival
force-pushed
the
login-in-page
branch
from
4ed9b1e to
9f58ff5
Compare
* Login Page * Added Login Page UI designs 3Fixed Test
Elkrival
force-pushed
the
login-in-page
branch
from
9f58ff5 to
13f795d
Compare
|
|
||
| const TextInput = (props) => { | ||
| const { name, control, onChange, ...rest } = props | ||
| const { name, onChange, ...rest } = props | ||
| const { control } = useFormContext() |
There was a problem hiding this comment.
@Elkrival this change broke the admin page when a user logs in. I think we need the control from props.
This pr updates the design for the Login Page, it adds an HeroLayout component that can be used for other pages in this format. It adds a form provider to mock the react hook form provider. The TextInput component now passes it's ref for focus.