-
Notifications
You must be signed in to change notification settings - Fork 799
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
Recommendations: update button in My Plan to go to Recommendations #18843
Conversation
In #12277 and #12429, we added a "checklist" component to the Jetpack dashboard, as well as a CTA that would lead you to the checklist in Calypso. We never really expanded on that checklist and its display in the React dashboard, but we now have a Recommendations component we use in Jetpack, since #18437. This commit removes the elements from the checklist that we never used, and updates the CTA link in "My Plan" to point to the new Recommendations, when those recommendations must be shown to site owners.
Scheduled Jetpack release: March 2, 2021. Thank you for the great PR description! When this PR is ready for review, please apply the |
This tests well for me! Going to hold off on approval until someone a bit more familiar with React can take a gander, but everything works as expected for me. 👍 |
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.
Tested following the instructions on an Atomic site and on a Jetpack site. All worked as expected and I saw no errors.
I'm not a React expert either but the code changes look good. Always feel good to remove old code :)
Well done
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 If you are an automattician, once your PR is ready for review add the "[Status] Needs Team review" label and ask someone from your team review the code. jetpack plugin:
|
I had to commit again to fix a linting warning. I would appreciate another review on this :) Thank you! |
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.
Still works!
Did a pass over _inc/client/components looking for things that weren't used anywhere, and found a bunch: * components/data/query-connect-url - Last use removed in #8014 * components/data/query-connection-status - Last use removed in 62e9ab0 * components/data/query-modules - Last use removed in bfc40ad * components/data/query-plugin-updates - Last use removed in #17003 * components/data/query-site-products - Last use removed in #21594 * components/form/* - Didn't check for last use, too many bits. But it looks like the `formsy-react` package many of these depended on wasn't even installed since #8208. * components/inline-expand - Last use removed in #6550 * components/jetpack-dialogue - Last use removed in #16518 * components/jetpack-logo - Last use removed in #20148 * components/jetpack-termination-dialog - Last use removed in #21048 * components/module-settings/index.jsx - Last use removed in #10644 * components/module-settings/inline-module-toggle.jsx - Last use removed in #12118 * components/screen-reader-text - Last use removed in #18843 * components/settings - Last use removed in 26315e1, I think * components/tags-input - Last use removed in #11772 Then there were a few more that were only used from some of the above: * components/data/query-connected-plugins * components/module-settings/form-components.jsx * components/multiple-choice-question * components/setting-toggle
Did a pass over _inc/client/components looking for things that weren't used anywhere, and found a bunch: * components/data/query-connect-url - Last use removed in #8014 * components/data/query-connection-status - Last use removed in 62e9ab0 * components/data/query-modules - Last use removed in bfc40ad * components/data/query-plugin-updates - Last use removed in #17003 * components/data/query-site-products - Last use removed in #21594 * components/form/* - Didn't check for last use, too many bits. But it looks like the `formsy-react` package many of these depended on wasn't even installed since #8208. * components/inline-expand - Last use removed in #6550 * components/jetpack-dialogue - Last use removed in #16518 * components/jetpack-logo - Last use removed in #20148 * components/jetpack-termination-dialog - Last use removed in #21048 * components/module-settings/index.jsx - Last use removed in #10644 * components/module-settings/inline-module-toggle.jsx - Last use removed in #12118 * components/screen-reader-text - Last use removed in #18843 * components/settings - Last use removed in 26315e1, I think * components/tags-input - Last use removed in #11772 Then there were a few more that were only used from some of the above: * components/data/query-connected-plugins * components/module-settings/form-components.jsx * components/multiple-choice-question * components/setting-toggle Co-authored-by: Brandon Kraft <public@brandonkraft.com>
Changes proposed in this Pull Request:
In #12277 and #12429, we added a "checklist" component to the Jetpack dashboard, as well as a CTA that would lead you to the checklist in Calypso.
We never really expanded on that checklist and its display in the React dashboard, but we now have a Recommendations component we use in Jetpack, since #18437.
This commit removes the elements from the checklist that we never used, and updates the CTA link in "My Plan" to point to the new Recommendations, when those recommendations must be shown to site owners.
Jetpack product discussion
Does this pull request change what data or activity we track or use?
This PR introduces a new Tracks event that will be triggered when clicking on the button. It respects the same rules as the existing Tracks events on that page.
Testing instructions:
I would recommend testing this with an Atomic site as well as a Jetpack site.
Proposed changelog entry for your changes: