Remove Parse.ly Dashboard page from UI#3327
Conversation
acicovic
added
Maintenance & Fixes
📝 WalkthroughWalkthroughThis change statically highlights the "Traffic Boost (beta)" submenu in the Parse.ly dashboard, removes dynamic submenu link and highlighting logic based on routing, updates the submenu label and slug, and redirects the default dashboard route to "/traffic-boost". The "Traffic Boost" page header is also updated to include "(beta)". Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WP_Admin_Menu
participant React_App
User->>WP_Admin_Menu: Opens Parse.ly Dashboard menu
WP_Admin_Menu->>User: Shows only "Traffic Boost (beta)" submenu (highlighted)
User->>React_App: Navigates to Dashboard root (/)
React_App-->>User: Redirects to /traffic-boost
User->>React_App: Views "Manage Traffic Boost (beta)" page
Possibly related PRs
Suggested labels
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/UI/class-dashboard-page.php (1)
149-166:⚠️ Potential issueDuplicate submenu slug may create two identical items.
add_menu_page()automatically generates the first submenu entry whose slug is identical to the top-level slug (parsely-dashboard-page).
Callingadd_submenu_page()with that same slug will therefore yield two visually identical “Traffic Boost (beta)” rows (one inserted automatically, one inserted manually).Either:
- Remove the explicit
add_submenu_page()call and change the label of the auto-added submenu item via$GLOBALS['submenu'], or- Keep the explicit call but immediately
remove_submenu_page( 'parsely-dashboard-page', 'parsely-dashboard-page' )to suppress the auto-added row.Example fix (option 1):
- add_submenu_page( - 'parsely-dashboard-page', - 'Parse.ly Traffic Boost', - 'Traffic Boost (beta)', - Parsely::CAPABILITY, // phpcs:ignore WordPress.WP.Capabilities.Undetermined - 'parsely-dashboard-page', - '__return_null' - ); + // Rename the auto-added submenu item created by add_menu_page(). + global $submenu; + if ( isset( $submenu['parsely-dashboard-page'][0] ) ) { + $submenu['parsely-dashboard-page'][0][0] = __( 'Traffic Boost (beta)', 'wp-parsely' ); + }Duplication negatively affects UX and may break automated UI tests.
🧹 Nitpick comments (4)
src/content-helper/dashboard-page/pages/traffic-boost/page-component.tsx (1)
31-33: Provide translator context for the new “(beta)” suffix.String changes that add clarifying suffixes can be confusing for translators.
Please prepend a translator comment explaining that “(beta)” should be kept verbatim so it is clear that the feature is in beta.- <h1>{ __( 'Manage Traffic Boost (beta)', 'wp-parsely' ) }</h1> + {/* translators: Title of the Traffic-Boost page. “(beta)” must stay in English. */} + <h1>{ __( 'Manage Traffic Boost (beta)', 'wp-parsely' ) }</h1>This keeps us aligned with WordPress-i18n best practices and the coding-guideline requirement that comments terminate with a period.
src/UI/class-dashboard-page.php (1)
160-163: Add translator comment for the new menu label.Same rationale as for the React component: translators need guidance on the “(beta)” token.
/* translators: Admin-menu label – keep “(beta)” in English. */ 'Traffic Boost (beta)',src/content-helper/dashboard-page/dashboard-page.tsx (2)
24-28: Hard-coded submenu selector is brittle and lacks documentation.
- The query selector assumes the Traffic-Boost item is always the first submenu entry. If future menu items are re-ordered this highlight will break.
- A brief comment with
@sincewould satisfy the coding-guideline requirement and explain the intent.Suggestion:
- // Highlight the Traffic Boost menu item under the Parse.ly menu. - document.querySelector( - '#toplevel_page_parsely-dashboard-page .wp-submenu li.wp-first-item' - )?.classList.add( 'current' ); + // Highlight Traffic-Boost submenu while dashboard is temporarily single-page. + // @since 3.20.0. + const trafficBoostItem = document.querySelector( + '#toplevel_page_parsely-dashboard-page .wp-submenu li.wp-first-item' + ); + trafficBoostItem?.classList.add( 'current' );For extra resilience, consider selecting by link
hrefor text content rather than positional class.
50-53: Redirect route looks good but add a test for it.The new
<Navigate>ensures users hitting “/” land on Traffic-Boost.
Please add a unit/integration test (e.g., React-Testing-Library) to guard against regressions when routes change again.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
build/content-helper/dashboard-page.asset.phpis excluded by!build/**build/content-helper/dashboard-page.jsis excluded by!build/**
📒 Files selected for processing (3)
src/UI/class-dashboard-page.php(1 hunks)src/content-helper/dashboard-page/dashboard-page.tsx(2 hunks)src/content-helper/dashboard-page/pages/traffic-boost/page-component.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.{js,ts,tsx,jsx}`: "Perform a detailed review of the provided code with following key aspects in mind: - Review the code to ensure it is well-structured and adheres to best ...
**/*.{js,ts,tsx,jsx}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/content-helper/dashboard-page/pages/traffic-boost/page-component.tsxsrc/content-helper/dashboard-page/dashboard-page.tsx
`**/*.{html,php}`: "Perform a detailed review of the provided code with following key aspects in mind: - Review the HTML and PHP code to ensure it is well-structured and adheres ...
**/*.{html,php}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the HTML and PHP code to ensure it is well-structured and adheres to best practices.
- Ensure the code follows WordPress coding standards and is well-documented.
- Confirm the code is secure and free from vulnerabilities.
- Optimize the code for performance, removing any unnecessary elements.
- Validate comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Verify code compatibility with the latest version of WordPress, avoiding deprecated functions or features."
src/UI/class-dashboard-page.php
🧬 Code Graph Analysis (1)
src/UI/class-dashboard-page.php (1)
src/class-parsely.php (1)
Parsely(75-1173)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: E2E against WordPress latest
alecgeatches
left a comment
alecgeatches
left a comment
There was a problem hiding this comment.
I've tested these changes along with #3328, see the approval there. Good to go from me!
2 participants
Description
With this PR, we're removing the Parse.ly Dashboard page from the UI. We'll bring it back when we have something to offer there. As this is considered a temporary removal, this isn't a full refactoring removing all related code.
Motivation and context
The Parse.ly Dashboard page didn't have anything to offer.
How has this been tested?
Manually tested the UI.
Summary by CodeRabbit
New Features
Style
Bug Fixes