Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: Change password hint color to red when invalid, else grey #5130

Merged
merged 26 commits into from
Nov 22, 2021
Merged

fix: Change password hint color to red when invalid, else grey #5130

Changes from 2 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
254e1d3
fix(password-hint-color): Change password hint color to red when it i…
mananjadhav Sep 8, 2021
5a3e33b
fix(password-hint-color): Changed function flow from isInvalidPasswo…
mananjadhav Sep 9, 2021
d33a958
Merge branch 'main' of https://github.com/mananjadhav/App into fix/pa…
mananjadhav Sep 10, 2021
25e16ad
style(password-hint-color): Fixed style changes
mananjadhav Sep 10, 2021
821a6b1
style(password-hint-color): Moved style prop to newline
mananjadhav Sep 10, 2021
226a6cd
Merge branch 'main' of https://github.com/mananjadhav/App into fix/pa…
mananjadhav Sep 12, 2021
8a8e16b
fix(password-hint-color): Changed logic from onBlur to onClick
mananjadhav Sep 13, 2021
52501c6
Merge branch 'main' of https://github.com/mananjadhav/App into fix/pa…
mananjadhav Sep 26, 2021
5cfca27
fix(password-onsubmit-error): Changed all errors to onSubmit
mananjadhav Sep 28, 2021
7e90a49
fix(password-onsubmit-error): Clear error when user starts typing
mananjadhav Sep 28, 2021
835794e
fix(password-hint-color): Added jsdoc comments
mananjadhav Oct 18, 2021
bb49ca2
Merge branch 'main' of https://github.com/mananjadhav/App into fix/pa…
mananjadhav Oct 18, 2021
c9f7c97
fix(password-hint-color): Moved isValidPassword to utils and removed …
mananjadhav Oct 18, 2021
4c4bb9e
fix(password-hint-color): Split functions validate and submit
mananjadhav Oct 18, 2021
b76536d
fix(password-hint-color): Updated translations for password errors
mananjadhav Oct 19, 2021
a167491
fix(password-hint-color): Split errors into different flags and updat…
mananjadhav Oct 19, 2021
5d1648c
fix(password-hint-color): Show error only for one of the flags
mananjadhav Oct 19, 2021
097c007
fix(password-hint-color): Show API error only if no UI error
mananjadhav Oct 19, 2021
dfc570a
fix(password-hint-color): Show errors and hint based condition
mananjadhav Nov 2, 2021
73db2f5
Merge branch 'main' of https://github.com/mananjadhav/App into fix/pa…
mananjadhav Nov 2, 2021
c6b2766
fix(password-hinit-color): Fixed lint issues and minor typos
mananjadhav Nov 4, 2021
e9399f3
Merge branch 'main' of https://github.com/mananjadhav/App into fix/pa…
mananjadhav Nov 4, 2021
8d97c12
fix(password-hint-color): Removed unwanted comments and renamed params
mananjadhav Nov 7, 2021
d3c5afc
fix(password-hint-color): Updated conditions and moved new password p…
mananjadhav Nov 7, 2021
7be90c8
fix(password-hinit-color): Changed _.some to _.every for better reada…
mananjadhav Nov 7, 2021
c94d639
fix(password-hint-color): Changed redirect from Settings to back
mananjadhav Nov 19, 2021
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
21 changes: 9 additions & 12 deletions src/pages/settings/PasswordPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ class PasswordPage extends Component {
currentPassword: '',
newPassword: '',
confirmNewPassword: '',
isPasswordRequirementsVisible: false,
shouldShowPasswordConfirmError: false,
};

Expand All @@ -63,11 +62,6 @@ class PasswordPage extends Component {

onBlurNewPassword() {
const stateToUpdate = {};
if (!this.state.newPassword || !this.isValidPassword()) {
stateToUpdate.isPasswordRequirementsVisible = true;
} else {
stateToUpdate.isPasswordRequirementsVisible = false;
}

if (this.state.newPassword && this.state.confirmNewPassword && !this.doPasswordsMatch()) {
stateToUpdate.shouldShowPasswordConfirmError = true;
Expand Down Expand Up @@ -143,14 +137,17 @@ class PasswordPage extends Component {
textContentType="password"
value={this.state.newPassword}
onChangeText={newPassword => this.setState({newPassword})}
onFocus={() => this.setState({isPasswordRequirementsVisible: true})}
onBlur={() => this.onBlurNewPassword()}
/>
{this.state.isPasswordRequirementsVisible && (
<Text style={[styles.textLabelSupporting, styles.mt1]}>
{this.props.translate('passwordPage.newPasswordPrompt')}
</Text>
)}

<Text style={[
styles.textLabelSupporting, styles.mt1,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put each of these on their own line?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move the style prop to its own line and indent accordingly? There's an example in the // Good section of https://github.com/Expensify/App/blob/main/STYLING.md#when-to-create-a-new-style

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay. Thanks for the share. I wasn't sure if every style will be moved to a new line. Do you think this should be added as a lint config?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds like a good improvement, I think it's worth bringing up in slack to at least get a discussion going, but we'll keep that separate from this PR.

this.state.newPassword && !this.isValidPassword() && styles.formError,
]}
>
{this.props.translate('passwordPage.newPasswordPrompt')}
</Text>

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove spaces between elements

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

</View>
<View style={styles.mb6}>
<ExpensiTextInput
Expand Down