From 08a8bc0e4f8626d1d71e452ab2c63364b022c441 Mon Sep 17 00:00:00 2001 From: Shafqat Farhan Date: Mon, 12 Sep 2022 13:56:23 +0500 Subject: [PATCH] fix: VAN-1047 - Handled country field's errors and state persistence (#640) --- src/register/CountryDropdown.jsx | 146 +++++++++---------- src/register/RegistrationPage.jsx | 81 +++++----- src/register/data/constants.js | 3 + src/register/tests/RegistrationPage.test.jsx | 44 ++++-- 4 files changed, 141 insertions(+), 133 deletions(-) diff --git a/src/register/CountryDropdown.jsx b/src/register/CountryDropdown.jsx index 4c4b5112..71049897 100644 --- a/src/register/CountryDropdown.jsx +++ b/src/register/CountryDropdown.jsx @@ -6,31 +6,51 @@ import PropTypes from 'prop-types'; import onClickOutside from 'react-onclickoutside'; import { FormGroup } from '../common-components'; -import { FORM_SUBMISSION_ERROR } from './data/constants'; +import { COUNTRY_CODE_KEY, COUNTRY_DISPLAY_KEY, FORM_SUBMISSION_ERROR } from './data/constants'; class CountryDropdown extends React.Component { constructor(props) { super(props); + this.handleFocus = this.handleFocus.bind(this); + this.handleOnBlur = this.handleOnBlur.bind(this); + this.state = { displayValue: '', icon: this.expandMoreButton(), errorMessage: '', showFieldError: true, }; - - this.handleFocus = this.handleFocus.bind(this); - this.handleOnBlur = this.handleOnBlur.bind(this); } shouldComponentUpdate(nextProps) { - if (this.props.value !== nextProps.value && nextProps.value !== null) { - const opt = this.props.options.find((o) => o[this.props.valueKey] === nextProps.value); - if (opt && opt[this.props.displayValueKey] !== this.state.displayValue) { - this.setState({ displayValue: opt[this.props.displayValueKey], showFieldError: false }); + const selectedCountry = this.props.options.find((o) => o[COUNTRY_CODE_KEY] === nextProps.value); + if (this.props.value !== nextProps.value) { + if (selectedCountry) { + this.setState({ + displayValue: selectedCountry[COUNTRY_DISPLAY_KEY], + showFieldError: false, + errorMessage: nextProps.errorMessage, + }); + return false; } + // Set persisted country value as display value. + this.setState({ displayValue: nextProps.value, showFieldError: true, errorMessage: nextProps.errorMessage }); + return false; + // eslint-disable-next-line no-else-return + } else if (nextProps.value && selectedCountry && this.state.displayValue === nextProps.value) { + // Handling a case here where user enters a valid country code that needs to be + // evaluated and set its display value. + this.setState({ displayValue: selectedCountry[COUNTRY_DISPLAY_KEY] }); + return false; + } + if (this.props.errorCode !== nextProps.errorCode && nextProps.errorCode === 'invalid-country') { + this.setState({ showFieldError: true, errorMessage: nextProps.errorMessage }); + return false; + } + if (this.state.errorMessage !== nextProps.errorMessage) { + this.setState({ showFieldError: true, errorMessage: nextProps.errorMessage }); return false; } - return true; } @@ -48,72 +68,30 @@ class CountryDropdown extends React.Component { } return options.map((opt) => { - const value = opt[this.props.valueKey]; - let displayValue = opt[this.props.displayValueKey]; - if (displayValue.length > 30) { - displayValue = displayValue.substring(0, 30).concat('...'); - } + const value = opt[COUNTRY_CODE_KEY]; + const displayValue = opt[COUNTRY_DISPLAY_KEY]; return ( - ); }); } - setValue(value) { - if (this.props.value === value) { - return; - } - - if (this.props.handleChange) { - this.props.handleChange(value); - } - - const opt = this.props.options.find((o) => o[this.props.valueKey] === value); - if (opt && opt[this.props.displayValueKey] !== this.state.displayValue) { - this.setState({ displayValue: opt[this.props.displayValueKey], showFieldError: false }); - } - } - - setDisplayValue(value) { - const normalized = value.toLowerCase(); - const opt = this.props.options.find((o) => o[this.props.displayValueKey].toLowerCase() === normalized); - if (opt) { - this.setValue(opt[this.props.valueKey]); - this.setState({ displayValue: opt[this.props.displayValueKey], showFieldError: false }); - } else { - this.setValue(null); - this.setState({ displayValue: value, showFieldError: true }); - } - } - - handleClick = (e) => { - const dropDownItems = this.getItems(e.target.value); - if (dropDownItems?.length > 1) { - this.setState({ dropDownItems, icon: this.expandLessButton(), errorMessage: '' }); - } - - if (this.state.dropDownItems?.length > 0) { - this.setState({ dropDownItems: '', icon: this.expandMoreButton(), errorMessage: '' }); - } - } - handleOnChange = (e) => { const filteredItems = this.getItems(e.target.value); - - this.setState({ dropDownItems: filteredItems, icon: this.expandLessButton(), errorMessage: '' }); - this.setDisplayValue(e.target.value); + this.setState({ + dropDownItems: filteredItems, + displayValue: e.target.value, + }); } handleClickOutside = () => { if (this.state.dropDownItems?.length > 0) { - const msg = this.state.displayValue === '' ? this.props.errorMessage : ''; this.setState(() => ({ icon: this.expandMoreButton(), dropDownItems: '', - errorMessage: msg, })); } } @@ -122,32 +100,51 @@ class CountryDropdown extends React.Component { this.setState({ dropDownItems: '', icon: this.expandMoreButton() }); } - handleExpandMore(e) { - const dropDownItems = this.getItems(e.target.value); - this.setState({ - dropDownItems, icon: this.expandLessButton(), errorMessage: '', showFieldError: false, - }); + handleExpandMore() { + this.setState(prevState => ({ + dropDownItems: this.getItems(prevState.displayValue), icon: this.expandLessButton(), errorMessage: '', showFieldError: false, + })); } handleFocus(e) { - this.setState({ showFieldError: false }); + const { name, value } = e.target; + this.setState(prevState => ({ + dropDownItems: this.getItems(name === 'country' ? value : prevState.displayValue), + icon: this.expandLessButton(), + errorMessage: '', + showFieldError: false, + })); if (this.props.handleFocus) { this.props.handleFocus(e); } } - handleOnBlur(e) { - if (this.props.handleBlur) { this.props.handleBlur(e); } + handleOnBlur(e, itemClicked = false) { + const { name } = e.target; + const countryValue = itemClicked ? e.target.value : this.state.displayValue; + // For a better user experience, do not validate when focus out from 'country' field + // and focus on 'countryItem' or 'countryExpand' button. + if (e.relatedTarget && e.relatedTarget.name === 'countryItem' && (name === 'country' || name === 'countryExpand')) { + return; + } + const normalized = countryValue.toLowerCase(); + const selectedCountry = this.props.options.find((o) => o[COUNTRY_DISPLAY_KEY].toLowerCase() === normalized); + if (!selectedCountry) { + this.setState({ errorMessage: this.props.errorMessage, showFieldError: true }); + } + if (this.props.handleBlur) { this.props.handleBlur({ target: { name: 'country', value: countryValue } }); } } handleItemClick(e) { - this.setValue(e.target.value); this.setState({ dropDownItems: '', icon: this.expandMoreButton() }); - this.handleOnBlur(e); + this.handleOnBlur(e, true); } expandMoreButton() { return ( { - // TODO: Remove this function once error message is passed as props from - // RegistrationPage to CountryDropdown component. - if (this.props.registrationFormData.errors.country) { - this.props.setRegistrationFormData({ - country: value, - errors: { - ...this.props.registrationFormData.errors, - country: '', - }, - }); - } else { - this.props.setRegistrationFormData({ - country: value, - }); - } - } - handleOnFocus = (e) => { const fieldName = e.target.name; this.setState({ @@ -350,8 +334,8 @@ class RegistrationPage extends React.Component { if (fieldName === 'username') { this.props.clearUsernameSuggestions(); } - if (fieldName === 'country') { - this.setState({ readOnly: false }); + if (fieldName === 'countryExpand') { + errors.country = ''; } if (fieldName === 'passwordValidation') { errors.password = ''; @@ -530,23 +514,35 @@ class RegistrationPage extends React.Component { } break; case 'country': - if (!value.trim()) { - errors.country = intl.formatMessage(messages['empty.country.field.error']); - } else { - errors.country = ''; + value = value.trim(); // eslint-disable-line no-param-reassign + if (value) { + const normalizedValue = value.toLowerCase(); + let selectedCountry = this.countryList.find((o) => o[COUNTRY_DISPLAY_KEY].toLowerCase() === normalizedValue); + if (selectedCountry) { + value = selectedCountry[COUNTRY_CODE_KEY]; // eslint-disable-line no-param-reassign + errors.country = ''; + break; + } else { + // Handling a case here where user enters a valid country code that needs to be + // evaluated and set its value as a valid value. + selectedCountry = this.countryList.find((o) => o[COUNTRY_CODE_KEY].toLowerCase() === normalizedValue); + if (selectedCountry) { + value = selectedCountry[COUNTRY_CODE_KEY]; // eslint-disable-line no-param-reassign + errors.country = ''; + break; + } + } } + errors.country = intl.formatMessage(messages['empty.country.field.error']); break; default: break; } - if (fieldName !== 'country') { - state = { - ...state, - [fieldName]: value, - }; - } - + state = { + ...state, + [fieldName]: value, + }; this.props.setRegistrationFormData({ ...state, errors, @@ -674,9 +670,7 @@ class RegistrationPage extends React.Component { this.setState(prevState => ({ values: { ...prevState.values, country: value } })) } errorCode={this.state.errorCode} - readOnly={this.state.readOnly} /> ); @@ -824,16 +817,12 @@ class RegistrationPage extends React.Component { this.handleCountryChange(value)} + errorMessage={this.state.errors.country} errorCode={this.state.errorCode} - readOnly={this.state.readOnly} /> )} {formFields} diff --git a/src/register/data/constants.js b/src/register/data/constants.js index 1e566cb7..4d9e79c4 100644 --- a/src/register/data/constants.js +++ b/src/register/data/constants.js @@ -169,3 +169,6 @@ export const DEFAULT_TOP_LEVEL_DOMAINS = [ 'xyz', 'yachts', 'yahoo', 'yamaxun', 'yandex', 'ye', 'yodobashi', 'yoga', 'yokohama', 'you', 'youtube', 'yt', 'yun', 'za', 'zappos', 'zara', 'zero', 'zip', 'zippo', 'zm', 'zone', 'zuerich', 'zw', ]; + +export const COUNTRY_CODE_KEY = 'code'; +export const COUNTRY_DISPLAY_KEY = 'name'; diff --git a/src/register/tests/RegistrationPage.test.jsx b/src/register/tests/RegistrationPage.test.jsx index 8fafef1b..1e703c4e 100644 --- a/src/register/tests/RegistrationPage.test.jsx +++ b/src/register/tests/RegistrationPage.test.jsx @@ -350,7 +350,7 @@ describe('RegistrationPage', () => { expect(registrationPage.find('div[feedback-for="password"]').text()).toContain(emptyFieldValidation.password); registrationPage.find('input#password').simulate('focus'); expect(registrationPage.find('div[feedback-for="country"]').text()).toEqual(emptyFieldValidation.country); - registrationPage.find('input#country').simulate('blur', { target: { value: 'US', name: 'country' } }); + registrationPage.find('input#country').simulate('focus'); expect(registrationPage.find('RegistrationPage').state('errors')).toEqual(errors); }); @@ -363,12 +363,6 @@ describe('RegistrationPage', () => { expect(store.dispatch).toHaveBeenCalledWith(clearUsernameSuggestions()); }); - it('should set readOnly state false if focus on country field', () => { - const registrationPage = mount(reduxWrapper()); - registrationPage.find('input#country').simulate('focus'); - expect(registrationPage.find('RegistrationPage').state('readOnly')).toEqual(false); - }); - // ******** test alert messages ******** it('should match third party auth alert', () => { @@ -958,11 +952,40 @@ describe('RegistrationPage', () => { expect(store.dispatch).toHaveBeenCalledWith(setRegistrationFormData(formData)); }); - it('should set country in redux store on country change', () => { + it('should set country code in redux store on country field blur', () => { + const formData = { + country: 'PK', + errors: { + country: '', + email: '', + name: '', + password: '', + username: '', + }, + }; store.dispatch = jest.fn(store.dispatch); const registerPage = mount(reduxWrapper()); - registerPage.find('input#country').simulate('change', { target: { value: 'Pakistan', name: 'country' } }); - expect(store.dispatch).toHaveBeenCalledWith(setRegistrationFormData({ country: 'PK' })); + registerPage.find('RegistrationPage').setState({ country: 'PK' }); + registerPage.find('input#country').simulate('blur'); + expect(store.dispatch).toHaveBeenCalledWith(setRegistrationFormData(formData)); + }); + + it('should set country value with field error in redux store on country field blur', () => { + const formData = { + country: 'test', + errors: { + country: 'Select your country or region of residence', + email: '', + name: '', + password: '', + username: '', + }, + }; + store.dispatch = jest.fn(store.dispatch); + const registerPage = mount(reduxWrapper()); + registerPage.find('RegistrationPage').setState({ country: 'test' }); + registerPage.find('input#country').simulate('blur'); + expect(store.dispatch).toHaveBeenCalledWith(setRegistrationFormData(formData)); }); it('should set country in component state on country change', () => { @@ -1042,6 +1065,7 @@ describe('RegistrationPage', () => { const registerPage = mount(reduxWrapper()); populateRequiredFields(registerPage, payload); + registerPage.find('RegistrationPage').setState({ values: { country: 'PK' } }); registerPage.find('input#profession').simulate('change', { target: { value: 'Engineer', name: 'profession' } }); registerPage.find('button.btn-brand').simulate('click'); expect(store.dispatch).toHaveBeenCalledWith(registerNewUser({ ...payload, country: 'PK' }));