Add contact information section and its edit subpage to the settings page #921

eason9487 · 2021-07-30T07:51:48Z

Changes proposed in this Pull Request:

Partial of #863 and based on #918

Add URLs to utils for settings and edit contact info pages
Implement the preview section and edit subpage of contact information, and add them to the settings page

💡 Please note that this PR doesn't include the API integration.

Screenshots:

🎥 . When the phone number got from Google MC is empty

Kapture.2021-07-30.at.15.46.09.mp4

🎥 . When the phone number got from Google MC has already exist

Kapture.2021-07-30.at.15.48.10.mp4

Detailed test instructions:

For easier testing, there is a mock phone number that is 50% chance for an empty phone number and 50% for an existed phone number when every time reload the page.

google-listings-and-ads/js/src/data/selectors.js

Lines 71 to 78 in 4f5eb4e

    
           const mockPhoneNumber = Math.random() > 0.5 ? '+12133734253' : ''; 
        
           export const getGoogleMCPhoneNumber = () => { 
        
           	// TODO: [lite-contact-info] integrate with API 
        
           	return { 
        
           		id: '123456789', 
        
           		phone_number: mockPhoneNumber, 
        
           	}; 
        
           };

Go to the settings page.
The "Contact information" should show up with
- a warning section that prompts users to add information if the phone data is empty.
- a folded section that displays phone number and store address if the phone data is not empty.
Click on "Add information" or "Edit" button, and it should bring you to contact info edit page.
In the contact info edit page,
- the implementations are the same as the previous PRs. Please refer to Implement PhoneNumberCard and ContactInformation components for contact info #917 and Implement StoreAddressCard component for contact info #918 if want to test them further.
- click on "<" icon at the top-left side, and it should bring you back to settings page.

Changelog entry

j111q · 2021-07-30T08:17:17Z

One note - we tweaked the wording on the message here:

So it should say Google requires the phone number and store address for all stores using Google Merchant Center.

We took out the phrase about "owner of the business" because that could be confusing - it's not necessarily the owner who is setting up the integration or managing it.

Address #921 (comment)

eason9487 · 2021-07-30T08:23:06Z

@j111q, thanks for notifying me. Updated by e9de8d5.

j111q · 2021-07-30T08:29:45Z

One more thing - link to documentation / all the "Read more" links should go here: https://docs.woocommerce.com/document/google-listings-and-ads/#contact-information

Thank you @eason9487 !

…page

Address #921 (comment)

eason9487 · 2021-08-03T03:53:15Z

js/src/components/contact-information/index.js

+					<PhoneNumberCard
+						isPreview
+						initEditing={ false }
+						onEditClick={ handleEditClick }
+					/>


Dev note: follow the change of #917 (comment) to pass phone as phoneNumber prop before merging this PR.

Added by 34e6f62.

ecgan

Tested as per test instructions, seems to be working good. Left one ❓ comment on external link and track event, and a few ❔ 💅 comments.

Think you would be able to handle the ❓ comment, approving this now as there isn't any real blocking issues.

ecgan · 2021-08-03T17:15:18Z

js/src/components/contact-information/index.js

+		getHistory().push( getEditContactInformationUrl() );
+	};
+
+	let sectionContent;


💅 Perhaps we can set a default sectionContent (the one with AppSpinner), then we don't need the else block in Line 61-67.

Changed by 3dbc1fa.

ecgan · 2021-08-03T17:18:00Z

js/src/components/contact-information/index.js

+			<Section.Card>
+				<AppSpinner />
+			</Section.Card>


❔ Would the SpinnerCard component be good here?

Suggested change

<Section.Card>

<AppSpinner />

</Section.Card>

<SpinnerCard />

The card height will be higher and looks better. Thanks!

Just recalled that the <Section.Card> <AppSpinner /> </Section.Card> was taken for aligning the height of all loading cards on the settings page.

But <SpinnerCard> looks better, to make all loading UIs consistency in the Setting page, I adjusted the spinner in the DisconnectAccounts as well - ffba448

ecgan · 2021-08-03T17:22:59Z

js/src/components/contact-information/index.js

+					<SpinnerCard />
+					<SpinnerCard />


Just a note: at first it looks weird to have two SpinnerCards like this, but after digging into the source code, I came to understand they correspond to the PhoneNumberCard and StoreAddressCard in Line 106-110.

ecgan · 2021-08-03T17:38:26Z

js/src/settings/edit-contact-information/index.js

+			/>
+			<div className="gla-settings">
+				<ContactInformation
+					view="settings"


❔ The value "settings" is not really being used in this PR. I'm guessing it is used in the other PRs? 🤔

Even though the value "settings" is not be used, I thought it still good to indicate the view prop because I think none of "settings" and "setup-mc" should be the default value. And I was planning to add JSDos for ContactInformation like this:

/** * @param {Object} props React props. * @param {'setup-mc'|'settings'} props.view */

ecgan · 2021-08-03T17:54:44Z

js/src/components/contact-information/no-contact-information-card.js

+						<AppButton
+							isTertiary
+							target="_blank"
+							href={ learnMoreUrl }
+						>
+							{ __( 'Learn more', 'google-listings-and-ads' ) }
+						</AppButton>


❓ I think we should do track event for this "Learn more" button here. Either we use AppButton with eventName and eventProps, or AppDocumentationLink (which I think is better in semantic, but I guess you would need to mingle with CSS layout a little).

Thanks for reminding. Added by bf236cb.

Address #921 (comment) Co-authored-by: Gan Eng Chin <ecgan@users.noreply.github.com>

…g UIs consistency in the Setting page Address #921 (comment)

Address #921 (comment)

…ionCard Address #921 (comment)

Address #921 (comment)

eason9487 requested a review from a team July 30, 2021 07:51

eason9487 self-assigned this Jul 30, 2021

eason9487 added a commit that referenced this pull request Jul 30, 2021

Update wording

986e32b

Address #921 (comment)

eason9487 added 6 commits August 2, 2021 11:14

Add URLs to utils for settings and edit contact info pages

2d4ec07

Implement ContactInformationPreview for the main page of settings

4debb52

Implement the EditContactInformation page

8c4a932

Add ContactInformationPreview and EditContactInformation to settings …

f654554

…page

Update wording

e9de8d5

Address #921 (comment)

Update the URL of "Learn more" links

53f1e64

eason9487 force-pushed the feature/863-mc-store-address branch from 05e45fc to 2eca6d2 Compare August 2, 2021 03:18

eason9487 force-pushed the feature/863-settings-contact-info branch from 36ca377 to 53f1e64 Compare August 2, 2021 03:18

eason9487 mentioned this pull request Aug 2, 2021

Integrate with contact information API and complete form submissions within MC setup and settings #926

Merged

eason9487 commented Aug 3, 2021

View reviewed changes

eason9487 mentioned this pull request Aug 3, 2021

Implement PhoneNumberCard and ContactInformation components for contact info #917

Merged

ecgan approved these changes Aug 3, 2021

View reviewed changes

eason9487 and others added 4 commits August 4, 2021 11:25

Use SpinnerCard instead

2e95109

Address #921 (comment) Co-authored-by: Gan Eng Chin <ecgan@users.noreply.github.com>

Use SpinnerCard instead for the DisconnectAccounts to make all loadin…

ffba448

…g UIs consistency in the Setting page Address #921 (comment)

Simplify the else condition for ContactInformationPreview

3dbc1fa

Address #921 (comment)

Add event tracking for the "Learn more" link in the NoContactInformat…

bf236cb

…ionCard Address #921 (comment)

Base automatically changed from feature/863-mc-store-address to feature/contact-info August 4, 2021 07:17

Pass phone number data as phoneNumber prop to PhoneNumberCard

34e6f62

Address #921 (comment)

eason9487 merged commit dfc4be1 into feature/contact-info Aug 4, 2021

eason9487 deleted the feature/863-settings-contact-info branch August 4, 2021 07:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add contact information section and its edit subpage to the settings page #921

Add contact information section and its edit subpage to the settings page #921

eason9487 commented Jul 30, 2021 •

edited

Loading

j111q commented Jul 30, 2021

eason9487 commented Jul 30, 2021 •

edited

Loading

j111q commented Jul 30, 2021

eason9487 Aug 3, 2021

eason9487 Aug 4, 2021

ecgan left a comment

ecgan Aug 3, 2021

eason9487 Aug 4, 2021

ecgan Aug 3, 2021

eason9487 Aug 4, 2021

eason9487 Aug 4, 2021

ecgan Aug 3, 2021

ecgan Aug 3, 2021

eason9487 Aug 4, 2021

ecgan Aug 3, 2021

eason9487 Aug 4, 2021

	const mockPhoneNumber = Math.random() > 0.5 ? '+12133734253' : '';
	export const getGoogleMCPhoneNumber = () => {
	// TODO: [lite-contact-info] integrate with API
	return {
	id: '123456789',
	phone_number: mockPhoneNumber,
	};
	};

Add contact information section and its edit subpage to the settings page #921

Add contact information section and its edit subpage to the settings page #921

Conversation

eason9487 commented Jul 30, 2021 • edited Loading

Changes proposed in this Pull Request:

Screenshots:

🎥 . When the phone number got from Google MC is empty

🎥 . When the phone number got from Google MC has already exist

Detailed test instructions:

Changelog entry

j111q commented Jul 30, 2021

eason9487 commented Jul 30, 2021 • edited Loading

j111q commented Jul 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecgan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eason9487 commented Jul 30, 2021 •

edited

Loading

eason9487 commented Jul 30, 2021 •

edited

Loading