Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

fix(onboarding-pass-validation): added password validation #424

Merged
merged 7 commits into from Jun 28, 2018
Merged

fix(onboarding-pass-validation): added password validation #424

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
902da0a
fix(onboarding-pass-validation): users are not allowed to proceed if …
odb366 Jun 15, 2018
4662417
Merge branch 'master' into fix/onboarding-pass-validation
odb366 Jun 25, 2018
d59c339
fix(onboarding-pass-validation): fixed prettier issue
odb366 Jun 25, 2018
d90c3a1
fix(onboarding-pass-validation): fixed prettier issue2
odb366 Jun 25, 2018
c22104d
fix(onboarding-pass-validation): add 8 char inline validation
odb366 Jun 27, 2018
9240c9b
fix(onboarding-pass-validation): update local repo
odb366 Jun 27, 2018
1ef2c96
fix(onboarding-pass-validation): improve password validation
odb366 Jun 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/components/Onboarding/NewWalletPassword.js
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@ const NewWalletPassword = ({
createWalletPassword,
createWalletPasswordConfirmation,
showCreateWalletPasswordConfirmationError,
passwordMinChars,
updateCreateWalletPassword,
updateCreateWalletPasswordConfirmation
}) => (
Expand Down Expand Up @@ -35,6 +36,9 @@ const NewWalletPassword = ({
>
Passwords do not match
</p>
<p className={`${styles.helpMessage} ${passwordMinChars && styles.visible}`}>
Copy link
Member

Choose a reason for hiding this comment

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

this visible class doesn't apply to helpMessage right? It looks like it's nested inside errorMessage in the scss file.

Also this logic seems to apply visible when the passwords match and the char count is 8 or over. Shouldn't it be the other way around?

Copy link
Member

Choose a reason for hiding this comment

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

When I'm testing this PR the error message shows up before I start typing anything

Copy link
Contributor Author

@odb366 odb366 Jun 26, 2018 •
edited

Choose a reason for hiding this comment

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

The help message is intended to always stay visible for better usability. I've just forgotten to clean up the code.
If the help message is hidden some users will try to create passwords with less than 8 chars and then fail. They will be forced to rethink and resubmit their pass which is considerable user effort. We can prevent such cases by leaving the help message always visible. See this article.

The password must be at least 8 characters long
</p>
</section>
</div>
)
Expand All @@ -43,6 +47,7 @@ NewWalletPassword.propTypes = {
createWalletPassword: PropTypes.string.isRequired,
createWalletPasswordConfirmation: PropTypes.string.isRequired,
showCreateWalletPasswordConfirmationError: PropTypes.bool.isRequired,
passwordMinChars: PropTypes.bool.isRequired,
updateCreateWalletPassword: PropTypes.func.isRequired,
updateCreateWalletPasswordConfirmation: PropTypes.func.isRequired
}
Expand Down
7 changes: 7 additions & 0 deletions app/components/Onboarding/NewWalletPassword.scss
View file
Open in desktop
Expand Up @@ -35,3 +35,10 @@
visibility: visible;
}
}

.helpMessage {
Copy link
Member

Choose a reason for hiding this comment

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

currently we highlight the bottom border of the input that isn;t correct and make the border $red. we also make the error text $red. can you make those style changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Working on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we use input field with all borders making just the bottom one red will look strange. I haven't seen this pattern in sites/apps, so in my opinion it will not be consistent in usability point of view. I've made all borders and error text to get $red when error occurs. Is that Ok?

color: white;
opacity: 0.87;
padding-top: 20px;
font-size: 14px;
}
4 changes: 4 additions & 0 deletions app/components/Onboarding/Onboarding.js
View file
Open in desktop
Expand Up @@ -127,6 +127,10 @@ const Onboarding = ({
if (newWalletPasswordProps.showCreateWalletPasswordConfirmationError) {
return
}
// if the password is less than 8 characters don't allow users to proceed
if (!newWalletPasswordProps.passwordMinChars) {
return
}

changeStep(5)
}}
Expand Down
2 changes: 2 additions & 0 deletions app/containers/Root.js
View file
Open in desktop
Expand Up @@ -62,6 +62,7 @@ const mapStateToProps = state => ({

syncPercentage: lndSelectors.syncPercentage(state),
passwordIsValid: onboardingSelectors.passwordIsValid(state),
passwordMinChars: onboardingSelectors.passwordMinChars(state),
showCreateWalletPasswordConfirmationError: onboardingSelectors.showCreateWalletPasswordConfirmationError(
state
),
Expand Down Expand Up @@ -132,6 +133,7 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => {
createWalletPassword: stateProps.onboarding.createWalletPassword,
createWalletPasswordConfirmation: stateProps.onboarding.createWalletPasswordConfirmation,
showCreateWalletPasswordConfirmationError: stateProps.showCreateWalletPasswordConfirmationError,
passwordMinChars: stateProps.passwordMinChars,
updateCreateWalletPassword: dispatchProps.updateCreateWalletPassword,
updateCreateWalletPasswordConfirmation: dispatchProps.updateCreateWalletPasswordConfirmation
}
Expand Down
6 changes: 6 additions & 0 deletions app/reducers/onboarding.js
View file
Open in desktop
Expand Up @@ -292,6 +292,12 @@ onboardingSelectors.passwordIsValid = createSelector(
password => password.length >= 8
)

onboardingSelectors.passwordMinChars = createSelector(
createWalletPasswordSelector,
createWalletPasswordConfirmationSelector,
(pass1, pass2) => pass1 === pass2 && pass1.length >= 8
)

onboardingSelectors.showCreateWalletPasswordConfirmationError = createSelector(
createWalletPasswordSelector,
createWalletPasswordConfirmationSelector,
Expand Down