site-kit-wp
site-kit-wp copied to clipboard
Implement a new modal component showing current vs new tailored suggestions
Feature Description
Once the site purpose question has been updated for tailored metrics users and the user has clicked the “Confirm changes” CTA, a new modal component will render, displaying the current metrics and the new tailored metrics alongside one another.
See New modal component to update the tailored metrics based on the changed answer section in the design doc
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- A new modal component is added matching the Figma design
- The
Current metricslist will show metric items currently displayed on the dashboard - The
New tailored metricslist will show the metric items that will be used instead, based on the updated site purpose answer - Clicking the
Keep current selectionCTA will only close the modal - Clicking the
Update metrics selectionCTA should apply the change and re-compute the tailored metrics based on the updated site purpose answer. - See the New modal component to update the tailored metrics based on the changed answer section in the design doc for a more technical overview
- This issue should only add new component and accompanying stories, trigger of the modal based on the answer update will be handled in #9439
Implementation Brief
- [x] Add
assets/js/components/KeyMetrics/ConfirmSitePurposeChangeModal.js- Use
assets/js/components/ModalDialog.jsas a starting point dialogActiveandonCloseprops will be used to control opening/closing of the dialog- Add the title and description from Figma design
- Update
DialogContentto show 2 column layout as per design - For the
Current metrics, retrieve the items usinggetKeyMetricsselector fromCORE_USERdatastore and render them in a list - For the
New tailored metrics, you can usegetAnswerBasedMetricsselector. But modify it accept extra param forpurposevalue. And then allowpurposeto be overridden when included https://github.com/google/site-kit-wp/blob/9a1a4f1d66be97d1839e663b4c3b4eceb839d081/assets/js/googlesitekit/datastore/user/key-metrics.js#L266- Retrieve the settings from
getUserInputSettingsselector and extract thepurposevalue like in the referenced code above. The new selected answer will be updated in the state upon changing, so it will reflect how tailored metrics will look like when passed as an argument to thegetAnswerBasedMetricsselector
- Retrieve the settings from
- First CTA should have label
Keep current selection- It should invoke
onClosecallback prop
- It should invoke
- Second CTA should have label
Update metrics selection- As a callback when this button is clicked, invoke
saveUserInputSettingsaction from theCORE_USERdatastore, and close the modal
- As a callback when this button is clicked, invoke
- Use
Test Coverage
- Add stories for
ConfirmSitePurposeChangeModalcomponent - Update
assets/js/googlesitekit/datastore/user/key-metrics.test.jsto include a test case forgetAnswerBasedMetricswhenpurposeargument is included
QA Brief (QA:ENG)
- Set up Site Kit with tailored KMW with an option other than
Publish News - The following sections will implement the modal in a forced visible mode in order to test it:
- Within the
SettingsCardKeyMetricscomponent:- Import
useStatein the existing import line for'@wordpress/element' - Within the
SettingsCardKeyMetricscomponent:- Add a default state with a setter for dialog visibility:
const [ dialogActive, toggleDialog ] = useState( true ); - Add a
handleDialogcallback function to handle closing of the modal:
- Add a default state with a setter for dialog visibility:
- Import
- Within the
const handleDialog = useCallback( () => {
toggleDialog( ! dialogActive );
}, [ dialogActive ] );
* Between the `isUserInputCompleted` and `isUserInputCompleted === false` statement blocks, output the modal for testing:
{ isUserInputCompleted && (
<ConfirmSitePurposeChangeModal
dialogActive={ dialogActive }
handleDialog={ handleDialog }
/>
) }
* This should trigger the modal on admin settings page load.
* Confirm that the current and new KMW items are the same in the modal (as no change to site purpose has been made), as per the screenshot below:
* Now, within the `ConfirmSitePurposeChangeModal` mock an update for site purpose by forcing the value of the `purpose` const to be `'publish_news'` as opposed to getting the value from the store.
* Reload the page and validate that the current vs new metrics lists are now different as per the image below:
- Validate that unit tests are passing.
Changelog entry
@zutigrm AC approved, thank you
- For
dialogActivevalue, leveragegetValueselector fromCORE_UIdatastore, and pull value from, sayKEY_METRICS_CONFIRM_SITE_PURPOSE_CHANGE_OPENkey ...- Callback function will close the modal updating the
KEY_METRICS_CONFIRM_SITE_PURPOSE_CHANGE_OPENtofalse. You can see an example usage here
@zutigrm, this component should always render the dialog whenever the component is rendered itself. In other words, it should not determine its state from the CORE_UI value. The parent component will have to make a decision whether this dialog should be displayed or not. To close the dialog, the parent component should pass an appropriate onClose callback.
This will make this component less coupled to the current state and it will be easier for us to maintain it and its stories.
Thanks @eugene-manuilov IB updated, reference to the UI state has been removed, the default props will control the dialog state
Thanks, IB ✔
QA Update ⚠
Note : For QA:Eng. I was testing the https://github.com/google/site-kit-wp/issues/9439 and verified the figma design as mentioned in AC of this ticket. I added couple of observations related to it below.
@10upsimon
1) In Figma title font size is 28px where as under actual implementation it is 22px.
2) Not a major issue, but I want to point out that on mobile devices, the modal title is slightly cut off.
QA Update ✅
- Tested on main environment.
- Verified logic and functionality under #9439
- Verified issue reported above is resolve now and tile font size is updated to 28px.