-
Notifications
You must be signed in to change notification settings - Fork 913
Remove conditional and rename dashboard to general page. #21691
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
Conversation
… for the future dashboard.
…e-new-dashboard-feature-flag # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
…e-new-dashboard-feature-flag # Conflicts: # packages/js/src/general/app.js # packages/js/src/general/components/sidebar-recommendations.js # src/general/user-interface/general-page-integration.php
CR: ✅ |
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.
I'm pausing this in favor of https://github.com/Yoast/reserved-tasks/issues/303.
In the meantime, a couple of remarks to save us time 👇
I see a bunch of mentions of dashboard
in our files, for example:
useSelectDashboard
- the store is still named
@yoast/dashboard
- actually a search of
dashboard
inpackages/js/src/general
will give you a lot more instances - also, the
yoast-dashboard-notice
class in thecss/src/general-page.css
Not sure which of these we want to clean up, but let's take a conscious decision.
Also, maybe we should consider removing admin/page/dashboard.php
altogether? Although that would extend the impact of our changes. So another conscious decision to be made.
…e-new-dashboard-feature-flag # Conflicts: # admin/class-config.php # src/general/user-interface/general-page-integration.php # tests/Unit/General/User_Interface/General_Page_Integration_Test.php
CR: ✅ |
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.
Acceptance test is ✅
Tested new dashbard (General page) without feature flag in single site and multisite. |
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:
define( 'YOAST_SEO_NEW_DASHBOARD_UI', true );
in your project.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 theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes #