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.
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
Connect MC Account within combo card #2639
base: feature/2509-consolidate-google-account-cards
Are you sure you want to change the base?
Connect MC Account within combo card #2639
Changes from 78 commits
874ef00
1ead7f9
4428639
bebfaed
872787e
9475ec2
3965234
8a4ba4c
8b850fd
176a4a8
b4b4347
e9626f0
57c4693
e86de9b
9ad469a
75723e2
9923f70
406f028
26ec13a
acedda6
13d8d2b
9e8649b
2981067
e03f3c3
3b096ff
deaa836
66d6eb9
fa5b3ea
8707fea
14a08e8
d3c2a36
02a607a
25d2ff3
a8d5db1
87a2046
06960d9
8b06ef7
96c11a2
3b1fe26
cd62e71
b582447
4eb41ac
eb0bfc1
6696ee6
d2e4b57
67b8d3b
ba5c054
6fd6dc4
c13c524
e845ece
b64bba7
841bb90
dc6ea40
881ee8c
b9809e1
f41df99
2cadd89
604c5c8
5f4435a
8875a28
49f51ca
4d41762
c117be7
fa72d4e
7a4e40d
53c80c5
a9ea1b1
cb667fd
14c653c
a9c8839
a8b4f8c
9c55a39
c40800b
7a757a2
cc3e804
8c4cd7a
a3040fe
e75a930
8fba3e4
17e7c84
0e15813
4e201c1
65e6854
4b777bc
9a368f1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Not sure I understand why we need to save the MC account separately as component state here rather than using
googleMcaccount.id
directly?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.
@joemcgill We need to save the value set from the select in case the user needs to connect to a different acccount.
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.
This is a clever workaround. I assume this is for a newly created account that has not yet shown up in the API response for existing accounts? If so, adding that context to the inline doc would be helpful.
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.
@joemcgill I added it at the start of the component where we explain why we have this intermediary component:
https://github.com/woocommerce/google-listings-and-ads/pull/2639/files#diff-38d86396eccec4e8e7e5b37e8de9cf25e28fb0a2536261feb312adfe9c6689aeR15-R24
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.
Adding the
gla-google-combo-account-card
to be consistent with the other cards.