From f8c8dc16630f11dfd2e39093a63471f86bfd74f5 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Mon, 12 Aug 2019 17:52:16 -0400 Subject: [PATCH 1/6] add support for positive validation messages --- src/components/formik-forms/formik-input.jsx | 11 ++++++++++- src/components/formik-forms/formik-select.jsx | 1 + src/components/forms/validation-message.jsx | 13 ++++++++++++- src/components/forms/validation-message.scss | 18 ++++++++++++++++-- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/components/formik-forms/formik-input.jsx b/src/components/formik-forms/formik-input.jsx index 26bff6a82..8df63032a 100644 --- a/src/components/formik-forms/formik-input.jsx +++ b/src/components/formik-forms/formik-input.jsx @@ -11,6 +11,7 @@ require('./formik-input.scss'); const FormikInput = ({ className, error, + toolTip, validationClassName, wrapperClassName, ...props @@ -31,11 +32,18 @@ const FormikInput = ({ )} {...props} /> - {error && ( + {error ? ( + ) : toolTip && ( + )} ); @@ -44,6 +52,7 @@ const FormikInput = ({ FormikInput.propTypes = { className: PropTypes.string, error: PropTypes.string, + toolTip: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]), type: PropTypes.string, validationClassName: PropTypes.string, wrapperClassName: PropTypes.string diff --git a/src/components/formik-forms/formik-select.jsx b/src/components/formik-forms/formik-select.jsx index 4578d8a64..2bd1f6ff5 100644 --- a/src/components/formik-forms/formik-select.jsx +++ b/src/components/formik-forms/formik-select.jsx @@ -37,6 +37,7 @@ const FormikSelect = ({ {error && ( diff --git a/src/components/forms/validation-message.jsx b/src/components/forms/validation-message.jsx index 10c05ce13..0664995fb 100644 --- a/src/components/forms/validation-message.jsx +++ b/src/components/forms/validation-message.jsx @@ -5,13 +5,24 @@ const React = require('react'); require('./validation-message.scss'); const ValidationMessage = props => ( -
+
{props.message}
); ValidationMessage.propTypes = { className: PropTypes.string, + isError: PropTypes.bool, + isOk: PropTypes.bool, message: PropTypes.string }; diff --git a/src/components/forms/validation-message.scss b/src/components/forms/validation-message.scss index d10e30e27..7a2acbd84 100644 --- a/src/components/forms/validation-message.scss +++ b/src/components/forms/validation-message.scss @@ -11,7 +11,6 @@ margin-left: $arrow-border-width; border: 1px solid $active-gray; border-radius: 5px; - background-color: $ui-orange; padding: 1rem; max-width: 18.75rem; min-height: 1rem; @@ -31,7 +30,6 @@ border-left: 1px solid $active-gray; border-radius: 5px; - background-color: $ui-orange; width: $arrow-border-width; height: $arrow-border-width; @@ -52,3 +50,19 @@ } } } + +.validation-error { + background-color: $ui-orange; + + &:before { + background-color: $ui-orange; + } +} + +.validation-ok { + background-color: $ui-blue; + + &:before { + background-color: $ui-blue; + } +} From 095493a313db01d2cbfeff99c52888b838c49cea Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Mon, 12 Aug 2019 17:53:09 -0400 Subject: [PATCH 2/6] add advice tooltip strings for username step --- src/l10n.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/l10n.json b/src/l10n.json index 92393beb0..70efa1cdd 100644 --- a/src/l10n.json +++ b/src/l10n.json @@ -180,6 +180,8 @@ "registration.nextStep": "Next Step", "registration.notYou": "Not you? Log in as another user", "registration.optIn": "Send me updates on using Scratch in educational settings", + "registration.passwordAdviceShort": "Write it down so you remember. Don’t share it with others!", + "registration.passwordConfirmAdviceShort": "Type password again", "registration.personalStepTitle": "Personal Information", "registration.personalStepDescription": "Your individual responses will not be displayed publicly, and will be kept confidential and secure", "registration.private": "Scratch will always keep this information private.", @@ -190,6 +192,7 @@ "registration.usernameStepDescription": "Fill in the following forms to request an account. The approval process may take up to one day.", "registration.usernameStepDescriptionNonEducator": "Create projects, share ideas, make friends. It’s free!", "registration.usernameStepRealName": "Please do not use any portion of your real name in your username.", + "registration.usernameAdviceShort": "Don't use your real name", "registration.studentUsernameStepDescription": "You can make games, animations, and stories using Scratch. Setting up an account is easy and it's free. Fill in the form below to get started.", "registration.studentUsernameStepHelpText": "Already have a Scratch account?", "registration.studentUsernameStepTooltip": "You'll need to create a new Scratch account to join this class.", From 0998171a6758069cba1b554212a4808ecd3275ab Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Mon, 12 Aug 2019 17:53:27 -0400 Subject: [PATCH 3/6] display username tooltip messages --- src/components/join-flow/username-step.jsx | 32 ++++++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/components/join-flow/username-step.jsx b/src/components/join-flow/username-step.jsx index 5e56440b8..b701ac825 100644 --- a/src/components/join-flow/username-step.jsx +++ b/src/components/join-flow/username-step.jsx @@ -21,16 +21,26 @@ class UsernameStep extends React.Component { super(props); bindAll(this, [ 'handleChangeShowPassword', + 'handleFocused', 'handleValidSubmit', 'validatePasswordIfPresent', 'validatePasswordConfirmIfPresent', 'validateUsernameIfPresent', 'validateForm' ]); + this.state = { + focused: null, + showPassword: false + }; } handleChangeShowPassword () { this.setState({showPassword: !this.state.showPassword}); } + // track the currently focused input field, to determine whether each field should + // display a tooltip. (We only display it if a field is focused and has never been touched.) + handleFocused (fieldName) { + this.setState({focused: fieldName}); + } // we allow username to be empty on blur, since you might not have typed anything yet validateUsernameIfPresent (username) { if (!username) return null; // skip validation if username is blank; null indicates valid @@ -109,7 +119,9 @@ class UsernameStep extends React.Component { handleSubmit, isSubmitting, setFieldError, + setFieldTouched, setFieldValue, + touched, validateField, values } = props; @@ -135,14 +147,18 @@ class UsernameStep extends React.Component { id="username" name="username" placeholder={this.props.intl.formatMessage({id: 'general.username'})} + toolTip={this.state.focused === 'username' && !touched.username && + this.props.intl.formatMessage({id: 'registration.usernameAdviceShort'})} validate={this.validateUsernameIfPresent} validationClassName="validation-full-width-input" /* eslint-disable react/jsx-no-bind */ onBlur={() => validateField('username')} onChange={e => { setFieldValue('username', e.target.value); + setFieldTouched('username'); setFieldError('username', null); }} + onFocus={() => this.handleFocused('username')} /* eslint-enable react/jsx-no-bind */ />
@@ -157,6 +173,8 @@ class UsernameStep extends React.Component { id="password" name="password" placeholder={this.props.intl.formatMessage({id: 'general.password'})} + toolTip={this.state.focused === 'password' && !touched.password && + this.props.intl.formatMessage({id: 'registration.passwordAdviceShort'})} type={values.showPassword ? 'text' : 'password'} /* eslint-disable react/jsx-no-bind */ validate={password => this.validatePasswordIfPresent(password, values.username)} @@ -164,8 +182,10 @@ class UsernameStep extends React.Component { onBlur={() => validateField('password')} onChange={e => { setFieldValue('password', e.target.value); + setFieldTouched('password'); setFieldError('password', null); }} + onFocus={() => this.handleFocused('password')} /* eslint-enable react/jsx-no-bind */ /> @@ -187,13 +213,13 @@ class UsernameStep extends React.Component { values.passwordConfirm) } validationClassName="validation-full-width-input" - onBlur={() => - validateField('passwordConfirm') - } + onBlur={() => validateField('passwordConfirm')} onChange={e => { setFieldValue('passwordConfirm', e.target.value); + setFieldTouched('passwordConfirm'); setFieldError('passwordConfirm', null); }} + onFocus={() => this.handleFocused('passwordConfirm')} /* eslint-enable react/jsx-no-bind */ />
From ff2cab476dde3c61a794b5114ab04b69a48bd129 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Fri, 16 Aug 2019 17:58:10 -0400 Subject: [PATCH 4/6] fixed proptypes bug, changed validationmessage to use mode --- src/components/formik-forms/formik-input.jsx | 6 +++--- src/components/formik-forms/formik-select.jsx | 2 +- src/components/forms/validation-message.jsx | 9 ++++----- src/components/forms/validation-message.scss | 2 +- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/components/formik-forms/formik-input.jsx b/src/components/formik-forms/formik-input.jsx index 8df63032a..908fc8890 100644 --- a/src/components/formik-forms/formik-input.jsx +++ b/src/components/formik-forms/formik-input.jsx @@ -34,15 +34,15 @@ const FormikInput = ({ /> {error ? ( ) : toolTip && ( )}
@@ -52,7 +52,7 @@ const FormikInput = ({ FormikInput.propTypes = { className: PropTypes.string, error: PropTypes.string, - toolTip: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]), + toolTip: PropTypes.string, type: PropTypes.string, validationClassName: PropTypes.string, wrapperClassName: PropTypes.string diff --git a/src/components/formik-forms/formik-select.jsx b/src/components/formik-forms/formik-select.jsx index 2bd1f6ff5..548e335e9 100644 --- a/src/components/formik-forms/formik-select.jsx +++ b/src/components/formik-forms/formik-select.jsx @@ -37,9 +37,9 @@ const FormikSelect = ({ {error && ( )}
diff --git a/src/components/forms/validation-message.jsx b/src/components/forms/validation-message.jsx index 0664995fb..717128e39 100644 --- a/src/components/forms/validation-message.jsx +++ b/src/components/forms/validation-message.jsx @@ -9,8 +9,8 @@ const ValidationMessage = props => ( className={classNames( 'validation-message', { - 'validation-error': props.isError, - 'validation-ok': props.isOk + 'validation-error': props.mode === 'error', + 'validation-info': props.mode === 'info' }, props.className )} @@ -21,9 +21,8 @@ const ValidationMessage = props => ( ValidationMessage.propTypes = { className: PropTypes.string, - isError: PropTypes.bool, - isOk: PropTypes.bool, - message: PropTypes.string + message: PropTypes.string, + mode: PropTypes.string }; module.exports = ValidationMessage; diff --git a/src/components/forms/validation-message.scss b/src/components/forms/validation-message.scss index 7a2acbd84..acb19820a 100644 --- a/src/components/forms/validation-message.scss +++ b/src/components/forms/validation-message.scss @@ -59,7 +59,7 @@ } } -.validation-ok { +.validation-info { background-color: $ui-blue; &:before { From 4f0a87e25b10407d505a050785a584632b37a879 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 20 Aug 2019 01:20:19 +0200 Subject: [PATCH 5/6] removed duplicate confirm password string --- src/components/join-flow/username-step.jsx | 2 +- src/l10n.json | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/join-flow/username-step.jsx b/src/components/join-flow/username-step.jsx index b701ac825..f588360cb 100644 --- a/src/components/join-flow/username-step.jsx +++ b/src/components/join-flow/username-step.jsx @@ -203,7 +203,7 @@ class UsernameStep extends React.Component { toolTip={ this.state.focused === 'passwordConfirm' && !touched.passwordConfirm && this.props.intl.formatMessage({ - id: 'registration.passwordConfirmAdviceShort' + id: 'registration.confirmPasswordInstruction' }) } type={values.showPassword ? 'text' : 'password'} diff --git a/src/l10n.json b/src/l10n.json index 70efa1cdd..e161a64e1 100644 --- a/src/l10n.json +++ b/src/l10n.json @@ -181,7 +181,6 @@ "registration.notYou": "Not you? Log in as another user", "registration.optIn": "Send me updates on using Scratch in educational settings", "registration.passwordAdviceShort": "Write it down so you remember. Don’t share it with others!", - "registration.passwordConfirmAdviceShort": "Type password again", "registration.personalStepTitle": "Personal Information", "registration.personalStepDescription": "Your individual responses will not be displayed publicly, and will be kept confidential and secure", "registration.private": "Scratch will always keep this information private.", From 31b1d300f6a9a1107f3fccfe185cc23eb0327f7a Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 20 Aug 2019 20:25:44 +0200 Subject: [PATCH 6/6] added tests of revised validation message --- test/unit/components/formik-input.test.jsx | 45 +++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/test/unit/components/formik-input.test.jsx b/test/unit/components/formik-input.test.jsx index 0a59f428c..9d4585c69 100644 --- a/test/unit/components/formik-input.test.jsx +++ b/test/unit/components/formik-input.test.jsx @@ -4,15 +4,29 @@ import FormikInput from '../../../src/components/formik-forms/formik-input.jsx'; import {Formik} from 'formik'; describe('FormikInput', () => { - test('No validation message without an error', () => { + test('No validation message without an error or a tooltip', () => { const component = mountWithIntl( ); expect(component.find('ValidationMessage').exists()).toEqual(false); + expect(component.find('div.validation-error').exists()).toEqual(false); + expect(component.find('div.validation-info').exists()).toEqual(false); + }); + + test('No validation message with blank error or tooltip', () => { + const component = mountWithIntl( + + + + ); + expect(component.find('ValidationMessage').exists()).toEqual(false); + expect(component.find('div.validation-error').exists()).toEqual(false); + expect(component.find('div.validation-info').exists()).toEqual(false); }); test('Validation message shown when error given', () => { @@ -24,5 +38,34 @@ describe('FormikInput', () => { ); expect(component.find('ValidationMessage').exists()).toEqual(true); + expect(component.find('div.validation-error').exists()).toEqual(true); + expect(component.find('div.validation-info').exists()).toEqual(false); + }); + + test('Tooltip shown when tooltip given', () => { + const component = mountWithIntl( + + + + ); + expect(component.find('ValidationMessage').exists()).toEqual(true); + expect(component.find('div.validation-error').exists()).toEqual(false); + expect(component.find('div.validation-info').exists()).toEqual(true); + }); + + test('If both error and tooltip messages, error takes precedence', () => { + const component = mountWithIntl( + + + + ); + expect(component.find('ValidationMessage').exists()).toEqual(true); + expect(component.find('div.validation-error').exists()).toEqual(true); + expect(component.find('div.validation-info').exists()).toEqual(false); }); });