-
Notifications
You must be signed in to change notification settings - Fork 21
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
Setup Paid Ads Campaign Step 1 Account Connection #301
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
9093085
Add link to setup paid ads campaign page.
ecgan cb5c142
Add TopBar for Setup Ads flow.
ecgan 5aa1ede
Code refactor TopBar component to accept title props.
ecgan d9683a8
Use TopBar component in SetupAds.
ecgan a080f21
Add AdsStepper.
ecgan bd7b1d5
Merge branch 'trunk' into feature/setup-ads-step-1
ecgan e893db7
Display GoogleAccount in AdsStepper.
ecgan 89e987a
Use p element for better layout spacing.
ecgan 113c0fd
Display GoogleAdsAccountSection.
ecgan e6765c1
Add audience section in CreateCampaign.
ecgan a11d10e
Code refactor with AccountSelectControl.
ecgan aab8275
Add wp-data for Google Ads account.
ecgan 46c53c0
Add hooks for Google Ads account.
ecgan 83afb7a
Move AccountId to shared components folder.
ecgan 9a516b3
Move SpinnerCard to shared components folder.
ecgan ad75a8e
Create new Google Ads account or link an existing account.
ecgan dfabf69
Use import alias.
ecgan 975b1ae
Fix termslink typo.
ecgan dd8ca71
Better handler name to avoid confusion with the onCreateAccount props.
ecgan 1e33bf3
Display "Account " text in AccountSelectControl label.
ecgan de77b1d
Fix existing_ads missing in reducer default state.
ecgan 14d69e2
Change "All Programs" label to "Programs" based on new design.
ecgan 36191e4
Merge branch 'trunk' into feature/setup-ads-step-1
ecgan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { SelectControl } from '@wordpress/components'; | ||
import { __ } from '@wordpress/i18n'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import AppSpinner from '.~/components/app-spinner'; | ||
import './index.scss'; | ||
|
||
const AccountSelectControl = ( props ) => { | ||
const { accounts, ...rest } = props; | ||
|
||
if ( ! accounts ) { | ||
return <AppSpinner />; | ||
} | ||
|
||
const options = [ | ||
{ | ||
value: '', | ||
label: __( 'Select one', 'google-listings-and-ads' ), | ||
}, | ||
...accounts.map( ( acc ) => ( { | ||
value: acc, | ||
label: __( 'Account ', 'google-listings-and-ads' ) + acc, | ||
} ) ), | ||
]; | ||
|
||
return ( | ||
<div className="gla-account-select-control"> | ||
<SelectControl options={ options } { ...rest } /> | ||
</div> | ||
); | ||
}; | ||
|
||
export default AccountSelectControl; |
2 changes: 1 addition & 1 deletion
2
...merchant-center-select-control/index.scss → ...ponents/account-select-control/index.scss
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import useExistingGoogleAdsAccounts from '.~/hooks/useExistingGoogleAdsAccounts'; | ||
import AccountSelectControl from '../account-select-control'; | ||
|
||
const AdsAccountSelectControl = ( props ) => { | ||
const { existingAccounts } = useExistingGoogleAdsAccounts(); | ||
|
||
return <AccountSelectControl accounts={ existingAccounts } { ...props } />; | ||
}; | ||
|
||
export default AdsAccountSelectControl; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,44 +1,16 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { SelectControl } from '@wordpress/components'; | ||
import { __ } from '@wordpress/i18n'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import useExistingGoogleMCAccounts from '.~/hooks/useExistingGoogleMCAccounts'; | ||
import AppSpinner from '.~/components/app-spinner'; | ||
import './index.scss'; | ||
import AccountSelectControl from '../account-select-control'; | ||
|
||
const MerchantCenterSelectControl = ( props ) => { | ||
const { value, onChange } = props; | ||
const { existingAccounts } = useExistingGoogleMCAccounts(); | ||
|
||
if ( ! existingAccounts ) { | ||
return <AppSpinner />; | ||
} | ||
|
||
const options = [ | ||
{ | ||
value: '', | ||
label: __( 'Select one', 'google-listings-and-ads' ), | ||
}, | ||
...existingAccounts.map( ( acc ) => ( { | ||
value: acc.id, | ||
label: acc.id, | ||
} ) ), | ||
]; | ||
const accounts = | ||
existingAccounts && existingAccounts.map( ( acc ) => acc.id ); | ||
|
||
return ( | ||
<div className="gla-merchant-center-select-control"> | ||
<SelectControl | ||
options={ options } | ||
value={ value } | ||
onChange={ onChange } | ||
/> | ||
</div> | ||
); | ||
return <AccountSelectControl accounts={ accounts } { ...props } />; | ||
}; | ||
|
||
export default MerchantCenterSelectControl; |
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { useSelect } from '@wordpress/data'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { STORE_KEY } from '.~/data/constants'; | ||
|
||
const useExistingGoogleAdsAccounts = () => { | ||
return useSelect( ( select ) => { | ||
const existingAccounts = select( | ||
STORE_KEY | ||
).getExistingGoogleAdsAccounts(); | ||
const isResolving = select( STORE_KEY ).isResolving( | ||
'getExistingGoogleAdsAccounts' | ||
); | ||
|
||
return { existingAccounts, isResolving }; | ||
} ); | ||
}; | ||
|
||
export default useExistingGoogleAdsAccounts; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { useSelect } from '@wordpress/data'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { STORE_KEY } from '.~/data/constants'; | ||
import useGoogleAccount from './useGoogleAccount'; | ||
|
||
const useGoogleAdsAccount = () => { | ||
const { google, isResolving } = useGoogleAccount(); | ||
|
||
return useSelect( ( select ) => { | ||
if ( ! google || google.active === 'no' ) { | ||
return { | ||
googleAdsAccount: undefined, | ||
isResolving, | ||
}; | ||
} | ||
|
||
const acc = select( STORE_KEY ).getGoogleAdsAccount(); | ||
const isResolvingGoogleAdsAccount = select( STORE_KEY ).isResolving( | ||
'getGoogleAdsAccount' | ||
); | ||
|
||
return { | ||
googleAdsAccount: acc, | ||
isResolving: isResolvingGoogleAdsAccount, | ||
}; | ||
} ); | ||
}; | ||
|
||
export default useGoogleAdsAccount; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably it was not introduced here, but the placement of this link seems off from Figma:
Figma:
Actual:
BTW, "All programs" seem to be just "Programs" now, but this is a good candidate for a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks fine for me, accessed from https://gla1.test/wp-admin/admin.php?page=wc-admin&path=%2Fgoogle%2Fdashboard (I changed to "Programs", thanks for the heads up):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange, for me it looks different, and according to what I see in
.woocommerce-table .components-card__header { /*...*/ grid-template-columns: auto 1fr auto;
, it should look that way
Anyway, it does look the same on
trunk
, so it's not the matter of this PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomalec , thanks for sharing your screenshot. That is weird. Here's my screenshot, based on current trunk branch:
The thing is, within my "app-table-card" div, I have
woocommerce-card
but you havecomponents-card
. I look into the code injs/src/components/app-table-card/index.js
, it should render WooCommerce Card. Do you have any local changes in your machine?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the same results as Tomek. 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found some clues but have no idea how to fix that in the right way. When the WooCommerce Admin is activated in the Plugins page, it would load v1.9.0 of
@woocommerce/components
. Otherwise, it would load v1.8.3. And you can see they import different <Card> component sources. 😂v1.9.0
https://github.com/woocommerce/woocommerce-admin/blob/v1.9.0/packages/components/src/table/index.js
v1.8.3
https://github.com/woocommerce/woocommerce-admin/blob/v1.8.3/packages/components/src/table/index.js
@ecgan I believe once you activate the WooCommerce Admin plugin, you will see the same results. 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More precisely, the exactly loading version of
@woocommerce/components
should depend on the dependency of installed versions of the WooCommerce and WooCommerce Admin plugins.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eason9487 , thanks so much, that's awesome troubleshooting! 😃 👍
I do have WC Admin activated, but it is version 1.8.0 😂
I created an issue to track this: #320