Skip to content

Geoff signup validation #277

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

Open
wants to merge 18 commits into
base: development
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
},
"scripts": {
"start": "react-scripts start",
"build": "react-scripts build",
"build": "CI= react-scripts build",
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this needed? I changed this while testing out netlify.

Copy link
Member

Choose a reason for hiding this comment

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

what does this line do?

Copy link
Member Author

Choose a reason for hiding this comment

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

When I was originally trying to get netlify working, I found this fix. Apparently it prevents warnings from creating a fatal error in the build process. This link explains it some.

"test": "react-scripts test",
"eject": "react-scripts eject",
"predeploy": "npm run build",
Expand Down
3 changes: 3 additions & 0 deletions public/red-x.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 11 additions & 0 deletions src/api/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,14 @@ export const changePassword = (passwords, user) => {
}
})
}

// Check availability of email or username in API
export const checkInfo = (data, type) => {
return axios({
url: apiUrl + `/check${type}`,
method: 'GET',
params: {
[`${type}`]: data
}
})
}
142 changes: 130 additions & 12 deletions src/components/HeaderAuthOptions/HeaderSignUp/HeaderSignUp.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import React, { Component } from 'react'
import { withRouter } from 'react-router-dom'
import { signUp, signIn } from '../../../api/auth'

import { signUp, signIn, checkInfo } from '../../../api/auth'
import messages from '../../AutoAlert/messages'
import signUpMessages from '../../SignUp/signUpMessages'

import * as validations from '../../../helpers/signUpValidation'

import Form from 'react-bootstrap/Form'
import Button from 'react-bootstrap/Button'
Expand All @@ -15,20 +19,68 @@ class HeaderSignUp extends Component {

this.state = {
email: '',
emailAvail: false,
emailValid: false,
emailVal: false,
username: '',
usernameVal: false,
usernameLength: false,
usernameAvail: false,
identifier: '',
submit: false,
password: '',
passwordConfirmation: ''
passwordVal: false,
passwordLength: false,
passwordCapital: false,
passwordLower: false,
passwordSpecial: false,
passwordNumber: false,
passwordConfirmation: '',
passwordConfirmationVal: false,
property: document.documentElement.style.setProperty('--border-show', 'none')
Copy link
Member

Choose a reason for hiding this comment

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

what is document.documentElement.style.setProperty('--border-show', 'none')?

Copy link
Member Author

Choose a reason for hiding this comment

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

I meant to comment on this. I'm using a CSS variable to change the border from 'none' to 'show' upon submit. This line resets the border display to 'none' when first loading the signup page. Without it, if you left the signup page and came back the borders would still be showing. We probably won't need the CSS variable when we implement the newer wire frames, but I wanted to leave it in for now... just in case.

Copy link
Member

Choose a reason for hiding this comment

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

If you want this code to execute on load, why don't we add this to componentDidMount()

Do we need to save property to state?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm setting state on line 83
this.setState({ property: document.documentElement.style.setProperty('--border-show', 'solid') })
Do you think it would still be a better idea to use componentDidMount()?

I was planning on addressing this issue when tackling the new wireframes. I'm not 100% sure we'll need this functionality at all. Most likely, we can just remove it.

}
}
// Set state of all fields using helper functions
Copy link
Member Author

Choose a reason for hiding this comment

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

I was looking into a cleaner way to do this. The checkValid() function checks the state of all the fields at once. I'm looking into just checking the specific fields that are being altered. Maybe a helper function? Or possibly a naming convention that would allow for easier refactoring?

checkValid = () => {
this.setState({ emailVal: validations.emailTest(this.state.email, this.state.emailAvail) })
this.setState({ emailValid: validations.emailValid(this.state.email) })
this.setState({ usernameVal: validations.usernameTest(this.state.username, this.state.usernameAvail) })
this.setState({ usernameLength: validations.usernameLength(this.state.username) })
this.setState({ passwordVal: validations.passwordTest(this.state.password) })
this.setState({ passwordLength: validations.passwordLength(this.state.password) })
this.setState({ passwordCapital: validations.passwordCapital(this.state.password) })
this.setState({ passwordLower: validations.passwordLower(this.state.password) })
this.setState({ passwordSpecial: validations.passwordSpecial(this.state.password) })
this.setState({ passwordNumber: validations.passwordNumber(this.state.password) })
this.setState({ passwordConfirmationVal: validations.passwordConfirmationTest(this.state.password, this.state.passwordConfirmation) })
}
// Testing a refactor
// checkApi = (name, avail) => {
// this.setState({ [`${name}Val`]: validations[`${name}Test`](this.state[name], this.state[avail]) })
// this.setState({ [`${name}Valid`]: validations[`${name}Valid`](this.state[name]) })
// this.setState({ [`${name}Length`]: validations[`${name}Length`](this.state[name]) })
// }

handleChange = event => this.setState({
[event.target.name]: event.target.value
})
handleChange = event => {
Copy link
Member

Choose a reason for hiding this comment

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

This code is difficult to follow and somewhat repetitive. How can we refactor? What are we trying to accomplish with this function?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted the ability to check username and email while the user is typing. I only need to do this on two of the four form fields. The other two form fields still need the checkValid function. I went with the conditional to get this to work, but I would love to clean it up some.

The two api calls for checkname() and checkemail() are what drove me to do it this way originally. I may be able to rename the functions and condense down one of the conditionals, but I'll have to play with some ideas. I'm open to suggestions.

Copy link
Member Author

Choose a reason for hiding this comment

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

I refactored the api calls so we can send two arguments. This allowed for the refactoring of handleChange a bit.

Maybe we can still do something with () => { this.checkValid()? I was toying with the idea of placing it once at the end of the function, but I'm having trouble thinking of a way to call the api only when necessary.

const name = event.target.name

this.setState({[event.target.name]: event.target.value},
(name === 'username' || name === 'email') ?
() => checkInfo(this.state[name], name)
.then(res => this.setState({ [`${name}Avail`]: res.data }))
.then(() => { this.checkValid() })
.catch(error => console.error()) : () => { this.checkValid() })
}

onSignUp = event => {
event.preventDefault()
this.setState({ identifier: this.state.email })
this.setState({
identifier: this.state.email,
submit: true })
// Check if fields are valid
this.checkValid()
// Set border property to show red or green border
this.setState({ property: document.documentElement.style.setProperty('--border-show', 'solid') })
const { alert, history, setUser } = this.props

signUp(this.state)
Expand All @@ -43,7 +95,6 @@ class HeaderSignUp extends Component {
.then(() => history.push('/first-signin'))
.catch(error => {
console.error(error)
this.setState({ email: '', username: '', password: '', passwordConfirmation: '' })
Copy link
Member Author

Choose a reason for hiding this comment

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

This cleared the form after an error. I removed it so the user has an opportunity to alter their inputs.

alert({
heading: 'Sign Up Failed',
message: messages.signUpFailure,
Expand All @@ -52,8 +103,31 @@ class HeaderSignUp extends Component {
})
}

// Hover effect opens error message on web browser.
// Needs to be tested on mobile
onHover = (prevState, state) => {
this.setState({ [`${state}`]: !prevState })
}

render () {
const { email, username, password, passwordConfirmation } = this.state
const {
email,
openEmail,
openUser,
openPass,
emailAvail,
emailValid,
username,
usernameLength,
password,
passwordConfirmation,
submit,
emailVal,
usernameVal,
usernameAvail,
passwordVal,
passwordConfirmationVal
} = this.state

return (
<div className="header-signup-container">
Expand All @@ -62,7 +136,7 @@ class HeaderSignUp extends Component {
<Form.Group controlId="username" className="mt-4">
<TextField
fullWidth={true}
className="account-info"
className={!usernameVal ? 'account-info-signup-red username input' : 'account-info-signup username input'}
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm using the class here to change the color of the form border. Red for invalid and Green for Valid. This will most likely be unnecessary when we implement the new figma drawings.

required
type="username"
name="username"
Expand All @@ -74,11 +148,22 @@ class HeaderSignUp extends Component {
"aria-label": "Username",
}}
/>
{submit && !usernameVal && <div className='image-div'><img
src='red-x.svg'
alt='red-x'
className='red-x'
onMouseEnter={!usernameVal ? () => this.onHover(openUser, 'openUser') : undefined}
onMouseLeave={!usernameVal ? () => this.onHover(openUser, 'openUser') : undefined}
/></div>}
{openUser && <div className='error-message-div'>
<div>{submit && !usernameVal && !usernameLength && signUpMessages.username}</div>
<div>{submit && !usernameVal && usernameAvail && signUpMessages.usernameAvail}</div>
</div>}
</Form.Group>
<Form.Group controlId="email">
<TextField
fullWidth={true}
className="account-info"
className={!emailVal ? 'account-info-signup-red email input' : 'account-info-signup email input'}
required
type="email"
name="email"
Expand All @@ -90,28 +175,61 @@ class HeaderSignUp extends Component {
"aria-label": "Email",
}}
/>
{submit && !emailVal && <div className='image-div'><img
src='red-x.svg'
alt='red-x'
className='red-x'
onMouseEnter={!emailVal ? () => this.onHover(openEmail, 'openEmail') : undefined}
onMouseLeave={!emailVal ? () => this.onHover(openEmail, 'openEmail') : undefined}
/></div>}
{openEmail && <div className='error-message-div'>
<div>{submit && !emailVal && !emailValid && signUpMessages.email}</div>
<div>{submit && !emailVal && emailAvail && signUpMessages.emailAvail}</div>
</div>}
</Form.Group>
<Form.Group controlId="password">
<PasswordInput
fullWidth={true}
className="account-info password"
className={submit && !passwordVal ? 'account-info-signup-red password input' : 'account-info-signup password input'}
required
name="password"
value={password}
placeholder="Password"
onChange={this.handleChange}
/>
{submit && !passwordVal && <div className='image-div'><img
src='red-x.svg'
alt='red-x'
className='red-x'
onMouseEnter={!passwordVal ? () => this.onHover(openPass, 'openPass') : undefined}
onMouseLeave={!passwordVal ? () => this.onHover(openPass, 'openPass') : undefined}
/></div>}
{openPass && <div className='error-message-div'>
<div>{submit && !passwordVal && !this.state.passwordLength && signUpMessages.passwordLength}</div>
<div>{submit && !passwordVal && !this.state.passwordCapital && signUpMessages.passwordCapital}</div>
<div>{submit && !passwordVal && !this.state.passwordSpecial && signUpMessages.passwordSpecial}</div>
<div>{submit && !passwordVal && !this.state.passwordLower && signUpMessages.passwordLower}</div>
<div>{submit && !passwordVal && !this.state.passwordNumber && signUpMessages.passwordNumber}</div>
</div>}
</Form.Group>
<Form.Group controlId="passwordConfirmation">
<PasswordInput
fullWidth={true}
className="account-info password"
className={!passwordConfirmationVal ? 'account-info-signup-red input' : 'account-info-signup input'}
required
name="passwordConfirmation"
value={passwordConfirmation}
placeholder="Confirm Password"
onChange={this.handleChange}
/>
{submit && !passwordConfirmationVal && <div className='image-div'><img
src='red-x.svg'
alt='red-x'
className='red-x'
/></div>}
<Form.Text className={!passwordConfirmationVal ? 'is-invalid' : 'is-valid'}>
{submit && !passwordConfirmationVal && signUpMessages.passwordConfirmation }
</Form.Text>
</Form.Group>
<Button
variant="primary"
Expand Down
Loading