Open joemcgill opened 1 month ago
The
BudgetRecommendations
component makes use of theuseFetchBudgetRecommendationEffect()
hook to fetch the current recommendations. This will need to be moved up to thePaidAdsSetupSections
component;
Adding a note for this solution. The initial budget for campaign creation also needs to be fetched for the CampaignAssetsForm
component or the component consumers of CampaignAssetsForm
:
js/src/pages/create-paid-ads-campaign/index.js
js/src/setup-ads/setup-ads-form.js
@eason9487 good note. I wonder if rather than trying to use useFetchBudgetRecommendationEffect()
in components that are setting the initial form context, we just handle this directly in the BudgetSection
component and use the recommended value any time the formProps.values.amount
value is set to 0
here. This way the logic for recommended value is consistent anywhere the BudgetSection
is displayed. What do you think?
I wonder if rather than trying to use
useFetchBudgetRecommendationEffect()
in components that are setting the initial form context, we just handle this directly in theBudgetSection
component and use the recommended value any time theformProps.values.amount
value is set to0
here. This way the logic for recommended value is consistent anywhere theBudgetSection
is displayed. What do you think?
Along with #2501, I expect this processing will be removed together as BudgetSection
should no longer have a disabled prop. In addition, the processing is a workaround since we don't have the later-added AdaptiveForm
to better handle this scenario of handling form states across multiple layers of components.
Therefore, I would suggest not adding additional logic to BudgetSection
to observe and set the initial budget value of the form. When possible, it would be better to handle the initial values in or on top of the form component.
Along with https://github.com/woocommerce/google-listings-and-ads/issues/2501, I expect this processing will be removed together as BudgetSection should no longer have a disabled prop. In addition, the processing is a workaround since we don't have the later-added AdaptiveForm to better handle this scenario of handling form states across multiple layers of components.
👍🏻 Sounds good.
Therefore, I would suggest not adding additional logic to
BudgetSection
to observe and set the initial budget value of the form. When possible, it would be better to handle the initial values in or on top of the form component.
This was my initial instinct as well. The downside to this approach is that if there are different forms that include the BudgetSection
component, we need to repeat the useFetchBudgetRecommendationEffect()
logic in each form when the initial formValues
are set, otherwise the behavior will be inconsistent.
After further discussion, we'll only need to set an initial value when creating a campaign, and not when editing, so handling this in the form rather than the BudgetSection
setting makes sense. Currently, we're using separate forms for setting up a campaign during onboarding vs setting up a campaign after onboarding (from the dashboard). However, we may consolidate that form in the future, so for now we'll only update this in the form used during onboarding. This currently happens here.
@eason9487 this issue is ready for review.
Assigning back to @ankitrox for iteration following Woo CR please
@ankitrox #2551 has been merged in the feature branch. Can you kindly update your branch with the latest from feature/2459-campaign-creation-flow
please?
Part of https://github.com/woocommerce/google-listings-and-ads/issues/2459
For the Budget card, we want to explain to users that:
Also, the warning message of “With a budget lower than your competitor range, your campaign may not get noticeable results.” appears from the start, before putting any number. Merchants might not connect that warning message to the actual budget they’re putting afterwards.
To set proper expectations with users we will automatically start with a the input set at the minimum recommended value and hide the initial budget warning from this card.
We'll also update the copy in the blue box to read something like this:
The recommendation amount will still be location-based as it is currently.
## Acceptance Criteria - [x] The budget warning message is no longer shown when the budget card first loads. - [x] The Daily Average Cost input should be set to the minimum recommended value initially instead of 0.00 - [x] When the Daily Average Cost input is changed, the budget warning should be shown only if the number is lower than the recommended daily average. - [x] The copy in the blue box should read `We recommend running campaigns at least 1 month so it can learn to optimize for your business. Tip: Most merchants targeting United States (US) set a daily budget of $%d USD.`, where %d is replaced by the recommended daily budget. - [x] When multiple target locations are selected, the location with the highest recommended daily budget is used (no change from current behavior) ## Implementation Brief The budget warning is already wired up so that it won't show if the daily average cost is higher than the recommended daily average, so the main thing to solve here is to ensure the initial form value is set to the recommended value. The `BudgetRecommendations` component makes use of the `useFetchBudgetRecommendationEffect()` hook to fetch the current recommendations. This will need to be moved up to the `PaidAdsSetupSections` component. Currently the default amount for campaigns is set to `0` [here](https://github.com/woocommerce/google-listings-and-ads/blob/develop/js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-setup-sections.js#L33-L38). This can be set using the `useFetchBudgetRecommendationEffect()` value. **Note for testing:** when this screen is loaded, the last value set gets stored in session storage in a `gla-onboarding-paid-ads` key. This will need to be cleared in order to see changes. The budget recommendations copy for both the warning and the tip (blue box) can be edited in `js/src/components/paid-ads/budget-section/budget-recommendation/index.js`. ## Test Coverage E2E tests for `tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js` need to be updated to confirm the behavior of the initial value, the visibility of the budget warning, and any tests confirming the copy of the budget tip needs to be updated.