Add logic to trigger modal once user input questions have been updated
Feature Description
Following the implementation of new modal in #9438 this issue should connect the trigger for that modal, once site purpose question is changed in the Key metrics admin settings. Once the save button is clicked, and answer was modified, modal should be triggered
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Once the site purpose question is changed by the user in the
Key metricsadmin settings and the save button is clicked, it should open the new modal implemented in the #9438.- Modal should be triggered only when
conversionReportingfeature flag is enabled
- Modal should be triggered only when
Implementation Brief
- [ ] In
assets/js/components/user-input/UserInputPreviewGroup.js- Include new prop
onChange - Update
submitChangescallback, invokeonChangeprop if present within theif( ! response.error )conditional check
- Include new prop
- [ ] Update
assets/js/components/user-input/UserInputPreview.js- Add local state
[isModalOpen, setIsModalOpen]for example, with default value offalse - Include
onChangeprop to the https://github.com/google/site-kit-wp/blob/14f36cc292f05d0e72213ff63277e3873d89c7a1/assets/js/components/user-input/UserInputPreview.js#L145-L146- In the callback, return early if
conversionReportingfeature flag is not enabled, otherwise set aisModalOpenlocal state totrue
- In the callback, return early if
- Include
ConfirmSitePurposeChangeModalcomponent, wrapped withinPortalcomponent, so it is appended in the body outside the settings container.- Pass the
isModalOpenvalue to thedialogActive - For
onCloseprop, use callback that resetsisModalOpentofalse
- Pass the
- Add local state
Test Coverage
- No update needed
QA Brief
Changelog entry
@zutigrm AC LGTM, moving to IB
@zutigrm, I think it would be better to add a new prop onChange to the UserInputPreviewGroup component that will let us execute a callback when the setting is changed. Then use this prop for the corresponding question and display the new modal dialog in the UserInputPreview component without dealing with the CORE_UI state.
@eugene-manuilov makes sense, thanks. I updated IB to reflect this approach
Thanks, IB ✔
Adding an update following internal sync and comms with @zutigrm:
The logic needed to be tweaked as the current user input options are saved to the user input settings store, they're just not persisted. This affects the modal logic (specifically due to how we retrieve the user input values) as the old vs new would always be the same in this case.
The solution was to introduce a temporary form and set the old/current site purpose. This value is in turn used in the modal to get the current values against the existing purpose slug stored in the temporary form store. The temporary form is reset on modal save so the logic can be repeated for future changes.
Flagging further that additional logic needed to be introduced to the modal that resets the user input settings site purpose value to the prior purpose if the user dismisses the metric updates in the modal.
QA Update ✅
- Tested on dev environment.
- Tested when 'conversionReporting' feature flag disabled and verified that Modal not triggered only when
conversionReportingfeature flag is disabled. - Tested when 'conversionReporting' feature flag enabled and verified that Modal triggered when
conversionReportingfeature flag is enabled. - Verified that clicking on 'Keep current selection' CTA or outside the modal do not change the purpose of the site.
- Verified that clicking on 'Update metrics selection' change the purpose and metrics selection as per selected purpose on main dashboard.
- Verified that once the site purpose question is changed by the user in the Key metrics admin settings and the save button is clicked, it open the new modal implemented in the https://github.com/google/site-kit-wp/issues/9438.
https://github.com/user-attachments/assets/6ffa1ee9-46fe-4eba-94ce-d5b6dc7e6794
https://github.com/user-attachments/assets/b040ed40-5d0e-4542-abec-6d7fa4ae1ceb