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

Consolidate accounts onboarding: Connect to existing Google Ads account via the combo card #2640

Merged
Show file tree
Hide file tree
Changes from 56 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
63b182b
Onboarding: Connect to existing Ads account in Google Accounts Card
ankitrox Oct 4, 2024
f73c4b2
Conflict resolution
ankitrox Oct 4, 2024
96e3ef2
Add rendering condition.
ankitrox Oct 4, 2024
7e73f98
Separate components for body and footer.
ankitrox Oct 4, 2024
ba5ca7b
Connected label styling
ankitrox Oct 4, 2024
df93a29
Pass isVisible prop to card component.
ankitrox Oct 4, 2024
6fce3e6
Use ref for keeping connected account selected.
ankitrox Oct 4, 2024
4173593
Add E2E tests.
ankitrox Oct 4, 2024
2466f7e
Fix lint css.
ankitrox Oct 4, 2024
a4d0eb4
Fix: E2E tests.
ankitrox Oct 8, 2024
f4fb5c2
Resolve conflicts and merge base branch.
ankitrox Oct 8, 2024
4ef6d25
Add styling for select.
ankitrox Oct 8, 2024
32864c3
CR feedback.
ankitrox Oct 14, 2024
2e6bcfe
Resolve conflicts and merge base branch.
ankitrox Oct 15, 2024
2b7c811
Update tests.
ankitrox Oct 15, 2024
aa6a670
Resolve conflicts and merge base branch.
ankitrox Oct 18, 2024
fbb199f
CR feedback.
ankitrox Oct 18, 2024
c20fbea
Fix: E2E tests.
ankitrox Oct 18, 2024
7c9c848
CR feedback.
ankitrox Oct 18, 2024
57023bc
Resolve conflicts and merge base branch.
ankitrox Oct 22, 2024
9617a63
CR feedback round 2.
ankitrox Oct 22, 2024
9475fa2
Fix: lint js.
ankitrox Oct 22, 2024
8a562d1
Rename ConnectCTA to ConnectButton.
asvinb Oct 22, 2024
693efd2
Merge branch 'feature/2567-kickoff-mc-ads-account-creation' into upda…
ankitrox Oct 22, 2024
63c4fba
Add nonInteractive prop to app-select.
asvinb Oct 22, 2024
a83c242
Merge branch 'update/2596-connect-ads-account' of github.com:woocomme…
asvinb Oct 22, 2024
73350ef
Merge branch 'feature/2567-kickoff-mc-ads-account-creation' into upda…
ankitrox Oct 22, 2024
d58278e
Fix: E2E.
ankitrox Oct 22, 2024
5c3c5c3
Resolve conflicts and merge base branch.
ankitrox Oct 23, 2024
47a0c3d
Merge branch 'feature/2567-kickoff-mc-ads-account-creation' into upda…
ankitrox Oct 23, 2024
713afc0
Remove redundant prop.
ankitrox Oct 23, 2024
5dd622e
Fix: E2E failing test.
ankitrox Oct 24, 2024
87c0686
Merge branch 'feature/2567-kickoff-mc-ads-account-creation' into upda…
ankitrox Oct 24, 2024
1299ad1
Merge branch 'feature/2567-kickoff-mc-ads-account-creation' into upda…
ankitrox Oct 24, 2024
efe843a
Resolve conflicts and merge base branch.
ankitrox Oct 24, 2024
da1a10e
Merge branch 'feature/2567-kickoff-mc-ads-account-creation' into upda…
ankitrox Oct 25, 2024
690067d
Fix: styling the tertiary link.
ankitrox Oct 25, 2024
fe1b2ec
Fix: lint css error.
ankitrox Oct 25, 2024
6f85011
Resolve conflicts and merge base branch.
ankitrox Oct 25, 2024
60a101a
Merge branch 'feature/2567-kickoff-mc-ads-account-creation' into upda…
ankitrox Oct 28, 2024
098b886
Fix: failing E2E test
ankitrox Oct 28, 2024
0046434
Fix: E2E tests.
ankitrox Oct 29, 2024
b449092
Merge branch 'feature/2509-consolidate-google-account-cards' into upd…
ankitrox Oct 29, 2024
31c35a8
CR feedback.
ankitrox Oct 29, 2024
f12e5e3
Update condition for isConnected google ads account.
ankitrox Oct 29, 2024
5632913
Use useGoogleAdsAccountReady hook.
ankitrox Oct 29, 2024
2724b0f
Revert the css change.
ankitrox Oct 29, 2024
e5edd82
Update AccountCard component
joemcgill Oct 30, 2024
4901758
Update ConnectAds to use new AccountCard
joemcgill Oct 30, 2024
0b23082
Update docblocks to AccountCard
joemcgill Oct 30, 2024
c7e0e70
Delete ConnectAccountCard
joemcgill Oct 30, 2024
4c9bb4b
Delete ConnectAccountCard (realy)
joemcgill Oct 30, 2024
79a66ff
Use ConnectButton component.
asvinb Oct 31, 2024
b77f799
Merge pull request #2658 from woocommerce/update/2596-connect-ads-ref…
asvinb Oct 31, 2024
e3bbd83
Add render conditions in the connected combo card.
asvinb Oct 31, 2024
84811bd
Update E2E tests.
asvinb Oct 31, 2024
4cba145
Make label non interactive.
asvinb Oct 31, 2024
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
43 changes: 30 additions & 13 deletions js/src/components/account-card/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import { __ } from '@wordpress/i18n';
import classnames from 'classnames';
import { Flex, FlexItem, FlexBlock } from '@wordpress/components';
import GridiconPhone from 'gridicons/dist/phone';
import { Icon, store as storeIcon } from '@wordpress/icons';

Expand Down Expand Up @@ -125,6 +124,11 @@
top: `gla-account-card__styled--align-top`,
};

const indicatorAlignStyleName = {
...alignStyleName,
toDetail: 'gla-account-card__indicator--align-to-detail',
};

/**
* Renders a Card component with account info and status.
*
Expand All @@ -142,6 +146,9 @@
* @param {JSX.Element} [props.indicator] Indicator of actions or status on the right side of the card.
* @param {'center'|'top'} [props.alignIcon='center'] Specify the vertical alignment of leading icon.
* @param {'center'|'top'} [props.alignIndicator='center'] Specify the vertical alignment of `indicator`.
* @param {JSX.Element} [props.detail] Detail content below the card description.
* @param {boolean} [props.expandedDetail=false] Whether to expand the detail content.
* @param {JSX.Element} [props.actions] Actions content below the card detail.
* @param {Array<JSX.Element>} [props.children] Children to be rendered if needs more content within the card.
* @param {Object} [props.restProps] Props to be forwarded to Section.Card.
*/
Expand All @@ -156,12 +163,16 @@
alignIcon = 'center',
indicator,
alignIndicator = 'center',
detail,
expandedDetail = false,
actions,
children,
...restProps
} ) {
const cardClassName = classnames(
'gla-account-card',
disabled ? 'gla-account-card--is-disabled' : false,
expandedDetail ? 'gla-account-card--is-expanded-detail' : false,
className
);

Expand All @@ -172,19 +183,15 @@

const indicatorClassName = classnames(
'gla-account-card__indicator',
alignStyleName[ alignIndicator ]
indicatorAlignStyleName[ alignIndicator ]
);

return (
<Section.Card className={ cardClassName } { ...restProps }>
<Section.Card.Body>
<Flex gap={ 4 }>
{ icon && (
<FlexItem className={ iconClassName }>
{ icon }
</FlexItem>
) }
<FlexBlock>
<div className="gla-account-card__body-layout">
{ icon && <div className={ iconClassName }>{ icon }</div> }
<div className="gla-account-card__subject">
{ title && (
<Subsection.Title className="gla-account-card__title">
{ title }
Expand All @@ -200,13 +207,23 @@
{ helper }
</div>
) }
</FlexBlock>
</div>
{ detail && (
<div className="gla-account-card__detail">

Check warning on line 212 in js/src/components/account-card/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/components/account-card/index.js#L212

Added line #L212 was not covered by tests
{ detail }
</div>
) }
{ indicator && (
<FlexItem className={ indicatorClassName }>
<div className={ indicatorClassName }>
{ indicator }
</FlexItem>
</div>
) }
{ actions && (
<div className="gla-account-card__actions">

Check warning on line 222 in js/src/components/account-card/index.js

View check run for this annotation

Codecov / codecov/patch

js/src/components/account-card/index.js#L222

Added line #L222 was not covered by tests
{ actions }
</div>
) }
</Flex>
</div>
</Section.Card.Body>
{ children }
</Section.Card>
Expand Down
52 changes: 48 additions & 4 deletions js/src/components/account-card/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,60 @@
opacity: 0.5;
}

&--is-expanded-detail {
.gla-account-card__indicator {
grid-area: 1/3;
}

.gla-account-card__detail {
grid-area: 2/2/auto/span 2;
}
}

&__styled {
&--align-top {
align-self: flex-start;
}
}

&__body-layout {
display: grid;
grid-template-columns: auto 1fr auto;
align-items: center;
}

&__icon {
grid-area: 1/1/span 2;
margin-right: $grid-unit-20;
line-height: 0;
}

&__subject {
grid-area: 1/2;
}

&__indicator {
grid-area: 1/3/span 2;
margin-left: $grid-unit-20;

&--align-to-detail {
grid-area: 2/3;
margin-top: $grid-unit-15;
}
}

&__detail {
grid-area: 2/2;
margin-top: $grid-unit-15;
}

&__actions {
grid-area: 3/2/auto/span 2;
margin-top: $grid-unit-15;
}

&__title {
margin-bottom: $grid-unit-05;
margin: 0;
color: $black;
}

Expand All @@ -25,6 +67,7 @@
flex-direction: column;
align-items: flex-start;
gap: 1em;
margin-top: $grid-unit-05;
color: $gray-900;

> p {
Expand Down Expand Up @@ -65,11 +108,12 @@
}

@media (max-width: $break-small) {
.components-card__body > .components-flex {
&__body-layout {
display: flex;
flex-direction: column;
align-items: flex-start;
> .components-flex-block,
> .components-flex-item {

> div {
margin: $grid-unit-10;
}
}
Expand Down
18 changes: 10 additions & 8 deletions js/src/components/app-select-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import './index.scss';
* @param {Function} [props.onChange=noop] Callback function triggered when the selected value changes. Receives the new value as an argument.
* @param {string} [props.value] The currently selected value. This component should be used as a controlled component. A special case is that after mounting, when `autoSelectFirstOption` is true and `value` is undefined, it tries to call back `onChange` once to select the first option so that the `value` can be consistent with the `<select>` element's own value.
* @param {boolean} [props.autoSelectFirstOption=false] If true, automatically triggers the onChange callback with the first option as value when no value is provided. If only one option is available, the select control is also changed to non-interactive.
* @param {boolean} [props.nonInteractive=false] If true, the select control is changed to non-interactive.
* @param {*} [props.rest] Additional props passed to the `SelectControl` component.
*/
const AppSelectControl = ( props ) => {
Expand All @@ -29,6 +30,7 @@ const AppSelectControl = ( props ) => {
onChange = noop,
value,
autoSelectFirstOption = false,
nonInteractive = false,
...rest
} = props;
const shouldAutoSelectOnceRef = useRef( autoSelectFirstOption === true );
Expand All @@ -51,22 +53,22 @@ const AppSelectControl = ( props ) => {
...rest,
};

const hasSingleValueStyle = autoSelectFirstOption && options?.length === 1;
if ( hasSingleValueStyle ) {
const isNonInteractive =
( autoSelectFirstOption && options?.length === 1 ) || nonInteractive;
if ( isNonInteractive ) {
selectProps = {
...selectProps,
readOnly: true,
style: {
pointerEvents: 'none',
},
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
suffix: ' ',
tabIndex: '-1',
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
readOnly: true,
};
}

return (
<div
className={ classNames( 'app-select-control', className, {
'app-select-control--has-single-value': hasSingleValueStyle,
} ) }
>
<div className={ classNames( 'app-select-control', className ) }>
<SelectControl { ...selectProps } />
</div>
);
Expand Down
4 changes: 0 additions & 4 deletions js/src/components/app-select-control/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,3 @@
margin-bottom: 0;
}
}

.app-select-control--has-single-value {
pointer-events: none;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* External dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import AppButton from '.~/components/app-button';
import useEventPropertiesFilter from '.~/hooks/useEventPropertiesFilter';
import { FILTER_ONBOARDING } from '.~/utils/tracks';

/**
* Clicking on the button to connect an existing Google Ads account.
*
* @event gla_ads_account_connect_button_click
* @property {number} id The account ID to be connected.
* @property {string} [context] Indicates the place where the button is located.
* @property {string} [step] Indicates the step in the onboarding process.
*/

/**
* Google Ads account connection button.
*
* @param {Object} props Props.
* @param {number} props.accountID The Google Ads account ID to be connected.
* @param {Object} props.restProps Rest props. Forwarded to AppButton.
* @fires gla_ads_account_connect_button_click when "Connect" button is clicked.
* @return {JSX.Element} Google Ads connect button component.
*/
const ConnectButton = ( { accountID, ...restProps } ) => {
const getEventProps = useEventPropertiesFilter( FILTER_ONBOARDING );

return (
<AppButton
isSecondary
disabled={ ! accountID }
eventName="gla_ads_account_connect_button_click"
eventProps={ getEventProps( {
id: Number( accountID ),
} ) }
{ ...restProps }
>
{ __( 'Connect', 'google-listings-and-ads' ) }
</AppButton>
);
};

export default ConnectButton;
28 changes: 5 additions & 23 deletions js/src/components/google-ads-account-card/connect-ads/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,20 @@ import { CardDivider } from '@wordpress/components';
import AccountCard, { APPEARANCE } from '.~/components/account-card';
import AppButton from '.~/components/app-button';
import AppDocumentationLink from '.~/components/app-documentation-link';
import ConnectButton from './connect-button';
import ContentButtonLayout from '.~/components/content-button-layout';
import LoadingLabel from '.~/components/loading-label';
import Section from '.~/wcdl/section';
import Subsection from '.~/wcdl/subsection';
import useApiFetchCallback from '.~/hooks/useApiFetchCallback';
import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices';
import useGoogleAdsAccount from '.~/hooks/useGoogleAdsAccount';
import useEventPropertiesFilter from '.~/hooks/useEventPropertiesFilter';
import AdsAccountSelectControl from '.~/components/ads-account-select-control';
import { useAppDispatch } from '.~/data';
import { FILTER_ONBOARDING } from '.~/utils/tracks';
import './index.scss';

/**
* Clicking on the button to connect an existing Google Ads account.
*
* @event gla_ads_account_connect_button_click
* @property {number} id The account ID to be connected.
* @property {string} [context] Indicates the place where the button is located.
* @property {string} [step] Indicates the step in the onboarding process.
*/

/**
* @fires gla_ads_account_connect_button_click when "Connect" button is clicked.
* Connect to an existing Google Ads account.
* @fires gla_documentation_link_click with `{ context: 'setup-ads-connect-account', link_id: 'connect-sub-account', href: 'https://support.google.com/google-ads/answer/6139186' }`
* @param {Object} props React props
* @return {JSX.Element} {@link AccountCard} filled with content.
Expand All @@ -49,7 +39,6 @@ const ConnectAds = ( props ) => {
data: { id: value },
} );
const { refetchGoogleAdsAccount } = useGoogleAdsAccount();
const getEventProps = useEventPropertiesFilter( FILTER_ONBOARDING );
const { createNotice } = useDispatchCoreNotices();
const { fetchGoogleAdsAccountStatus } = useAppDispatch();

Expand Down Expand Up @@ -131,17 +120,10 @@ const ConnectAds = ( props ) => {
) }
/>
) : (
<AppButton
isSecondary
disabled={ ! value }
eventName="gla_ads_account_connect_button_click"
eventProps={ getEventProps( {
id: Number( value ),
} ) }
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
<ConnectButton
accountID={ value }
onClick={ handleConnectClick }
>
{ __( 'Connect', 'google-listings-and-ads' ) }
</AppButton>
/>
) }
</ContentButtonLayout>
</Section.Card.Body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const AccountDetails = () => {
<>
<p>{ google.email }</p>
<p>
{ googleMCAccount.id > 0 &&
{ googleMCAccount?.id > 0 &&
sprintf(
// Translators: %s is the Merchant Center ID
__(
Expand All @@ -34,7 +34,7 @@ const AccountDetails = () => {
) }
</p>
<p>
{ googleAdsAccount.id > 0 &&
{ googleAdsAccount?.id > 0 &&
sprintf(
// Translators: %s is the Google Ads ID
__( 'Google Ads ID: %s', 'google-listings-and-ads' ),
Expand Down
Loading