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.

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

Handle all loading states within the PhoneNumberCard component #934

Merged
merged 5 commits into from
Aug 6, 2021
Merged
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
26 changes: 9 additions & 17 deletions js/src/components/contact-information/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export function ContactInformationPreview() {
<VerticalGapLayout size="overlap">
<PhoneNumberCard
isPreview
initEditing={ false }
phoneNumber={ phone }
onEditClick={ handleEditClick }
/>
Expand Down Expand Up @@ -73,7 +72,7 @@ export default function ContactInformation( { view, onPhoneNumberChange } ) {
const phone = useGoogleMCPhoneNumber();
const isSetupMC = view === 'setup-mc';

const initEditing = isSetupMC ? ! phone.data.isValid : true;
const initEditing = isSetupMC ? null : true;
const title = isSetupMC ? mcTitle : settingsTitle;
const trackContext = isSetupMC
? 'setup-mc-contact-information'
Expand All @@ -97,21 +96,14 @@ export default function ContactInformation( { view, onPhoneNumberChange } ) {
</div>
}
>
{ phone.loaded ? (
<VerticalGapLayout size="large">
<PhoneNumberCard
phoneNumber={ phone }
initEditing={ initEditing }
onPhoneNumberChange={ onPhoneNumberChange }
/>
<StoreAddressCard />
</VerticalGapLayout>
) : (
<VerticalGapLayout size="large">
<SpinnerCard />
<SpinnerCard />
</VerticalGapLayout>
) }
<VerticalGapLayout size="large">
<PhoneNumberCard
phoneNumber={ phone }
initEditing={ initEditing }
onPhoneNumberChange={ onPhoneNumberChange }
/>
<StoreAddressCard />
</VerticalGapLayout>
</Section>
);
}
158 changes: 113 additions & 45 deletions js/src/components/contact-information/phone-number-card.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
parsePhoneNumberFromString as parsePhoneNumber,
} from 'libphonenumber-js';
import { __ } from '@wordpress/i18n';
import { useState } from '@wordpress/element';
import { useState, useEffect } from '@wordpress/element';
import { Flex, FlexItem, FlexBlock, CardDivider } from '@wordpress/components';
import { Spinner } from '@woocommerce/components';

Expand All @@ -24,6 +24,28 @@ import './phone-number-card.scss';

const noop = () => {};

const basePhoneNumberCardProps = {
className: 'gla-phone-number-card',
appearance: APPEARANCE.PHONE,
};

/**
* @typedef { import(".~/hooks/useGoogleMCPhoneNumber").PhoneNumber } PhoneNumber
* @typedef { import(".~/hooks/useGoogleMCPhoneNumber").PhoneNumberData } PhoneNumberData
*/

/**
* @typedef {Object} ExtraPhoneNumberData
* @property {boolean} isDirty Whether the phone number data contain unsaved changes.
*
* @typedef {PhoneNumberData & ExtraPhoneNumberData} CallbackPhoneNumberData
*/

/**
* @callback onPhoneNumberChange
* @param {CallbackPhoneNumberData} phoneNumberData The changed phone number data.
*/

function PhoneNumberContent( {
initCountry,
initNationalNumber,
Expand Down Expand Up @@ -64,6 +86,7 @@ function PhoneNumberContent( {
isDirty,
countryCallingCode,
nationalNumber: nextNumber,
country: nextCountry,
} );
}
};
Expand Down Expand Up @@ -105,67 +128,112 @@ function PhoneNumberContent( {
);
}

function EditPhoneNumberCard( { phoneNumber, onPhoneNumberChange } ) {
const { loaded, data } = phoneNumber;
const phoneNumberContent = loaded ? (
<PhoneNumberContent
initCountry={ data.country }
initNationalNumber={ data.nationalNumber }
onPhoneNumberChange={ onPhoneNumberChange }
/>
) : (
<AppSpinner />
);

return (
<AccountCard
{ ...basePhoneNumberCardProps }
description={ __(
'Please enter a phone number to be used for verification.',
'google-listings-and-ads'
) }
>
<CardDivider />
{ phoneNumberContent }
</AccountCard>
);
}

/**
* Renders phone number data in Card UI and is able to edit.
*
* @param {Object} props React props.
* @param {PhoneNumber} props.phoneNumber Phone number data.
* @param {boolean} [props.isPreview=false] Whether to display as preview UI.
* @param {boolean|null} [props.initEditing=null] Specify the inital UI state. This prop would be ignored if `isPreview` is true.
* `true`: initialize with the editing UI.
* `false`: initialize with the viewing UI.
* `null`: determine the initial UI state according to the `data.isValid` after the `phoneNumber` loaded.
* @param {Function} [props.onEditClick] Called when clicking on "Edit" button.
* If this callback is omitted, it will enter edit mode when clicking on "Edit" button.
* @param {onPhoneNumberChange} [props.onPhoneNumberChange] Called when inputs of phone number are changed in edit mode.
*/
Comment on lines +157 to +170
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the JSDoc! 😻

export default function PhoneNumberCard( {
phoneNumber,
isPreview,
initEditing,
isPreview = false,
initEditing = null,
onEditClick,
onPhoneNumberChange = noop,
} ) {
const [ isEditing, setEditing ] = useState( initEditing );
const { loaded, data } = phoneNumber;

const editButton = isEditing ? null : (
<AppButton
isSecondary
onClick={ () => {
if ( onEditClick ) {
onEditClick();
} else {
setEditing( true );
}
} }
>
{ __( 'Edit', 'google-listings-and-ads' ) }
</AppButton>
const [ isEditing, setEditing ] = useState(
isPreview ? false : initEditing
);

let description = null;
let phoneNumberContent = null;
// Handle the initial UI state of `initEditing = null`.
// The `isEditing` state is on hold. Determine it after the `phoneNumber` loaded.
useEffect( () => {
if ( loaded && isEditing === null ) {
setEditing( ! data.isValid );
}
}, [ loaded, data.isValid, isEditing ] );
Comment on lines +185 to +189
Copy link
Member

Choose a reason for hiding this comment

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

💅 It took me a while to figure out this relates to your JSDoc comment "determine the initial UI state according to the `data.isValid` after the `phoneNumber` loaded.". Would be good to have a comment to explain this, or even refactor it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a comment by 63460f7.


// Return a simple loading AccountCard since the initial edit state is unknown before loaded.
if ( isEditing === null ) {
return (
<AccountCard
{ ...basePhoneNumberCardProps }
indicator={ <Spinner /> }
/>
);
}

if ( isEditing ) {
description = __(
'Please enter a phone number to be used for verification.',
'google-listings-and-ads'
return (
<EditPhoneNumberCard
phoneNumber={ phoneNumber }
onPhoneNumberChange={ onPhoneNumberChange }
/>
);
}

if ( loaded ) {
phoneNumberContent = (
<>
<CardDivider />
<PhoneNumberContent
initCountry={ data.country }
initNationalNumber={ data.nationalNumber }
onPhoneNumberChange={ onPhoneNumberChange }
/>
</>
);
} else {
phoneNumberContent = <AppSpinner />;
}
} else {
description = loaded ? data.display : <Spinner />;
let description = null;
let indicator = <Spinner />;

if ( loaded ) {
description = data.display;
indicator = (
<AppButton
isSecondary
onClick={ () => {
if ( onEditClick ) {
onEditClick();
} else {
setEditing( true );
}
} }
>
{ __( 'Edit', 'google-listings-and-ads' ) }
</AppButton>
);
}

return (
<AccountCard
className="gla-phone-number-card"
appearance={ APPEARANCE.PHONE }
{ ...basePhoneNumberCardProps }
description={ description }
hideIcon={ isPreview }
indicator={ editButton }
>
{ phoneNumberContent }
</AccountCard>
indicator={ indicator }
/>
);
}
20 changes: 20 additions & 0 deletions js/src/hooks/useGoogleMCPhoneNumber.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,26 @@ const emptyData = {
display: '',
};

/**
* @typedef {Object} PhoneNumber
* @property {boolean} loaded Whether the data have been loaded.
* @property {PhoneNumberData} data User's phone number data fetched from Google Merchant Center.
*/

/**
* @typedef {Object} PhoneNumberData
* @property {string} country The country code. Example: 'US'.
* @property {string} countryCallingCode The country calling code. Example: '1'.
* @property {string} nationalNumber The national (significant) number. Example: '2133734253'.
* @property {boolean} isValid Whether the phone number is valid.
* @property {string} display The phone number string in international format. Example: '+1 213 373 4253'.
*/

/**
* A hook to load user's phone number data from Google Merchant Center.
*
* @return {PhoneNumber} [description]
*/
export default function useGoogleMCPhoneNumber() {
return useSelect( ( select ) => {
const { getGoogleMCPhoneNumber } = select( STORE_KEY );
Expand Down