feat: React Skeleton #2403
feat: React Skeleton #2403
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/Layout/index.tsx (1)
46-69: LGTM: Layout component structure is well-designed and flexible.The Layout component is well-structured and provides flexibility through conditional rendering of header and footer components. The use of SlotFillProvider is appropriate for WordPress plugin integration.
Consider using default prop values for better readability:
const Layout = ({ children, route, - title = '', + title = '', // Consider moving this default to the prop definition headerComponent, footerComponent, }: LayoutProps) => { // ... }This change doesn't affect functionality but can improve code readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- src/Dashboard/index.tsx (1 hunks)
- src/Layout/Header.tsx (1 hunks)
- src/Layout/index.tsx (1 hunks)
- src/Routing/index.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Dashboard/index.tsx
- src/Layout/Header.tsx
- src/Routing/index.tsx
🧰 Additional context used
🔇 Additional comments (6)
src/Layout/index.tsx (6)
1-9: LGTM: Imports are appropriate and comprehensive.The imports cover all necessary dependencies for the component's functionality, including WordPress-specific elements and custom components.
14-22: LGTM: ThemeProvider implementation is correct and follows best practices.The ThemeProvider component correctly manages the theme state and provides it to its children through the context. The use of useState for theme management is appropriate for this use case.
24-43: LGTM: Type definitions are comprehensive and enhance type safety.The DokanRoute type and LayoutProps interface provide clear and comprehensive type definitions, which will improve code readability and catch potential type-related errors early in development.
72-74: LGTM: useTheme custom hook is correctly implemented.The useTheme custom hook provides a convenient way to access the theme context in other components. Its implementation follows React hook conventions.
76-76: LGTM: Default export is correct.The Layout component is correctly exported as the default export, following common React component export patterns.
1-76: Overall, the Layout component is well-implemented with room for minor improvements.The new Layout component provides a flexible and type-safe structure for the application. It effectively integrates WordPress-specific components and implements theming capabilities. The code follows React best practices and is well-organized.
To further improve the code:
- Address the issue raised in the previous review comment regarding the default value for ThemeContext (lines 11-12).
- Implement the null check for
route.idas suggested in the previous review comment (line 65).- Consider moving the default value for the
titleprop to the prop definition for improved readability.These changes will enhance the robustness and maintainability of the component.
To ensure that the Layout component is being used correctly throughout the project, you can run the following verification script:
This script will help identify where the Layout component and useTheme hook are being used, allowing you to verify that they are being implemented correctly across the project.
This commit introduces a new REST API endpoint for searching customers specific to a vendor's orders. Key changes include: - Add new route `/dokan/v1/customers/search` in CustomersController - Implement `search_customers` method with the following features: * Search customers by ID or term (name/email) * Limit results to customers who have ordered from the current vendor * Support excluding specific customer IDs * Limit results to 20 when search term is less than 3 characters * Apply `dokan_json_search_found_customers` filter for extensibility - Add permission check to ensure only users with 'edit_shop_orders' capability can access - Include detailed PHPDoc for the new filter This feature allows vendors to efficiently search their customer base through the REST API, mirroring the functionality of the existing AJAX action while providing a standardized API interface.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/Routing/index.tsx (2)
1-5: Remove unused imports to improve code cleanliness and performance.The following imports are not used in this file and should be removed:
redirectreplaceApply this diff to remove unused imports:
-import { useNavigate, useParams, useLocation, redirect, replace, useMatches, useNavigation, } from 'react-router-dom'; +import { useNavigate, useParams, useLocation, useMatches, useNavigation } from 'react-router-dom';
7-41: LGTM! Minor formatting suggestion.The
withRouterhigher-order component is well-implemented and provides a good abstraction for adding routing props to other components. It correctly handles both function and element components.Consider removing extra spaces in the parameter destructuring for consistency:
- let params = useParams(); + let params = useParams();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/Dashboard/index.tsx (1 hunks)
- src/Routing/index.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Dashboard/index.tsx
🧰 Additional context used
🔇 Additional comments (2)
src/Routing/index.tsx (2)
70-70: LGTM! Default export is correct.The default export of the
getRoutesfunction is appropriate and follows good module design practices.
43-68:⚠️ Potential issueAddress TypeScript errors and ensure consistent path prefixes.
The paths in the route definitions are inconsistent. Some start with '/' while others don't. This can lead to unexpected routing behavior.
The use of
@ts-ignoresuppresses TypeScript errors, which can hide potential issues. Instead, properly type thewpobject.To resolve these issues:
- Make the paths consistent by adding a leading '/' to all paths:
- path: 'settings', + path: '/settings',
- Remove the
@ts-ignorecomment and properly type thewpobject. You can declare the type forwpor extend theWindowinterface:declare global { interface Window { wp: { hooks: { applyFilters: (filterName: string, value: any, ...args: any[]) => any; }; }; } } routes = window.wp.hooks.applyFilters('dokan-dashboard-routes', routes) as Array<DokanRoute>;These changes will improve type safety and consistency in your routing logic.
…ure' into update/vendor-dashboard-structure
…e' into update/vendor-dashboard-structure
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/Layout/Header.tsx (2)
7-18: LGTM: Well-structured and flexible component rendering.The component's structure is well-organized and utilizes Tailwind CSS for responsive styling. The use of Slot components for extensibility is a great practice. The conditional rendering of the title and passing of the navigate function to the header actions slot are implemented correctly.
Consider adding an
aria-labelto the outerdivto improve accessibility:- <div className="flex items-center sm:flex-wrap md:flex-nowrap"> + <div className="flex items-center sm:flex-wrap md:flex-nowrap" aria-label="Page header">
1-21: Overall: Excellent implementation of the Header component.This Header component is well-implemented and aligns perfectly with the PR objectives of setting up a skeleton for the new vendor dashboard. It provides a flexible and extensible structure, utilizing WordPress components and React Router for navigation. The use of Slot components allows for easy customization, which is crucial for a dashboard layout.
The component follows React best practices, uses modern functional component syntax, and leverages Tailwind CSS for responsive styling. This implementation sets a solid foundation for further development of the dashboard layout.
As you continue developing the dashboard, consider the following:
- Implement unit tests for this component to ensure its behavior remains consistent as the project evolves.
- Document the usage of the Slot components to guide other developers on how to extend the header functionality.
- Consider creating a theme context if you plan to implement dark mode or other theme variations in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/Layout/Header.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (4)
src/Layout/Header.tsx (4)
1-2: LGTM: Imports are appropriate and necessary.The imports of
Slotfrom@wordpress/componentsanduseNavigatefromreact-router-domare well-chosen for the component's requirements. They align with the WordPress ecosystem and provide necessary routing functionality.
4-4: LGTM: Component definition is clear and follows best practices.The
Headercomponent is well-defined as a functional component with a descriptive name. The use of default prop value fortitleis a good practice for handling optional props.
5-5: LGTM: Proper use of the useNavigate hook.The
useNavigatehook is correctly used and stored in a well-named constant. This setup will allow for programmatic navigation within the component.
21-21: LGTM: Appropriate default export.The default export of the Header component is correct and follows common React component export patterns.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
src/Layout/index.tsx (1)
25-44: Add JSDoc documentation and improve type consistency.The type definitions would benefit from documentation and consistent optionality.
+/** + * Represents a route configuration in the Dokan dashboard + */ export type DokanRoute = { + /** Unique identifier for the route */ id: string; + /** Display title for the route */ title?: string; - icon?: JSX.Element | React.ReactNode; + /** Icon component to display (React.ReactNode includes JSX.Element) */ + icon?: React.ReactNode; - element: JSX.Element | React.ReactNode; + /** Component to render for this route */ + element: React.ReactNode; // ... rest of the type }; +/** + * Props for the Layout component + */ interface LayoutProps { children: React.ReactNode; route: DokanRoute; title?: string; - headerComponent?: JSX.Element|React.ReactNode; - footerComponent?: JSX.Element|React.ReactNode; + headerComponent?: React.ReactNode; + footerComponent?: React.ReactNode; }package.json (2)
64-64: Consider code-splitting for React Router.The addition of
react-router-domis appropriate for dashboard navigation. However, ensure route-based code splitting is implemented to optimize initial load performance.Recommendations:
- Use React.lazy() for route components
- Implement Suspense boundaries
- Consider implementing progressive loading for dashboard sections
Line range hint
25-64: Review bundle size impact.The addition of multiple new dependencies could significantly increase the bundle size. Consider implementing appropriate optimization strategies.
Recommendations:
- Implement tree-shaking for all new dependencies
- Consider using dynamic imports for less frequently used components
- Monitor the bundle size impact using webpack-bundle-analyzer
- Implement chunking strategies to optimize loading performance
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- package.json (3 hunks)
- src/Layout/index.tsx (1 hunks)
🔇 Additional comments (4)
src/Layout/index.tsx (1)
1-10: LGTM! Well-organized imports following WordPress conventions.The imports are properly structured, using WordPress-specific packages where appropriate.
package.json (3)
57-63: Verify WordPress package versions compatibility.The WordPress packages being added (
@wordpress/components,@wordpress/data, etc.) are at different major versions (v28.x vs v4.x vs v7.x). This might lead to compatibility issues.#!/bin/bash # Check for potential version conflicts in WordPress packages echo "Checking @wordpress/* package usage..." rg -l "@wordpress" | xargs cat | rg "from '@wordpress/"
56-56: Verify @getdokan/dokan-ui compatibility.The addition of
@getdokan/dokan-ui(v1.0.14) is a significant change. Ensure it's compatible with the WordPress admin environment and follows the project's design system.#!/bin/bash # Check for potential UI component usage echo "Checking dokan-ui component usage..." rg -l "@getdokan/dokan-ui" | xargs cat | rg "from '@getdokan/dokan-ui"
25-26: Verify Tailwind CSS configuration to prevent style conflicts.The addition of Tailwind CSS utilities (
@tailwindcss/forms,@tailwindcss/typography,tailwindcss-scoped-preflight) is appropriate for a modern React dashboard. However, ensure proper scoping to prevent conflicts with WordPress admin styles.Consider the following recommendations:
- Use
tailwindcss-scoped-preflightto scope Tailwind's base styles- Configure content paths in
tailwind.config.jsto minimize bundle size- Add appropriate CSS cascade layers to manage style precedence
Also applies to: 39-39
…er-api' into update/vendor-dashboard-structure # Conflicts: # includes/REST/Manager.php
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
…shboard-structure
…ure' into update/vendor-dashboard-structure
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
includes/Admin/Status/Status.php (1)
61-76: 🛠️ Refactor suggestionAdd version fallback for assets
The asset version should have a fallback in case the asset file
dokan-status.asset.phpis not found. This will prevent errors when the asset file is missing, especially in development environments where assets might not be built yet.Apply this diff to include a version fallback:
public function register_scripts() { - $asset_file = include DOKAN_DIR . '/assets/js/dokan-status.asset.php'; + $asset_file = file_exists( DOKAN_DIR . '/assets/js/dokan-status.asset.php' ) + ? include DOKAN_DIR . '/assets/js/dokan-status.asset.php' + : [ 'dependencies' => [], 'version' => DOKAN_VERSION ]; wp_register_script( 'dokan-status',
🧹 Nitpick comments (1)
includes/Admin/Status/Status.php (1)
104-146: Remove commented-out code from thedescribemethodThe
describe()method currently contains only commented-out code. Leaving large blocks of commented-out code can clutter the codebase and reduce readability. If this code is not needed, consider removing it. If it's intended for future development, consider using feature flags or adding TODO comments with clear explanations.
|
|
||
| wp_register_script( | ||
| 'dokan-status', | ||
| DOKAN_PLUGIN_ASSEST . '/js/dokan-status.js', |
There was a problem hiding this comment.
Fix typo in constant name 'DOKAN_PLUGIN_ASSEST'
The constant DOKAN_PLUGIN_ASSEST appears to be misspelled. It should likely be DOKAN_PLUGIN_ASSET (singular) or DOKAN_PLUGIN_ASSETS (plural). Using an undefined constant will result in an error.
Apply this diff to correct the constant name:
'dokan-status',
- DOKAN_PLUGIN_ASSEST . '/js/dokan-status.js',
+ DOKAN_PLUGIN_ASSETS . '/js/dokan-status.js',
$asset_file['dependencies'],
$asset_file['version'],
[Similarly, update line 74:
- wp_register_style( 'dokan-status', DOKAN_PLUGIN_ASSEST . '/css/dokan-status.css', [], $asset_file['version'] );
+ wp_register_style( 'dokan-status', DOKAN_PLUGIN_ASSETS . '/css/dokan-status.css', [], $asset_file['version'] );Please ensure that the constant DOKAN_PLUGIN_ASSETS is defined correctly in your codebase.
Also applies to: 74-74
| if ( 'dokan_page_dokan-status' !== get_current_screen()->id ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Check for null return value from get_current_screen()
The function get_current_screen() may return null in certain circumstances. Accessing ->id on a null value will cause a fatal error. To prevent this, you should check if the result is not null before accessing ->id.
Apply this diff to add a null check:
public function enqueue_scripts() {
+ $screen = get_current_screen();
+ if ( ! $screen || 'dokan_page_dokan-status' !== $screen->id ) {
return;
}
- if ( 'dokan_page_dokan-status' !== get_current_screen()->id ) {
- return;
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( 'dokan_page_dokan-status' !== get_current_screen()->id ) { | |
| return; | |
| } | |
| $screen = get_current_screen(); | |
| if ( ! $screen || 'dokan_page_dokan-status' !== $screen->id ) { | |
| return; | |
| } |
| public function escape_data( string $data ): string { | ||
| return $data; | ||
| } |
There was a problem hiding this comment.
Ensure data is properly escaped to prevent security vulnerabilities
The escape_data method currently returns data without any escaping, which may lead to security vulnerabilities like XSS attacks if the data is output directly to the browser. Consider implementing appropriate data escaping using WordPress functions such as esc_html() or esc_attr(), depending on the context where the data will be used.
Apply this diff to escape the data properly:
public function escape_data( string $data ): string {
- return $data;
+ return esc_html( $data );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function escape_data( string $data ): string { | |
| return $data; | |
| } | |
| public function escape_data( string $data ): string { | |
| return esc_html( $data ); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
Line range hint
25-69: Consider reorganizing package dependencies.A few suggestions to improve package organization:
- Move development-only packages to devDependencies (e.g., Tailwind packages)
- Consider separating Vue and React dependencies into different workspace packages if they're not used together
- Ensure consistent versioning strategy across related packages (e.g., all @wordpress/* packages)
🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Package dependencies and lock file are out of sync. Multiple missing and invalid dependencies detected including @dnd-kit/[email protected], @wordpress/[email protected], and others.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: E2E_API Tests
package.json
[error] Package dependencies and lock file are out of sync. Multiple missing and invalid dependencies detected including @dnd-kit/[email protected], @wordpress/[email protected], and others.
🔇 Additional comments (3)
package.json (3)
58-58: LGTM! @getdokan/dokan-ui version is up to date.The package has been updated to version 1.0.18, which is newer than the previously suggested version 1.0.17.
🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Package dependencies and lock file are out of sync. Multiple missing and invalid dependencies detected including @dnd-kit/[email protected], @wordpress/[email protected], and others.
56-57:⚠️ Potential issueAdd missing @dnd-kit dependencies.
The pipeline is failing due to incomplete @dnd-kit package dependencies. Add the following packages:
- @dnd-kit/[email protected]
- @dnd-kit/[email protected]
Apply this diff to add the missing dependencies:
"@dnd-kit/core": "^6.3.1", "@dnd-kit/sortable": "^10.0.0", + "@dnd-kit/accessibility": "^3.1.1", + "@dnd-kit/utilities": "^3.2.2",Likely invalid or redundant comment.
🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Package dependencies and lock file are out of sync. Multiple missing and invalid dependencies detected including @dnd-kit/[email protected], @wordpress/[email protected], and others.
27-27: Verify compatibility with the major version update of @wordpress/scripts.The update from version ^27.9.0 to ^30.7.0 is a significant jump that might introduce breaking changes. Please ensure all build scripts and configurations are compatible with the new version.
✅ Verification successful
The @wordpress/scripts update to ^30.7.0 appears safe to proceed with
The webpack configuration follows best practices for extending @wordpress/scripts, and the build process completes without any related errors. The custom configuration properly extends the default setup without overriding core functionality, which maintains compatibility across major versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for breaking changes in build output npm install npm run build 2>&1 | grep -i "error\|warning\|deprecated"Length of output: 1894
Script:
#!/bin/bash # Check @wordpress/scripts changelog for breaking changes between v27 and v30 curl -s "https://raw.githubusercontent.com/WordPress/gutenberg/trunk/packages/scripts/CHANGELOG.md" | awk '/## \[30\./ || /## \[29\./ || /## \[28\./ {p=1}; p; /## \[27\./{p=0}'Length of output: 176
Script:
#!/bin/bash # Check for webpack config and build scripts usage echo "=== Build Configuration Files ===" fd -e js -e json -e cjs webpack.config fd -e js -e json -e cjs babel.config echo -e "\n=== Package.json Scripts Section ===" cat package.json | jq -r '.scripts // empty'Length of output: 1316
Script:
#!/bin/bash # Check webpack configuration content echo "=== Webpack Configuration ===" cat webpack.config.jsLength of output: 2886
🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Package dependencies and lock file are out of sync. Multiple missing and invalid dependencies detected including @dnd-kit/[email protected], @wordpress/[email protected], and others.
* feat: introduce dataviews from dokan free. * feat: introduce dataviews from dokan free. * revert: add dokan category comission js. * update: remove category commission js updates. * remove: revert component package js file. * remove: dokan sidebar class from data view table. * enhance: update dataviews doc. * update: filter naming convension for dataviews property. * remove: module federation node package from json file. * remove: example dataviews table usage component. * remove: example dataviews table usage component. * update: button class name for color scheme customizer. * update: split webpack entries, use change-case for namespace cases, handle responsive preview from base components. * update: introduce utilities directory, make exporter from dokan free & add documentation for components, utilities. * update: routing folder naming convesional stuff. * update: add component and utilities accessor doc for dokan free. * Add router param add dataview table id * fix: add file exists check before register components scripts. * revert: remove default config from webpack entries. * Filter component and documentation added. * Filter component and documentation added. * Filter component and documentation added. * Filter component and documentation added. --------- Co-authored-by: MdAsifHossainNadim <[email protected]> Co-authored-by: Md. Asif Hossain Nadim <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/hooks/ViewportDimensions.ts (3)
1-2: Consider direct React imports if not strictly tied to WordPress.
While importing from@wordpress/elementis perfectly valid for WP contexts, if your application can run standalone, direct imports from React (e.g.,react) may be more conventional.
8-14: Fill in the@since DOKAN_PRO_SINCEtag with a valid version.
The doc block placeholder should be replaced with the appropriate version or release milestone.
23-43: Enable cleanup forrequestAnimationFramecalls.
While usingrequestAnimationFrameis an excellent way to throttle the resize updates, consider storing its return ID and cancelling it in the cleanup function. This extra step can be beneficial in cases where unmounting might occur mid-frame, preventing potential memory leaks and ensuring a smoother teardown.+ let rafId: number | null = null; const handleResize = () => { - window.requestAnimationFrame(() => { + rafId = window.requestAnimationFrame(() => { setViewport(getViewportDimensions()); }); }; return () => { + if (rafId) { + cancelAnimationFrame(rafId); + } window.removeEventListener('resize', handleResize); };src/components/Filter.tsx (2)
4-6: Avoid relying on TS ignore and eslint disable if possible
Consider fixing the underlying type definition or updating your import resolve configuration instead of relying on@ts-ignoreandeslint-disable-next-line import/no-unresolved. This will prevent potential hidden errors and improve maintainability.
49-55: Consider using a stable key instead of index for mapped fields
When rendering lists, using the array index as a key can lead to unexpected behavior if the list items change order. For better stability and future-proofing, provide a unique value or stable identifier for each mapped item.docs/frontend/filter.md (1)
14-14: Fix heading level for proper markdown structure
Heading levels should increment by one. Change line 14 from####to###:-#### Dokan lite +### Dokan lite🧰 Tools
🪛 Markdownlint (0.37.0)
14-14: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/frontend/filter.md(1 hunks)package.json(3 hunks)src/components/Filter.tsx(1 hunks)src/components/index.tsx(1 hunks)src/hooks/ViewportDimensions.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🪛 Markdownlint (0.37.0)
docs/frontend/filter.md
14-14: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
🔇 Additional comments (9)
src/hooks/ViewportDimensions.ts (3)
3-6: Interface definition is clear and concise.
This type definition covers both SSR (null) and runtime usage for window dimensions.
16-19: Smart SSR-safe dimension initialization.
Using a null check forwindoweffectively prevents crashes in SSR environments. This approach is well-implemented.
45-46: Overall logic validated.
The hook returns the correct viewport dimensions in a predictable manner.src/components/Filter.tsx (3)
8-23: Interface definition is clear and well-organized
The inline JSDoc-style comments and descriptive prop names make yourFilterPropsinterface easy to understand and maintain. Great job!
25-33: Good use of default props and destructuring
Using default property values and object destructuring for the component props is a neat approach that keeps the code concise and ensures consistent behavior.
37-39: Verify availability of the globalwpobject
Becausewp.hooks.applyFiltersrelies on the globalwpnamespace, please ensure that this component is only used in a WordPress environment that provideswp.hooks. Otherwise, a reference error may occur.src/components/index.tsx (2)
1-3: Re-exporting default components
The approach to re-exportDataViewsandSortableListis straightforward and provides clarity for consumers of this module. Good practice!
11-11: Consistent export of theFiltercomponent
ExportingFilterhere aligns well with your other component exports, ensuring consistent usage patterns across the codebase.docs/frontend/filter.md (1)
28-40: Well-structured usage example
Your example code snippet clearly demonstrates how to integrate theFiltercomponent with custom fields, making it easier for developers to follow best practices and implement filtering in their own projects.
| export { | ||
| DataForm, | ||
| VIEW_LAYOUTS, | ||
| filterSortAndPaginate, | ||
| isItemValid | ||
| } from '@wordpress/dataviews/wp'; |
There was a problem hiding this comment.
💡 Codebase verification
Potential issue with VIEW_LAYOUTS export
Three exports (DataForm, filterSortAndPaginate, isItemValid) exist in @wordpress/dataviews, but VIEW_LAYOUTS appears to be mismatched with VIE_LAYOUTS in the package. Please verify the correct spelling or version of the package you're using.
🔗 Analysis chain
Confirm named exports exist in @wordpress/dataviews/wp
Double-check that DataForm, VIEW_LAYOUTS, filterSortAndPaginate, and isItemValid are valid named exports from @wordpress/dataviews/wp to avoid runtime errors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for references to these named exports in the codebase
rg -A 5 "DataForm|VIEW_LAYOUTS|filterSortAndPaginate|isItemValid"
Length of output: 398
Script:
#!/bin/bash
# Check if @wordpress/dataviews is in package.json
cat package.json | grep -A 5 -B 5 "@wordpress/dataviews"
# Try to find these exports in node_modules
fd -t f "index.js|index.ts|index.tsx" -p node_modules/@wordpress/dataviews
# As fallback, let's check the WordPress GitHub repository
gh api repos/WordPress/gutenberg/contents/packages/dataviews/src/index.ts
Length of output: 1730
Script:
#!/bin/bash
# Decode the base64 content of index.ts
python3 -c "
import json
import base64
# Get the content from the previous output
content = 'ZXhwb3J0IHsgZGVmYXVsdCBhcyBEYXRhVmlld3MgfSBmcm9tICcuL2NvbXBvbmVudHMvZGF0YXZpZXdzJzsKZXhwb3J0IHsgZGVmYXVsdCBhcyBEYXRhRm9ybSB9IGZyb20gJy4vY29tcG9uZW50cy9kYXRhZm9ybSc7CmV4cG9ydCB7IFZJRV9MQVlPVVRTIH0gZnJvbSAnLi9kYXRhdmlld3MtbGF5b3V0cyc7CmV4cG9ydCB7IGZpbHRlclNvcnRBbmRQYWdpbmF0ZSB9IGZyb20gJy4vZmlsdGVyLWFuZC1zb3J0LWRhdGEtdmlldyc7CmV4cG9ydCB0eXBlICogZnJvbSAnLi90eXBlcyc7CmV4cG9ydCB7IGlzSXRlbVZhbGlkIH0gZnJvbSAnLi92YWxpZGF0aW9uJzsK'
# Decode and print
decoded = base64.b64decode(content).decode('utf-8')
print(decoded)
"
Length of output: 941
* Adding withdraw page. * wi * Balance and payment methods done * List page done. * List and pagination done. * Added slot fill in PaymentDetails.tsx * feat: introduce dataviews from dokan free. * feat: introduce dataviews from dokan free. * revert: add dokan category comission js. * update: remove category commission js updates. * remove: revert component package js file. * remove: dokan sidebar class from data view table. * enhance: update dataviews doc. * update: filter naming convension for dataviews property. * remove: module federation node package from json file. * remove: example dataviews table usage component. * remove: example dataviews table usage component. * update: button class name for color scheme customizer. * update: split webpack entries, use change-case for namespace cases, handle responsive preview from base components. * update: introduce utilities directory, make exporter from dokan free & add documentation for components, utilities. * update: routing folder naming convesional stuff. * update: add component and utilities accessor doc for dokan free. * Integrate wp data view. * Add router param add dataview table id * fix: add file exists check before register components scripts. * revert: remove default config from webpack entries. * Add support for template fo dokan status. * Dokan currency localize data added * Rename directory * refactor: change route id * refactor: replace user current user api call by wp data refactor: use hook from @/hooks * refactor: removed useless codes. refactor: removed unnecessery types * refactor: revert the query * add formatPrice.ts utility function. * date and price component added in dokan component. * Fix dokan lite dependency. * Fix dokan lite dependency. * Fix dokan lite dependency. * Fix Accounting.ts library check * Fix Accounting.ts library check --------- Co-authored-by: MdAsifHossainNadim <[email protected]> Co-authored-by: Md. Asif Hossain Nadim <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (1)
package.json (1)
Line range hint
1-77: Update package-lock.json to match package.json.The pipeline is failing because package-lock.json is out of sync with package.json. Run
npm installto update package-lock.json with the latest dependencies.🧰 Tools
🪛 Biome (1.9.4)
[error] 60-60: The key @wordpress/api-fetch was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
[error] 61-61: The key @wordpress/components was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
[error] 62-62: The key @wordpress/data was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
[error] 63-63: The key @wordpress/url was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🪛 GitHub Actions: E2E_API Tests
[error] Package lock file is out of sync with package.json. Multiple dependencies are missing or have version mismatches in the lock file, including @dnd-kit/[email protected], @dnd-kit/[email protected], and others.
♻️ Duplicate comments (1)
includes/Dashboard/Templates/NewDashboard.php (1)
98-115: 🛠️ Refactor suggestionImprove portal script reliability.
The inline script might fail if the portal element isn't loaded when the script runs. Consider adding error handling and potentially using MutationObserver for reliability.
<script> (function() { - const portal = document.getElementById( 'headlessui-portal-root' ); - if ( portal ) { - portal.classList.add( 'dokan-layout' ); - } + // Use MutationObserver to handle dynamic portal creation + const observer = new MutationObserver((mutations, obs) => { + const portal = document.getElementById('headlessui-portal-root'); + if (portal) { + portal.classList.add('dokan-layout'); + obs.disconnect(); // Stop observing once we've found and modified the portal + } + }); + + observer.observe(document.body, { + childList: true, + subtree: true + }); })(); </script>
🧹 Nitpick comments (43)
src/dashboard/Withdraw/tailwind.scss (4)
1-1: Fix typo in mixin name.The mixin name contains a typo:
windraw-resetshould bewithdraw-reset.-@mixin windraw-reset { +@mixin withdraw-reset {
28-39: Optimize table reset styles.The
box-sizingdeclaration is redundant as it's typically set globally in Tailwind's base styles. Consider removing it to avoid specificity conflicts.table, th, td { margin: 0; padding: 0; border: 0; border-spacing: 0; border-collapse: collapse; font-size: inherit; font-weight: inherit; text-align: inherit; vertical-align: inherit; - box-sizing: border-box; }
60-68: Document the purpose of hiding view actions.Add a comment explaining why the view actions are hidden in this context to improve code maintainability.
#dokan-withdraw-request-data-view { &.dokan-dashboard-datatable { .dataviews-wrapper { + // Hide view actions as they are handled by custom withdraw request controls .dataviews__view-actions { display: none; } } } }
70-72: Consider import order for proper CSS cascade.The current import order looks good, ensuring that Tailwind's base styles can be overridden by Dokan UI's custom styles. However, add a comment explaining this intentional ordering to prevent future reordering.
@config './../../../base-tailwind.config.js'; +// Import order matters: base styles first, then Dokan UI overrides @import '../../base-tailwind'; @import "@getdokan/dokan-ui/dist/dokan-ui.css";includes/VendorNavMenuChecker.php (5)
12-12: Replace placeholder version constant.The
@since DOKAN_SINCEdocblock tag contains a placeholder version number.Replace
DOKAN_SINCEwith the actual version number where this feature was introduced.
17-30: Consider making template dependencies configurable.The hardcoded template dependencies array might be better suited as a configuration file, especially since it may need frequent updates as new routes are added.
Consider moving the template dependencies to a separate configuration file:
- protected array $template_dependencies = [ - 'withdraw' => [ - [ 'slug' => 'withdraw/withdraw-dashboard' ], - [ 'slug' => 'withdraw/withdraw' ], - [ 'slug' => 'withdraw/header' ], - [ 'slug' => 'withdraw/status-listing' ], - [ 'slug' => 'withdraw/pending-request-listing' ], - [ 'slug' => 'withdraw/approved-request-listing' ], - [ 'slug' => 'withdraw/cancelled-request-listing' ], - [ 'slug' => 'withdraw/tmpl-withdraw-request-popup' ], - [ 'slug' => 'withdraw/request-form' ], - [ 'slug' => 'withdraw/pending-request-listing-dashboard' ], - ], - ]; + protected array $template_dependencies; + + public function __construct() { + $this->template_dependencies = require_once __DIR__ . '/../config/template-dependencies.php'; + // ... rest of the constructor + }
99-114: Improve dependency resolution logic.The dependency resolution logic could be more robust:
- The variable name
$clearis not descriptive of its purpose- The method mutates state by modifying
$forcefully_resolved_dependenciesConsider these improvements:
protected function is_dependency_resolved( string $route ): bool { - $clear = true; + $is_resolved = true; $dependencies = $this->get_template_dependencies_resolutions(); if ( ! empty( $dependencies[ trim( $route, '/' ) ] ) ) { - $clear = false; + $is_resolved = false; } - $filtered_clear = apply_filters( 'dokan_is_dashboard_nav_dependency_resolved', $clear, $route ); + $filtered_resolution = apply_filters( 'dokan_is_dashboard_nav_dependency_resolved', $is_resolved, $route ); - if ( $clear !== $filtered_clear ) { - $this->forcefully_resolved_dependencies[ $route ] = $filtered_clear; + if ( $is_resolved !== $filtered_resolution ) { + $this->record_forceful_resolution($route, $filtered_resolution); } - return $filtered_clear; + return $filtered_resolution; } + + protected function record_forceful_resolution(string $route, bool $resolution): void { + $this->forcefully_resolved_dependencies[$route] = $resolution; + }
314-314: Update duplicate table titles.The title "Overridden Template Table" is used for both tables, which could be confusing.
Update the second table's title to be more specific:
- ->set_title( __( 'Overridden Template Table', 'dokan-lite' ) ) + ->set_title( __( 'Overridden Routes Table', 'dokan-lite' ) )Also applies to: 287-287
277-358: Consider breaking down theadd_status_sectionmethod.The
add_status_sectionmethod is quite long (82 lines) and handles multiple responsibilities. This makes it harder to maintain and test.Consider breaking it down into smaller, focused methods:
+ private function create_template_table(array $overridden_templates): StatusElementInterface { + $table = StatusElementFactory::table('override_templates_table') + ->set_title(__('Overridden Template Table', 'dokan-lite')) + ->set_headers([__('Template', 'dokan-lite')]); + + foreach ($overridden_templates as $id => $template) { + $table->add($this->create_template_row($id, $template)); + } + + return $table; + } + + private function create_route_table(array $overridden_routes): StatusElementInterface { + $table = StatusElementFactory::table('override_features_table') + ->set_title(__('Overridden Routes Table', 'dokan-lite')) + ->set_headers([ + __('Route', 'dokan-lite'), + __('Override Status', 'dokan-lite'), + ]); + + foreach ($overridden_routes as $route => $clearance) { + $table->add($this->create_route_row($route, $clearance)); + } + + return $table; + }This would make the main method more concise and easier to understand.
includes/Dashboard/Templates/NewDashboard.php (2)
1-34: Enhance class documentation.The class documentation needs improvement in several areas:
- Replace placeholder
DOKAN_SINCEwith actual version number- Add a class-level docblock describing the purpose and responsibility of this class
- Add proper param/return tags to the constructor docblock
Apply these documentation improvements:
+/** + * Handles the new React-based dashboard template implementation. + * + * @since 3.0.0 + */ class NewDashboard { /** * Class constructor * - * @since DOKAN_SINCE + * @since 3.0.0 + * + * @return void */ public function __construct() {
45-48: Consider more robust query var handling.The current implementation uses 'new' as both key and value. Consider using a more descriptive value and adding input validation.
public function add_query_var( $query_vars ) { - $query_vars['new'] = 'new'; + $query_vars['new'] = 'react_dashboard'; return $query_vars; }src/hooks/useCurrentUser.ts (3)
3-7: Consider refining theLinksinterface.
Right now, thehrefandtargetHintsproperties are typed as generic objects and arrays. If these fields have a known structure, specifying more detailed interfaces (e.g.,interface TargetHints { allow: string[] }) can improve type safety and readability.
22-27: Useundefinedinstead ofnullfor errors if possible.
React patterns often preferundefinedovernull. This is optional but can help maintain consistency with common TypeScript and React conventions.
29-31: Rename theenabledparameter for clarity.
Althoughenabledworks, naming itshouldFetchorfetchUsermight be more descriptive, clarifying its purpose at call sites.src/definitions/window-types.ts (1)
27-28: Remove the redundant empty export
Since the presence of imports already marks the file as a module, the empty export statement is unnecessary and can be removed:-// This empty export is necessary to make this a module -export {};🧰 Tools
🪛 Biome (1.9.4)
[error] 27-28: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
src/components/PriceHtml.tsx (1)
4-11: Align default value ofprecisionwith its declared type
Currently,precisionis typed asnumber | undefinedbut defaults tonull. Consider either addingnullto the type definition or providing a numeric default to avoid potential type mismatches whenstrictNullChecksare enabled.type PriceHtmlProps = { price: string | number; currencySymbol?: string; - precision?: number; + precision?: number | null; thousand?: string; decimal?: string; format?: PriceFormat; };src/dashboard/Withdraw/Hooks/useMakeDefaultMethod.ts (1)
14-37: Provide a success feedback mechanism
The current implementation sets loading and captures errors but lacks any success feedback (e.g., toast notification, user-facing message). Consider informing the user upon successful completion ofmakeDefaultMethod.try { setIsLoading( true ); setError( null ); await apiFetch( { path: '/dokan/v2/withdraw/make-default-method', method: 'POST', data: { method }, } ); + // e.g., Show success notification: + // showSuccessNotice('Default withdrawal method updated successfully!'); } catch ( err ) { setError( err instanceof Error ? err : new Error( 'Failed to make default method' ) ); console.error( 'Error making default method:', err ); } finally { setIsLoading( false ); }src/components/DateTimeHtml.tsx (3)
5-24: Safely handle empty or invalid date
Currently, ifdateI18n()orgetSettings()fails, the component might break. Consider adding a fallback for these function calls or verifying that settings are always available before usage to prevent unhandled runtime errors.if ( ! date ) { return defaultDate; } +if ( ! getSettings()?.formats?.datetime ) { + return defaultDate; +} return ( <RawHTML> { dateI18n(🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Module not found: Cannot resolve '../Definitions/window-types' in '/home/runner/work/dokan/dokan/src/components'
26-45: Reduce repeated logic betweenDateTimeHtml.Dateand the mainDateTimeHtml
DateTimeHtmlandDateTimeHtml.Datecontain similar code for date handling. Consider extracting reusable formatting logic into a shared function to avoid duplication.🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Module not found: Cannot resolve '../Definitions/window-types' in '/home/runner/work/dokan/dokan/src/components'
46-65: Match structure inDateTimeHtml.Time
Similar to the above,DateTimeHtml.Timemostly replicates the pattern used inDateTimeHtml.Date. A shared utility function or parameterized approach could make maintenance easier.🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Module not found: Cannot resolve '../Definitions/window-types' in '/home/runner/work/dokan/dokan/src/components'
src/dashboard/Withdraw/Hooks/useCharge.ts (1)
22-54: Good fetch pattern with loading and error states
Overall, the logic for fetching the charge data is consistent and includes robust error handling. Consider logging themethodandamountwhen errors occur, to aid debugging.catch ( err ) { + console.error( 'Error fetching charge for method:', method, 'amount:', amount ); setError( err instanceof Error ? err : new Error( 'Failed to fetch charge' ) );src/utilities/Accounting.ts (1)
51-73: Enhance type definitions forformatNumber
Currently,formatNumberaccepts ananytype. For stronger type safety, consider specifying a type signature that narrows the accepted values (e.g.,number | string).-export const formatNumber = (value) => { +export function formatNumber(value: number | string): number | string { if ( value === '' ) { return value; } // ... }src/dashboard/Withdraw/Hooks/useBalance.ts (1)
4-23: Consider making data structures more strictly typed
"Record<any, any>" might cause a loss of type safety. If the object structure is known, defining a more precise interface could improve maintainability and reduce runtime errors.src/dashboard/Withdraw/index.tsx (1)
29-61: Extract repeated loading checks into a utility variable
The repeated expressionscurrentUser.isLoading || useWithdrawRequestHook.isLoadingcould be simplified to a single variable, enhancing readability.- <Balance - masterLoading={ - currentUser.isLoading || - useWithdrawRequestHook.isLoading - } - ... - /> + const masterLoading = currentUser.isLoading || useWithdrawRequestHook.isLoading; + + <Balance + masterLoading={ masterLoading } + ... + />src/dashboard/Withdraw/Hooks/useWithdraw.ts (3)
4-8: Enforce consistent numeric/decimal usage for withdrawal amounts
If the system always treats amounts as decimals, consider specifying decimal constraints or using a library for currency parsing to ensure consistent handling of amounts.
9-10: Check whether you can derive a narrower type than Record<any, any>
A more descriptive type for the data structure being updated can help prevent runtime errors and improve readability.
51-78: Ensure concurrency safety for updates
If multiple updates can happen in quick succession, ensure that the state updates correctly reflect the latest request. A quick solution might be to store request tokens or use an abort controller.src/definitions/woocommerce-accounting.d.ts (2)
19-31: Consider using explicit union or string literal types instead of generic strings
If you have known possible decimal or thousand separators, restrict them to known characters (e.g., "." or ","). This helps prevent invalid user input or unexpected formatting.
32-110: Centralize business logic
These formatting methods may be reused in multiple components. Ensure consistent usage of the same locale or currency context to avoid region-specific mismatches.src/dashboard/Withdraw/Balance.tsx (1)
49-98: Ensure consistent error boundaries.
Currently, ifbodyData.datais undefined or incorrectly shaped, the component might fail silently. Consider adding an error boundary or fallback handling for missing data fields, so users see a graceful error message or fallback UI instead of a blank screen or JavaScript error.src/dashboard/Withdraw/Hooks/useWithdrawRequests.ts (3)
45-56: Handle race conditions for rapid calls.
When multiple components or user actions triggerfetchWithdrawRequestsin quick succession, consider advanced handling (e.g., request cancellation or a concurrency check) to prevent overlapping requests. This is especially relevant for unreliable or slow networks.
61-75: Use structured clone or direct assignment.
ReplacingObject.assign({}, lastPayload.current, payload)might mask or overwrite keys. Consider using structured clones (e.g., the spread operator) or verifying key merges to ensure no accidental overwrites:-if (lastPayload.current) { - lastPayload.current = Object.assign( - {}, - lastPayload.current, - payload - ); -} else { - lastPayload.current = payload; -} +lastPayload.current = lastPayload.current + ? { ...lastPayload.current, ...payload } + : payload;
95-101: Consider external error logging.
The console error is helpful for debugging, but in production, it might be beneficial to integrate with a logging or monitoring service (e.g., Sentry) for better visibility of errors.includes/REST/WithdrawControllerV2.php (2)
90-107: Expand validations for payment method data.
When returningactive_methods, consider verifying the method data beyond a simple label/value check, such as ensuring the user has properly configured payment fields. This can help avoid edge cases where a partially set up method is incorrectly displayed as active.
124-148: Success response inclusion of updated default method.
It might be beneficial to return the updated default method in the success response so the client can update UI state automatically without making another request.-return new WP_REST_Response( __( 'Default method update successful.', 'dokan-lite' ), 200 ); +return new WP_REST_Response( + [ + 'message' => __( 'Default method update successful.', 'dokan-lite' ), + 'default_method' => $method, + ], + 200 +);src/dashboard/Withdraw/WithdrawRequests.tsx (1)
53-122: Tooltip or help text for user actions.
For example, "Pending Requests" and "Approved Requests" might benefit from brief tooltips clarifying each state. This can improve clarity if your user base may be unfamiliar with the specific status flows of withdrawal requests.src/dashboard/Withdraw/PaymentDetails.tsx (1)
44-49: UseObject.hasOwn()instead of accessing.hasOwnPropertyfrom the instance.Static analysis warns about using
Object.prototype.hasOwnPropertyon an object instance. Replace it withObject.hasOwn()or an alternative check like'isLoading' in bodyData.- if ( - ! bodyData || - ! bodyData.hasOwnProperty( 'isLoading' ) || - bodyData.isLoading || - masterLoading - ) { + if ( + ! bodyData || + ! Object.hasOwn( bodyData, 'isLoading' ) || + bodyData.isLoading || + masterLoading + ) {🧰 Tools
🪛 Biome (1.9.4)
[error] 46-46: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
src/dashboard/Withdraw/PaymentMethods.tsx (1)
109-116: UseObject.hasOwn()instead of accessing.hasOwnPropertyfrom the instance.Static analysis flags this as a potential risk. Consider using
Object.hasOwn()or an alternative property check.- if ( - ! bodyData || - ! bodyData.hasOwnProperty( 'isLoading' ) || - bodyData.isLoading || - masterLoading - ) { + if ( + ! bodyData || + ! Object.hasOwn( bodyData, 'isLoading' ) || + bodyData.isLoading || + masterLoading + ) {🧰 Tools
🪛 Biome (1.9.4)
[error] 111-111: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
src/dashboard/Withdraw/RequestList.tsx (1)
185-206: Correct the spelling of the function name.Renaming the function for clarity and consistency helps maintain code readability.
- const canclePendingRequest = () => { + const cancelPendingRequest = () => { withdrawHook .updateWithdraw( Number( cancelRequestId ), { status: 'cancelled', } ) .then( () => { ... }) ... }; ... - onClick={ canclePendingRequest } + onClick={ cancelPendingRequest }src/dashboard/Withdraw/RequestWithdrawBtn.tsx (1)
102-104: Prefer React Router navigation overwindow.location.href.To preserve SPA behavior and avoid full page reloads, consider using the
useNavigatehook from react-router instead of settingwindow.location.href.- onClick={ () => { - window.location.href = bodyData?.data?.setup_url; - } } + // Example usage: + import { useNavigate } from 'react-router-dom'; + ... + const navigate = useNavigate(); + ... + onClick={ () => { + navigate('/path-to-setup'); // or: navigate(bodyData?.data?.setup_url) + } }includes/REST/WithdrawController.php (1)
977-977: Ensure backward compatibility after switching to'number'.
Changing'type'from'string'to'number'should be accompanied by validations or conversions when older clients might still send string values.'amount' => [ 'required' => true, 'description' => __( 'Withdraw amount.', 'dokan-lite' ), - 'type' => 'string', + 'type' => 'number', 'context' => [ 'view', 'edit' ], ],webpack.config.js (1)
27-31: Global namespace approach.
library.type = 'window'exposes these modules on the globalwindowobject. Verify that this is intended and secure for your use case.src/routing/index.tsx (1)
9-42:withRouterHOC approach.
The higher-order component usage is correct, but be sure your application code callswithRouter()on actual component references rather than JSX elements. Wrapping JSX directly can create confusion.🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Multiple module resolution errors: Cannot resolve '../Layout/404', '../Dashboard/Withdraw', and '../Dashboard/Withdraw/WithdrawRequests'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
includes/Assets.php(4 hunks)includes/Dashboard/Templates/NewDashboard.php(1 hunks)includes/REST/WithdrawController.php(4 hunks)includes/REST/WithdrawControllerV2.php(4 hunks)includes/VendorNavMenuChecker.php(1 hunks)includes/functions-dashboard-navigation.php(3 hunks)package.json(3 hunks)src/components/DateTimeHtml.tsx(1 hunks)src/components/PriceHtml.tsx(1 hunks)src/components/index.tsx(1 hunks)src/dashboard/Withdraw/Balance.tsx(1 hunks)src/dashboard/Withdraw/Hooks/useBalance.ts(1 hunks)src/dashboard/Withdraw/Hooks/useCharge.ts(1 hunks)src/dashboard/Withdraw/Hooks/useMakeDefaultMethod.ts(1 hunks)src/dashboard/Withdraw/Hooks/useWithdraw.ts(1 hunks)src/dashboard/Withdraw/Hooks/useWithdrawRequests.ts(1 hunks)src/dashboard/Withdraw/Hooks/useWithdrawSettings.ts(1 hunks)src/dashboard/Withdraw/PaymentDetails.tsx(1 hunks)src/dashboard/Withdraw/PaymentMethods.tsx(1 hunks)src/dashboard/Withdraw/RequestList.tsx(1 hunks)src/dashboard/Withdraw/RequestWithdrawBtn.tsx(1 hunks)src/dashboard/Withdraw/WithdrawRequests.tsx(1 hunks)src/dashboard/Withdraw/index.tsx(1 hunks)src/dashboard/Withdraw/tailwind.scss(1 hunks)src/definitions/RouterProps.ts(1 hunks)src/definitions/window-types.ts(1 hunks)src/definitions/woocommerce-accounting.d.ts(1 hunks)src/hooks/index.tsx(1 hunks)src/hooks/useCurrentUser.ts(1 hunks)src/routing/index.tsx(1 hunks)src/utilities/Accounting.ts(1 hunks)src/utilities/index.ts(1 hunks)webpack.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/index.tsx
- src/utilities/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/dashboard/Withdraw/PaymentDetails.tsx
[error] 46-46: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
src/definitions/window-types.ts
[error] 27-28: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
src/dashboard/Withdraw/PaymentMethods.tsx
[error] 111-111: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
src/dashboard/Withdraw/Balance.tsx
[error] 43-43: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
package.json
[error] 60-60: The key @wordpress/api-fetch was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
[error] 61-61: The key @wordpress/components was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
[error] 62-62: The key @wordpress/data was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
[error] 63-63: The key @wordpress/url was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🪛 GitHub Actions: E2E_API Tests
src/components/DateTimeHtml.tsx
[error] Module not found: Cannot resolve '../Definitions/window-types' in '/home/runner/work/dokan/dokan/src/components'
src/routing/index.tsx
[error] Multiple module resolution errors: Cannot resolve '../Layout/404', '../Dashboard/Withdraw', and '../Dashboard/Withdraw/WithdrawRequests'
package.json
[error] Package lock file is out of sync with package.json. Multiple dependencies are missing or have version mismatches in the lock file, including @dnd-kit/[email protected], @dnd-kit/[email protected], and others.
🔇 Additional comments (41)
includes/VendorNavMenuChecker.php (5)
75-88: Add type validation for menu items.This is a duplicate of a previous review comment. The menu conversion logic should validate the structure of input items to prevent potential errors.
139-143: Sanitize route parameter for URL generation.This is a duplicate of a previous review comment. The route parameter should be sanitized before being used in URL generation to prevent XSS.
152-170: Add error handling for malformed dependencies.This is a duplicate of a previous review comment. The dependency resolution could fail if the dependency array structure is invalid.
183-215: Sanitize template paths to prevent directory traversal.This is a duplicate of a previous review comment. The template path construction should be sanitized to prevent potential directory traversal.
243-243: Add notice for forcefully resolved dependencies.As requested by @mrabbani in a previous review comment, show a notice if
dokan_is_dashboard_nav_dependency_resolvedis used to prevent the React implementation.src/hooks/useCurrentUser.ts (2)
58-60: No issues found here.
This final part of the hook correctly implements the dependencies array.
1-2: Verify the external dependency.
Please ensure that@wordpress/datais listed as a dependency in your package or plugin manifest so it’s available at runtime.✅ Verification successful
@wordpress/datadependency is properly declared
The package is listed as a runtime dependency in package.json with version constraint^10.9.0.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that '@wordpress/data' is declared as a dependency rg "@wordpress/data" package*.json -A 3Length of output: 95
Script:
#!/bin/bash # Find all package manifest files and WordPress plugin files echo "=== Package manifests ===" fd -e json -e php "^(package|composer|plugin)$" echo -e "\n=== Searching for @wordpress/data ===" # Search in all JSON files (might catch other config files with dependencies) fd -e json . -x rg -l "@wordpress/data" {} echo -e "\n=== Checking WordPress plugin headers ===" # Search for WordPress plugin headers that might declare dependencies fd -e php . -x rg -l "Plugin Name:|Requires:"Length of output: 420
Script:
#!/bin/bash # Check how @wordpress/data is declared in package.json echo "=== package.json dependencies ===" cat package.json | jq '.dependencies["@wordpress/data"], .devDependencies["@wordpress/data"], .peerDependencies["@wordpress/data"]'Length of output: 230
src/definitions/RouterProps.ts (1)
10-18: Confirm usage ofRedirectFunctionin React Router v6
React Router v6 no longer officially exports aRedirectcomponent in favor of<Navigate />, andRedirectFunctionmay be deprecated. If you’re on React Router v6 or above, please verify thatredirectandreplaceare still relevant in your codebase.Run the following shell script to locate all references to
redirectandreplacein the project to ensure they are valid:✅ Verification successful
Custom type definition is valid for React Router v6 navigation functions
The codebase is using React Router v6 (^6.27.0) and correctly importsredirectandreplacefunctions. WhileRedirectFunctionmay be a custom type definition, the underlying navigation functionality is valid and properly implemented.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash rg -A 3 -B 2 "redirect|replace"Length of output: 81641
Script:
#!/bin/bash # Check react-router-dom version and search for RedirectFunction echo "=== React Router Version ===" cat package.json | jq '.dependencies["react-router-dom"]' echo -e "\n=== Searching for RedirectFunction in node_modules ===" cd node_modules/react-router-dom || exit rg -A 3 "RedirectFunction" || echo "RedirectFunction not found in react-router-dom" echo -e "\n=== Checking actual redirect/replace function types ===" rg -A 3 "redirect.*:|replace.*:" || echo "redirect/replace types not found"Length of output: 435
src/components/PriceHtml.tsx (1)
23-34: Ensure formatted price output is sanitized
RawHTMLdoes not sanitize potentially unsafe content. Verify thatformatPricereturns safe, sanitized content (e.g., free from user-injected HTML) to prevent XSS vulnerabilities.✅ Verification successful
The price formatting implementation is safe from XSS
The
formatPricefunction only processes numbers and standardized formatting characters (currency symbols, decimal/thousand separators) using WooCommerce's accounting library. It cannot output HTML or process user-injected content, making it safe to use withRawHTML.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check all references to `formatPrice` to see if it processes user-controlled data. rg -A 5 -B 5 "formatPrice"Length of output: 6058
Script:
#!/bin/bash # Get the implementation of formatPrice function ast-grep --pattern 'export const formatPrice = ($_) => { $$$ }' # Also get it using ripgrep in case the implementation is different rg -A 20 "^export const formatPrice"Length of output: 1222
Script:
#!/bin/bash # Get the complete formatPrice implementation rg -A 40 "^export const formatPrice" # Check the accounting library type definitions cat src/definitions/woocommerce-accounting.d.tsLength of output: 4944
src/dashboard/Withdraw/Hooks/useCharge.ts (2)
1-2: Import usage is correct
The usage ofuseStateanduseCallbackfrom@wordpress/elementlooks good, andapiFetchis properly imported.
4-13: Interfaces provide clear structure
ChargeDataandChargeResponseare well-defined. This clarity helps maintain typed data across fetch boundaries.src/utilities/Accounting.ts (1)
3-49: Verify fallback behavior when library or currency data is missing
You return the original price ifwindow.accountingorwindow.dokanCurrencyare missing. This prevents crashes, but consider whether returning unformatted values could cause UI inconsistencies elsewhere.src/dashboard/Withdraw/Hooks/useWithdrawSettings.ts (4)
1-2: Imports are consistent
UsinguseState,useEffect, anduseCallbackfrom@wordpress/elementis appropriate for a custom hook. TheapiFetchimport is also aligned with WordPress standards.
4-15: Interfaces define a clear data contract
PaymentMethodandWithdrawMethodeffectively communicate data for the user’s withdrawal options, ensuring clarity in the UI layer.
36-57: Solid error handling approach
The error state is gracefully managed, and the user is informed if settings fail to load. Logging the error is helpful for debugging.
59-67: Refresh method is well implemented
Providing a standalonerefreshmethod simplifies data re-fetching, improving reusability.src/dashboard/Withdraw/Hooks/useBalance.ts (2)
25-31: Handle partial or missing API fields
If any of these fields can be absent or null in responses, consider marking them as optional or providing default values to ensure safer access and avoid potential undefined behavior.
33-38: Interfaces look solid
The returned structure is succinct and well-suited for typical React usage.src/dashboard/Withdraw/index.tsx (1)
12-27: Validate that personal data is not inadvertently exposed
When fetching user-owned data, ensure no sensitive information is leaked in the console or UI. Carefully verify backend security checks, particularly if user IDs can be tampered with.src/dashboard/Withdraw/Hooks/useWithdraw.ts (1)
25-49: Revisit error handling strategy
Setting an error state and rethrowing the error may be helpful for advanced logic, but confirm that any downstream consumer properly handles the thrown error to avoid uncaught exceptions.src/definitions/woocommerce-accounting.d.ts (1)
2-17: Confirm consistent currency definitions in your API
Ensure these fields align with back-end or external APIs. Maintaining a single source of truth for currency formatting (e.g., server-side config) might prevent mismatches in decimal or symbol usage.src/dashboard/Withdraw/Balance.tsx (1)
10-29: Well-organized Loader component.
This skeleton loading pattern is clear and user-friendly, providing a smooth interim experience while data loads.src/dashboard/Withdraw/Hooks/useWithdrawRequests.ts (1)
1-11: Good use of typed payload interfaces.
TheWithdrawRequestPayloadinterface documents expected parameters nicely, reducing confusion and type errors when passing data to thefetchWithdrawRequestsfunction.includes/REST/WithdrawControllerV2.php (1)
49-65: Enforce capability checks for setting default withdraw methods.
Even thoughget_items_permissions_checkis used, ensure it thoroughly verifies the user’s capability to make changes to their withdrawal methods (e.g.,dokan_is_user_seller). Improper checks could allow unauthorized changes.src/dashboard/Withdraw/WithdrawRequests.tsx (2)
23-51: Keep the URL and internal states in sync.
When updatingstatusParamand page, ensure theuseEffectdependencies reflect changes to avoid potential stale reads. For example, ifuseWithdrawRequestHook.view.pagechanges outside of location updates, you may need additional checks or a different data flow strategy.
124-133: Well-structured final return.
The overall component structure is neatly organized. TheDokanToasterusage for toast notifications and separate table rendering keep the code modular and readable.includes/REST/WithdrawController.php (3)
7-7: Validate usage ofstdClass.
The direct usage ofstdClassis fine for basic objects; however, consider implementing a lightweight class or a typed data structure if these objects grow more complex over time.
477-477: No issues detected.
The parameter array foris_valid_approval_requestis correctly passed. Ensure all required keys remain consistent with the withdraw module.
415-432: Check array status before callingreset().
If$last_withdrawisn’t an array or is empty,reset()will yieldfalse. Add a safety check or ensure$last_withdrawis always a non-empty array to avoid potential warnings.+ if ( empty( $last_withdraw ) || ! is_array( $last_withdraw ) ) { + $last_withdraw = []; + } $last_withdraw = reset( $last_withdraw );src/components/index.tsx (3)
1-2: Confirm naming consistency for DataViews.
ExportingDataViewsfrom the file namedDataViewTablecould be confusing. Verify that the default export’s name matches the file’s core functionality.
10-12: Check usage of newly introduced components.
PriceHtml,DateTimeHtml, andFilterlook good. Make sure all references to these default exports in other files align with their updated usage.
4-9: Validate existing exports in@wordpress/dataviews/wp.
Before merging, ensureDataForm,VIEW_LAYOUTS,filterSortAndPaginate, andisItemValidare indeed valid exports. Missing or misspelled exports can cause runtime errors.webpack.config.js (3)
1-3: Initial requires look correct.
The imported dependenciespath,VueLoaderPlugin,entryPoints, andMiniCssExtractPluginmatch typical Webpack usage for a Vue + React environment.
35-35: Inherit resolvers carefully.
SpreadingdefaultConfig.resolveis convenient, but be sure that no unintended merges occur. Confirm that aliases and fallback modules remain correct.
10-22:⚠️ Potential issueUse consistent path references for custom entries.
Defining'dokan-status': '/src/Status/index.tsx'uses a leading slash. Adjust to a relative path for consistency and to avoid potential resolution conflicts.'dokan-status': '/src/Status/index.tsx', + 'dokan-status': './src/Status/index.tsx',Likely invalid or redundant comment.
src/routing/index.tsx (1)
44-91: Route definitions look well-structured.
Consider adding explicit type definitions for each route object, especially if more properties (likeiconorcomponentProps) appear in the future.🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Multiple module resolution errors: Cannot resolve '../Layout/404', '../Dashboard/Withdraw', and '../Dashboard/Withdraw/WithdrawRequests'
package.json (1)
57-59: LGTM! React dependencies look good.The new React-related dependencies are appropriate for implementing the React Skeleton feature:
@dnd-kit/*for drag-and-drop functionality@getdokan/dokan-uifor UI components@headlessui/reactfor accessible components- React-specific WordPress packages
react-router-domfor routing- Utility packages
Also applies to: 64-65, 69-76
🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Package lock file is out of sync with package.json. Multiple dependencies are missing or have version mismatches in the lock file, including @dnd-kit/[email protected], @dnd-kit/[email protected], and others.
includes/functions-dashboard-navigation.php (2)
51-56: LGTM! Clean implementation of React route for withdraw menu.The addition of the
react_routeproperty to the withdraw menu item is well-structured and follows the existing pattern.
263-268: LGTM! Robust menu rendering with React routes.Good improvements to the menu rendering:
- Added data-react-route attribute for React integration
- Using null coalescing operator for optional properties
- Maintained backward compatibility
Also applies to: 284-290
includes/Assets.php (2)
360-370: LGTM! Well-structured React asset integration.The new React-related assets are properly integrated:
- Added styles with proper dependencies
- Added scripts with correct versioning
- Proper conditional loading of components
- Clear separation of concerns
Also applies to: 580-610
385-385: 🛠️ Refactor suggestionRename misleading variable.
The variable
$frontend_shipping_assetsuggests it's related to shipping, but it's actually loading the general frontend React asset.Apply this diff to improve clarity:
-$frontend_shipping_asset = require DOKAN_DIR . '/assets/js/frontend.asset.php'; +$frontend_asset = require DOKAN_DIR . '/assets/js/frontend.asset.php';Likely invalid or redundant comment.
| .dokan-withdraw-style-reset { | ||
| @include windraw-reset; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update mixin name in class definition.
Once the mixin name is corrected, update its usage in the class definition.
.dokan-withdraw-style-reset {
- @include windraw-reset;
+ @include withdraw-reset;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .dokan-withdraw-style-reset { | |
| @include windraw-reset; | |
| } | |
| .dokan-withdraw-style-reset { | |
| @include withdraw-reset; | |
| } |
| background-color: var(--dokan-button-hover-color, #F05025); | ||
| border-color: var(--dokan-button-hover-background-color, #F05025); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Correct CSS custom property name for border color.
The border-color property references an incorrect variable name. The "background" in the variable name doesn't match its usage for border color.
background-color: var(--dokan-button-hover-color, #F05025);
- border-color: var(--dokan-button-hover-background-color, #F05025);
+ border-color: var(--dokan-button-hover-border-color, #F05025);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| background-color: var(--dokan-button-hover-color, #F05025); | |
| border-color: var(--dokan-button-hover-background-color, #F05025); | |
| } | |
| background-color: var(--dokan-button-hover-color, #F05025); | |
| border-color: var(--dokan-button-hover-border-color, #F05025); | |
| } |
| public function enqueue_scripts() { | ||
| global $wp; | ||
| if ( ! dokan_is_seller_dashboard() || ! isset( $wp->query_vars['new'] ) ) { | ||
| return; | ||
| } | ||
|
|
||
| $wc_instance = WCAdminAssets::get_instance(); | ||
| $wc_instance->register_scripts(); | ||
|
|
||
| wp_enqueue_script( 'dokan-react-frontend' ); | ||
| wp_enqueue_style( 'dokan-react-frontend' ); | ||
| wp_localize_script( 'dokan-react-frontend', 'dokanCurrency', dokan_get_container()->get( 'scripts' )->get_localized_price() ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve script enqueuing reliability.
The script enqueuing needs several improvements:
- Add version numbers for cache busting
- Add proper dependency checks
- Add error handling for WC Admin instance
public function enqueue_scripts() {
global $wp;
if ( ! dokan_is_seller_dashboard() || ! isset( $wp->query_vars['new'] ) ) {
return;
}
+ // Check if WooCommerce is active
+ if ( ! class_exists( 'WooCommerce' ) ) {
+ return;
+ }
+
$wc_instance = WCAdminAssets::get_instance();
$wc_instance->register_scripts();
- wp_enqueue_script( 'dokan-react-frontend' );
- wp_enqueue_style( 'dokan-react-frontend' );
+ $version = defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ? time() : DOKAN_VERSION;
+
+ wp_enqueue_script(
+ 'dokan-react-frontend',
+ '', // URL should be defined elsewhere
+ array( 'wp-element', 'wp-components' ),
+ $version,
+ true
+ );
+ wp_enqueue_style(
+ 'dokan-react-frontend',
+ '', // URL should be defined elsewhere
+ array(),
+ $version
+ );
wp_localize_script( 'dokan-react-frontend', 'dokanCurrency', dokan_get_container()->get( 'scripts' )->get_localized_price() );
}Committable suggestion skipped: line range outside the PR's diff.
| public function new_dashboard_content( $query_vars ) { | ||
| if ( isset( $query_vars['new'] ) ) { | ||
| if ( ! current_user_can( 'dokan_view_overview_menu' ) ) { | ||
| dokan_get_template_part( 'global/no-permission' ); | ||
| } else { | ||
| dokan_get_template_part( 'dashboard/new-dashboard' ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance security and user feedback.
The current implementation needs improvements in security and user experience:
- Add nonce verification for the query var
- Add proper error messages for unauthorized access
- Verify template existence before loading
public function new_dashboard_content( $query_vars ) {
- if ( isset( $query_vars['new'] ) ) {
+ if ( isset( $query_vars['new'] ) && wp_verify_nonce( $_REQUEST['_wpnonce'], 'dokan_dashboard' ) ) {
if ( ! current_user_can( 'dokan_view_overview_menu' ) ) {
+ wp_die(
+ esc_html__( 'You do not have permission to access this page.', 'dokan-lite' ),
+ esc_html__( 'Permission Error', 'dokan-lite' ),
+ array( 'response' => 403 )
+ );
dokan_get_template_part( 'global/no-permission' );
} else {
+ $template = 'dashboard/new-dashboard';
+ if ( ! dokan_locate_template( $template . '.php' ) ) {
+ error_log( sprintf( 'Template %s.php not found', $template ) );
+ return;
+ }
dokan_get_template_part( 'dashboard/new-dashboard' );
}
}
}Committable suggestion skipped: line range outside the PR's diff.
| interface CurrentUserResponse { | ||
| id: number; | ||
| name: string; | ||
| url: string; | ||
| description: string; | ||
| link: string; | ||
| slug: string; | ||
| avatar_urls: Record< any, any >; | ||
| meta: any[]; | ||
| is_super_admin: boolean; | ||
| woocommerce_meta: Record< string, any >; | ||
| _links: Links; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace multiple any usages with stricter types.
avatar_urls, meta, and woocommerce_meta are typed with any. If these fields have reasonably consistent structures, consider defining richer interfaces to avoid losing type safety.
| @@ -0,0 +1,73 @@ | |||
| import '../definitions/window-types'; | |||
There was a problem hiding this comment.
Path mismatch may cause build failures
The file imports '../definitions/window-types', whereas another file imports '../Definitions/window-types'. Ensure consistent case sensitivity for the import path to avoid module resolution errors.
| export const useBalance = (): UseBalanceReturn => { | ||
| const [ data, setData ] = useState< BalanceData | null >( null ); | ||
| const [ isLoading, setIsLoading ] = useState< boolean >( true ); | ||
| const [ error, setError ] = useState< Error | null >( null ); | ||
|
|
||
| const fetchBalance = useCallback( async () => { | ||
| try { | ||
| setIsLoading( true ); | ||
| setError( null ); | ||
|
|
||
| const response = await apiFetch< BalanceData >( { | ||
| path: '/dokan/v1/withdraw/balance', | ||
| method: 'GET', | ||
| } ); | ||
|
|
||
| setData( response ); | ||
| } catch ( err ) { | ||
| setError( | ||
| err instanceof Error | ||
| ? err | ||
| : new Error( 'Failed to fetch balance' ) | ||
| ); | ||
| console.error( 'Error fetching balance:', err ); | ||
| } finally { | ||
| setIsLoading( false ); | ||
| } | ||
| }, [] ); | ||
|
|
||
| useEffect( () => { | ||
| fetchBalance(); | ||
| }, [ fetchBalance ] ); | ||
|
|
||
| const refresh = useCallback( () => { | ||
| fetchBalance(); | ||
| }, [ fetchBalance ] ); | ||
|
|
||
| return { data, isLoading, error, refresh }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider retry logic or request cancellation in your async calls
Adding a cancellation or retry mechanism could help avoid potential memory leaks or race conditions in complex React flows. Also, ensure that console logging aligns with production logging strategies.
| ! bodyData || | ||
| ! bodyData.hasOwnProperty( 'isLoading' ) || | ||
| bodyData.isLoading || | ||
| masterLoading |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using hasOwnProperty directly.
Static analysis flagged the direct usage of hasOwnProperty. Consider using Object.hasOwn() or a simple 'isLoading' in bodyData check for better reliability:
-if ( ! bodyData || ! bodyData.hasOwnProperty( 'isLoading' ) || bodyData.isLoading || masterLoading ) {
+if (
+ ! bodyData ||
+ ! Object.hasOwn( bodyData, 'isLoading' ) ||
+ bodyData.isLoading ||
+ masterLoading
+) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ! bodyData || | |
| ! bodyData.hasOwnProperty( 'isLoading' ) || | |
| bodyData.isLoading || | |
| masterLoading | |
| ! bodyData || | |
| ! Object.hasOwn(bodyData, 'isLoading') || | |
| bodyData.isLoading || | |
| masterLoading |
🧰 Tools
🪛 Biome (1.9.4)
[error] 43-43: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
| import NotFound from "../Layout/404"; | ||
| import {__} from "@wordpress/i18n"; | ||
| import {DokanRoute} from "../Layout"; | ||
| import { isValidElement, cloneElement, createElement } from '@wordpress/element'; | ||
| import { useNavigate, useParams, useLocation, redirect, replace, useMatches, useNavigation, createSearchParams } from 'react-router-dom'; | ||
| import Withdraw from "../Dashboard/Withdraw"; | ||
| import WithdrawRequests from "../Dashboard/Withdraw/WithdrawRequests"; |
There was a problem hiding this comment.
💡 Codebase verification
Case sensitivity mismatch in import paths
The pipeline failures are due to case mismatches in the import paths. Update the imports to match the actual filesystem paths:
- Change
"../Layout/404"to"../layout/404" - Change
"../Dashboard/Withdraw"to"../dashboard/Withdraw" - Change
"../Dashboard/Withdraw/WithdrawRequests"to"../dashboard/Withdraw/WithdrawRequests"
🔗 Analysis chain
Pipeline failures: verify module paths.
The E2E_API pipeline indicates failures resolving ../Layout/404, ../Dashboard/Withdraw, and ../Dashboard/Withdraw/WithdrawRequests. Confirm these directories and filenames match your imports (case-sensitive).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the referenced files to confirm they exist and match the import paths
fd 404 --glob --case-sensitive
fd Withdraw --glob --case-sensitive
fd WithdrawRequests --glob --case-sensitive
Length of output: 159
Script:
#!/bin/bash
# Search for 404 component with various extensions
fd "404\.(tsx|ts|js|jsx)" --case-sensitive
# Check contents of Withdraw directories
ls -R src/dashboard/Withdraw/
ls -R includes/Withdraw/
# Search for Layout directory
fd Layout --type d --case-sensitive
# Additional search for WithdrawRequests with extensions
fd "WithdrawRequests\.(tsx|ts|js|jsx)" --case-sensitive
Length of output: 777
🧰 Tools
🪛 GitHub Actions: E2E_API Tests
[error] Multiple module resolution errors: Cannot resolve '../Layout/404', '../Dashboard/Withdraw', and '../Dashboard/Withdraw/WithdrawRequests'
| "@wordpress/api-fetch": "^7.14.0", | ||
| "@wordpress/components": "^28.9.0", |
There was a problem hiding this comment.
Remove duplicate dependencies.
The following dependencies are duplicated in the dependencies section:
@wordpress/api-fetch@wordpress/components@wordpress/data@wordpress/url
Apply this diff to remove the duplicates:
{
"dependencies": {
"@wordpress/api-fetch": "^7.14.0",
"@wordpress/components": "^28.9.0",
"@wordpress/data": "^10.9.0",
"@wordpress/url": "^4.15.0",
- "@wordpress/api-fetch": "^7.14.0",
- "@wordpress/components": "^28.9.0",
- "@wordpress/data": "^10.9.0",
- "@wordpress/url": "^4.15.0"
}
}Also applies to: 66-67, 62-62, 68-68, 63-63, 77-77
🧰 Tools
🪛 Biome (1.9.4)
[error] 60-60: The key @wordpress/api-fetch was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
[error] 61-61: The key @wordpress/components was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🪛 GitHub Actions: E2E_API Tests
[error] Package lock file is out of sync with package.json. Multiple dependencies are missing or have version mismatches in the lock file, including @dnd-kit/[email protected], @dnd-kit/[email protected], and others.
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
Based on the comprehensive summary, here are the updated release notes:
New Features
useWindowDimensionshook for tracking viewport dimensions.VendorNavMenuCheckerfor managing vendor dashboard navigation.NewDashboardclass for custom dashboard templates.Improvements
Technical Updates
Documentation