Conversation
a5e8822 to
f924c7b
Compare
There was a problem hiding this comment.
Now that we can connect and disconnect, we should change isActive to isConnected:
packages/js/src/general/initialize.js
// If site kit feature is enabled and connected: add the top pages widget.
if ( siteKitConfiguration.isFeatureEnabled && siteKitConfiguration.isActive ) {
initialWidgets.push( "topPages" );
}The dismissal of the widget after connecting should be permanently dismissed if we don't have "take a tour" button. Following Create an endpoint for permanently dismissing the site kit set up flow, maybe for a separate issue.
|
For the first, yes, that is good addition! 👍 About the permanent dismissal, that is indeed a separate issue: https://github.com/Yoast/reserved-tasks/issues/450 |
|
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
# Conflicts: # packages/js/src/dashboard/index.js # packages/js/src/general/initialize.js
# Conflicts: # packages/js/src/dashboard/index.js # packages/js/src/general/initialize.js
Not actual functionality
Using import instead of imageLink to prevent having to know the plugin URL
* remove tour for now * change dismiss to primary with Got it! instead
Using the same in the setup widget as in the grant consent modal
a46781b to
0106c02
Compare
|
CR & AC ✅ We need a follow up issue to show the "Top 5 most popular content" once connected on the dashboard (now it's only after refresh) and consider the order of the widgets. With this patch it is added last and we might want it first |
Pull Request Test Coverage Report for Build 968b0b709b30bf218fcedaabe3e170e95f223ff7Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Learn morelink ishttps://yoa.st/dashboard-site-kit-learn-moreLearn morelink ishttps://yoa.st/dashboard-site-kit-consent-learn-moresite_kit_manage_consentwith consenttrueGot it!primary button, that closes the setup widget.Learn morelink ishttps://yoa.st/integrations-about-site-kitDisconnectbuttonsite_kit_manage_consentwith consentfalseLearn morelink ishttps://yoa.st/integrations-site-kit-consent-learn-moresite_kit_manage_consentwith consenttrueNothing without feature flag
Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
UI changes
Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.Documentation
Quality assurance
Innovation
innovationlabel.Fixes https://github.com/Yoast/reserved-tasks/issues/435