-
Notifications
You must be signed in to change notification settings - Fork 32
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
Dashboard: Implement React Routing and Page Navigation #2940
Conversation
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several changes to the Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
|
@vaurdan, my messaging app is down, I've sent you an email. Will look into this soon! 🙂 |
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.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (6)
src/content-helper/dashboard-page/pages/index.tsx (1)
1-3
: Add JSDoc documentation for exportsWhile the barrel file pattern is well-implemented, consider adding JSDoc documentation with
@since
tags for each export to comply with WordPress coding standards.+/** + * Dashboard page component. + * + * @since 3.13.0 + */ export { DashboardPage } from './dashboard/page-component'; + +/** + * Traffic boost page component. + * + * @since 3.13.0 + */ export { TrafficBoostPage } from './traffic-boost/page-component'; + +/** + * Settings page component. + * + * @since 3.13.0 + */ export { SettingsPage } from './settings/page-component';src/content-helper/dashboard-page/pages/dashboard/page-component.tsx (1)
1-5
: Enhance JSDoc documentation for better maintainability.The JSDoc comment should be more descriptive and include TypeScript-specific documentation:
/** - * The main dashboard page component. + * Renders the main Parse.ly dashboard page component with a welcome message. * + * @return {JSX.Element} The rendered dashboard page component. * @since 3.18.0 */src/content-helper/dashboard-page/pages/traffic-boost/page-component.tsx (1)
6-13
: Consider adding error boundaries and loading states.As a page component, it should handle potential errors and loading states gracefully.
Would you like me to provide an example implementation with error boundaries and loading states?
src/UI/class-dashboard-page.php (2)
54-81
: Consider refactoring submenu registration for better maintainability.The submenu registration code contains repetitive patterns that could be simplified using a data structure and a loop.
Consider refactoring like this:
- add_submenu_page( - 'parsely-dashboard-page', - 'Parse.ly Dashboard Page', - 'Dashboard', - Parsely::CAPABILITY, - 'parsely-dashboard-page', - '__return_null' - ); - - add_submenu_page( - 'parsely-dashboard-page', - 'Parse.ly Traffic Boost', - 'Traffic Boost', - Parsely::CAPABILITY, - 'parsely-dashboard-page#/traffic-boost', - '__return_null' - ); - - add_submenu_page( - 'parsely-dashboard-page', - 'Parse.ly Settings', - 'Settings', - Parsely::CAPABILITY, - 'parsely-dashboard-page#/settings', - '__return_null' - ); + $submenus = array( + array( + 'title' => 'Parse.ly Dashboard Page', + 'menu_title' => 'Dashboard', + 'slug' => 'parsely-dashboard-page', + ), + array( + 'title' => 'Parse.ly Traffic Boost', + 'menu_title' => 'Traffic Boost', + 'slug' => 'parsely-dashboard-page#/traffic-boost', + ), + array( + 'title' => 'Parse.ly Settings', + 'menu_title' => 'Settings', + 'slug' => 'parsely-dashboard-page#/settings', + ), + ); + + foreach ( $submenus as $submenu ) { + add_submenu_page( + 'parsely-dashboard-page', + $submenu['title'], + $submenu['menu_title'], + Parsely::CAPABILITY, + $submenu['slug'], + '__return_null' + ); + }
93-113
: Consider defining the non-existent slug as a class constant.The magic string 'non-existent-slug' would be better defined as a class constant for maintainability and clarity.
Consider this change:
final class Dashboard_Page { + /** + * Slug used to disable default submenu highlighting. + * + * @var string + */ + private const DISABLED_SUBMENU_SLUG = 'non-existent-slug'; + // ... existing code ... public function fix_submenu_highlighting( string $parent_file ): string { global $submenu_file, $current_screen; if ( 'toplevel_page_parsely-dashboard-page' === $current_screen->base ) { // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited - $submenu_file = 'non-existent-slug'; + $submenu_file = self::DISABLED_SUBMENU_SLUG; } return $parent_file; }src/content-helper/dashboard-page/dashboard-page.tsx (1)
34-40
: Remove the unnecessary@class
tag in the JSDoc comment.The
@class
tag is intended for documenting class components, butParselyDashboard
is a functional component.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (3)
build/content-helper/dashboard-page.asset.php
is excluded by!build/**
build/content-helper/dashboard-page.js
is excluded by!build/**
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (7)
package.json
(1 hunks)src/UI/class-dashboard-page.php
(3 hunks)src/content-helper/dashboard-page/dashboard-page.tsx
(1 hunks)src/content-helper/dashboard-page/pages/dashboard/page-component.tsx
(1 hunks)src/content-helper/dashboard-page/pages/index.tsx
(1 hunks)src/content-helper/dashboard-page/pages/settings/page-component.tsx
(1 hunks)src/content-helper/dashboard-page/pages/traffic-boost/page-component.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/content-helper/dashboard-page/pages/settings/page-component.tsx
🧰 Additional context used
📓 Path-based instructions (5)
src/UI/class-dashboard-page.php (1)
Pattern **/*.{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/content-helper/dashboard-page/dashboard-page.tsx (1)
Pattern **/*.{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/dashboard/page-component.tsx (1)
Pattern **/*.{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/index.tsx (1)
Pattern **/*.{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.tsx (1)
Pattern **/*.{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."
🔇 Additional comments (2)
src/UI/class-dashboard-page.php (1)
32-32
: LGTM: Filter addition is appropriate.
The addition of the parent_file
filter is a good approach to handle submenu highlighting in the WordPress admin menu.
src/content-helper/dashboard-page/dashboard-page.tsx (1)
24-28
: Verify compatibility of the future
prop in the Router
component.
The future
prop with options like v7_relativeSplatPath
and v7_startTransition
may rely on features from unreleased versions of React Router. Ensure these features are stable and compatible with react-router-dom
version "^6.28.0".
src/content-helper/dashboard-page/pages/traffic-boost/page-component.tsx
Show resolved
Hide resolved
src/content-helper/dashboard-page/pages/traffic-boost/page-component.tsx
Show resolved
Hide resolved
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.
Looks good to me! 🙂
I don't know if we'll have all these subpages when we first release since we might have no time to put everything in, but it's good to have the code already.
Let's merge this after we've merged #2937.
Also, check whether there are actionable CodeRabbit comments.
Description
This PR extends the work on #2937, by converting the dashboard in a sort of SPA, using React Router to handle the hash routing, and adds some barebones components and effects to handle the navigation.
Motivation and context
Be able to extend the dashboard in different pages, and still being able to navigate to those pages using the native WordPress sidebar menu.
How has this been tested?
Tested locally on my environment by playing the the different pages.
Screenshots (if appropriate)
Screen.Recording.2024-11-13.at.09.27.47.mov
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes