Skip to content
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

feat: React Skeleton #2403

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions base-tailwind.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import {
scopedPreflightStyles,
isolateInsideOfContainer,
} from 'tailwindcss-scoped-preflight';

const rootClass = '.dokan-layout'; //We will use this class to scope the styles.

/** @type {import('tailwindcss').Config} */
const baseConfig = {
important: rootClass,
content: [ './src/**/*.{js,jsx,ts,tsx}', '!./**/*.asset.php' ],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use 'exclude' option instead of negation in 'content' array

In line 11, the content array includes a negation pattern '!./**/*.asset.php'. Tailwind CSS does not support negation patterns in the content array. Instead, you should use the exclude option to exclude files or directories.

Apply this diff to correct the configuration:

 const baseConfig = {
     important: rootClass,
-    content: [ './src/**/*.{js,jsx,ts,tsx}', '!./**/*.asset.php' ],
+    content: [ './src/**/*.{js,jsx,ts,tsx}' ],
+    exclude: [ './**/*.asset.php' ],
     theme: {

Committable suggestion was skipped due to low confidence.

theme: {
extend: {
backgroundColor: {
dokan: {
sidebar: {
DEFAULT:
'var(--dokan-sidebar-background-color, #F05025)',
hover: 'var(--dokan-sidebar-hover-background-color, #F05025)',
},
btn: {
DEFAULT:
'var(--dokan-button-background-color, #F05025)',
hover: 'var(--dokan-button-hover-background-color, #F05025)',
},
},
},
textColor: {
dokan: {
sidebar: {
DEFAULT: 'var(--dokan-sidebar-text-color, #CFCFCF)',
hover: 'var(--dokan-sidebar-hover-text-color, #ffffff)',
},
btn: {
DEFAULT: 'var(--dokan-button-text-color, #ffffff)',
hover: 'var(--dokan-button-hover-text-color, #ffffff)',
},
},
},
borderColor: {
dokan: {
btn: {
DEFAULT: 'var(--dokan-button-border-color, #F05025)',
hover: 'var(--dokan-button-hover-border-color, #F05025)',
},
},
},
colors: {
primary: 'var(--dokan-button-background-color, #F05025)',
dokan: {
sidebar: 'var(--dokan-button-background-color, #1B233B)',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential typo in 'sidebar' color variable

In line 51, the sidebar color is set to 'var(--dokan-button-background-color, #1B233B)'. This seems inconsistent with the variable naming convention used elsewhere. Should it be 'var(--dokan-sidebar-background-color, #1B233B)' to match the sidebar background color variable used in the backgroundColor section?

Apply this diff to correct the variable name:

                     sidebar: 'var(--dokan-button-background-color, #1B233B)',
+                    sidebar: 'var(--dokan-sidebar-background-color, #1B233B)',
📝 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.

Suggested change
sidebar: 'var(--dokan-button-background-color, #1B233B)',
sidebar: 'var(--dokan-sidebar-background-color, #1B233B)',

btn: 'var(--dokan-button-background-color, #F05025)',
},
},
},
},
plugins: [
scopedPreflightStyles( {
isolationStrategy: isolateInsideOfContainer( rootClass, {} ),
} ),
require( '@tailwindcss/typography' ),
require( '@tailwindcss/forms' ),
],
};

module.exports = baseConfig;
12 changes: 12 additions & 0 deletions includes/Assets.php
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,10 @@ public function get_styles() {
'src' => DOKAN_PLUGIN_ASSEST . '/css/dokan-tailwind.css',
'version' => filemtime( DOKAN_DIR . '/assets/css/dokan-tailwind.css' ),
],
'dokan-react-frontend' => [
'src' => DOKAN_PLUGIN_ASSEST . '/css/frontend.css',
'version' => filemtime( DOKAN_DIR . '/assets/css/frontend.css' ),
],
];

return $styles;
Expand All @@ -365,6 +369,7 @@ public function get_styles() {
public function get_scripts() {
global $wp_version;

$frontend_shipping_asset = require DOKAN_DIR . '/assets/js/frontend.asset.php';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Correct the variable name to match the asset being loaded

The variable $frontend_shipping_asset is assigned the contents of frontend.asset.php. The name suggests it's related to shipping assets, which might be misleading in this context. To improve clarity and maintain consistent naming conventions, please rename the variable to $frontend_asset.

Apply this diff to correct the variable name:

-$frontend_shipping_asset = require DOKAN_DIR . '/assets/js/frontend.asset.php';
+$frontend_asset = require DOKAN_DIR . '/assets/js/frontend.asset.php';

Ensure to update all references to $frontend_shipping_asset accordingly.

📝 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.

Suggested change
$frontend_shipping_asset = require DOKAN_DIR . '/assets/js/frontend.asset.php';
$frontend_asset = require DOKAN_DIR . '/assets/js/frontend.asset.php';

$suffix = defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ? '' : '.min';
$asset_url = DOKAN_PLUGIN_ASSEST;
$asset_path = DOKAN_DIR . '/assets/';
Expand Down Expand Up @@ -554,6 +559,11 @@ public function get_scripts() {
'deps' => [ 'jquery' ],
'version' => filemtime( $asset_path . 'js/dokan-frontend.js' ),
],
'dokan-react-frontend' => [
'src' => $asset_url . '/js/frontend.js',
'deps' => $frontend_shipping_asset['dependencies'],
'version' => $frontend_shipping_asset['version'],
],
Comment on lines +562 to +566
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Update variable references after renaming

Following the renaming of $frontend_shipping_asset to $frontend_asset, please update the variable references in the script registration to reflect this change.

Apply this diff to update the variable references:

 'dokan-react-frontend' => [
     'src'     => $asset_url . '/js/frontend.js',
-    'deps'    => $frontend_shipping_asset['dependencies'],
-    'version' => $frontend_shipping_asset['version'],
+    'deps'    => $frontend_asset['dependencies'],
+    'version' => $frontend_asset['version'],
 ],
📝 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.

Suggested change
'dokan-react-frontend' => [
'src' => $asset_url . '/js/frontend.js',
'deps' => $frontend_shipping_asset['dependencies'],
'version' => $frontend_shipping_asset['version'],
],
'dokan-react-frontend' => [
'src' => $asset_url . '/js/frontend.js',
'deps' => $frontend_asset['dependencies'],
'version' => $frontend_asset['version'],
],

];

return $scripts;
Expand Down Expand Up @@ -856,6 +866,8 @@ public function dokan_dashboard_scripts() {
self::load_form_validate_script();
$this->load_gmap_script();

wp_enqueue_script( 'dokan-react-frontend' );
wp_enqueue_style( 'dokan-react-frontend' );
Comment on lines +869 to +870
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Conditionally enqueue 'dokan-react-frontend' assets to optimize performance

Currently, the scripts and styles for 'dokan-react-frontend' are enqueued unconditionally in the dokan_dashboard_scripts() method. Loading these assets on pages where they are not required can negatively impact performance. Consider enqueueing these assets only when they are needed.

For example, you can modify the code to enqueue the assets conditionally:

if ( /* condition to check if the assets are needed */ ) {
    wp_enqueue_script( 'dokan-react-frontend' );
    wp_enqueue_style( 'dokan-react-frontend' );
}

Replace /* condition to check if the assets are needed */ with the appropriate conditional logic based on your application's requirements.

wp_enqueue_script( 'jquery' );
wp_enqueue_script( 'jquery-ui' );
wp_enqueue_script( 'jquery-ui-autocomplete' );
Expand Down
12 changes: 11 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
"release:dev": "npm install && npm run build && npm run clean-files && npm run makepot && npm run zip"
},
"devDependencies": {
"@tailwindcss/forms": "^0.5.9",
"@tailwindcss/typography": "^0.5.15",
"@wordpress/scripts": "^27.9.0",
"chartjs-adapter-moment": "^1.0.1",
"debounce": "^1.2.1",
Expand All @@ -34,6 +36,7 @@
"papaparse": "^5.4.1",
"replace-in-file": "^6.3.5",
"tailwindcss": "^3.3.3",
"tailwindcss-scoped-preflight": "^3.4.5",
"vue": "^2.7.14",
"vue-chartjs": "^3.5.1",
"vue-color": "^2.8.1",
Expand All @@ -50,6 +53,13 @@
"wp-readme-to-markdown": "^1.0.1"
},
"dependencies": {
"@wordpress/i18n": "^5.8.0"
"@wordpress/components": "^28.9.0",
"@wordpress/data": "^10.9.0",
"@wordpress/dom-ready": "^4.9.0",
"@wordpress/element": "^6.9.0",
"@wordpress/hooks": "^4.9.0",
"@wordpress/i18n": "^5.8.0",
"@wordpress/plugins": "^7.10.0",
"react-router-dom": "^6.27.0"
}
}
32 changes: 32 additions & 0 deletions src/Dashboard/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import {createRoot} from "@wordpress/element";
import domReady from "@wordpress/dom-ready";
import Layout from "../Layout";
import getRoutes from "../Routing";
import {
createBrowserRouter, createHashRouter,
RouterProvider,
} from "react-router-dom";
import './tailwind.scss';

const App = () => {
const routes = getRoutes();

const mapedRoutes = routes.map((route) => {
return {
path: route.path,
element: <Layout headerComponent={route?.header} footerComponent={route?.footer} route={route} title={route?.title}>{route.element}</Layout>,
}
});

const router = createHashRouter(mapedRoutes);

return <RouterProvider router={router} />;
}

domReady( function () {
const rootElement = document.querySelector( '.dashboard-content-area' );
const root = createRoot( rootElement! );
root.render(
<App />
);
} );
2 changes: 2 additions & 0 deletions src/Dashboard/tailwind.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@config './../../base-tailwind.config.js';
@import '../base-tailwind';
9 changes: 9 additions & 0 deletions src/Layout/404.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const NotFound = () => {
return (
<div>
<h1>404 - Not Found!</h1>
</div>
);
}
Comment on lines +1 to +7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider enhancing the NotFound component for better user experience and maintainability.

While the current implementation is functional, consider the following improvements:

  1. Use more semantic HTML elements, such as <main> or <section>.
  2. Add a brief explanation and a link back to the home page to help users navigate.
  3. Consider adding some basic styling for better visual appeal.
  4. Add TypeScript type annotation for the component.

Here's an example of how you could improve the component:

import React from 'react';
import { Link } from 'react-router-dom';

const NotFound: React.FC = () => {
  return (
    <main className="not-found">
      <h1>404 - Not Found!</h1>
      <p>Sorry, the page you are looking for does not exist.</p>
      <Link to="/">Go back to homepage</Link>
    </main>
  );
}

export default NotFound;

Note: This assumes you're using React Router for navigation. If not, adjust the import and Link component accordingly.


export default NotFound;
17 changes: 17 additions & 0 deletions src/Layout/ContentArea.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import Sidebar from './Sidebar';
import {Slot} from "@wordpress/components";

const ContentArea = ( { children } ) => {
return (
<>
<Sidebar />
<div className="dokan-layout-content-area">
<Slot name="dokan-layout-content-area-before" />
{ children }
<Slot name="dokan-layout-content-area-after" />
</div>
</>
);
};

export default ContentArea;
7 changes: 7 additions & 0 deletions src/Layout/Footer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import {Slot} from "@wordpress/components";

const Footer = () => {
return <><Slot name="dokan-footer-area" /></>;
};

export default Footer;
Comment on lines +1 to +7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider enhancing type safety and documentation.

The Footer component looks good overall, but consider these enhancements:

  1. Add TypeScript type annotations:

    import React from 'react';
    import { Slot } from "@wordpress/components";
    
    const Footer: React.FC = () => <Slot name="dokan-footer-area" />;
    
    export default Footer;
  2. Add a JSDoc comment to describe the component's purpose and usage:

    /**
     * Footer component that renders a slot for the "dokan-footer-area".
     * This allows for dynamic injection of content into the footer area.
     *
     * @returns {React.ReactElement} The Footer component
     */
    const Footer: React.FC = () => <Slot name="dokan-footer-area" />;

These changes would improve type safety and make the component's purpose clearer to other developers.

Would you like me to prepare a diff with these changes?

19 changes: 19 additions & 0 deletions src/Layout/Header.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import {Slot} from "@wordpress/components";

const Header = ( { title = '' } ) => {

return (
<>
<Slot name="dokan-before-header" />
<div className="dokan-header-title">
{ title && (<h1 className="mb-4 text-3xl font-extrabold text-gray-900 dark:text-white md:text-5xl lg:text-6xl">{title}</h1>)}
</div>
<div className="dokan-header-actions">
<Slot name="dokan-header-actions"/>
</div>
<Slot name="dokan-after-header"/>
</>
);
};

export default Header;
5 changes: 5 additions & 0 deletions src/Layout/Sidebar.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const Sidebar = () => {
return <></>;
};

export default Sidebar;
76 changes: 76 additions & 0 deletions src/Layout/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { createContext, useContext, useState } from '@wordpress/element';
import Header from './Header';
import Footer from './Footer';
import ContentArea from './ContentArea';
import {
SlotFillProvider
} from '@wordpress/components';
import { PluginArea } from '@wordpress/plugins';

// Create a ThemeContext
const ThemeContext = createContext( null );

Comment on lines +11 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Provide a default value for ThemeContext to enhance type safety.

Currently, ThemeContext is initialized with null, which could lead to undefined errors when consuming the context without a provider. It's advisable to provide a default value that matches the expected shape of the context to improve type safety and prevent potential runtime errors.

Apply this diff to set a default value for ThemeContext:

-const ThemeContext = createContext( null );
+interface ThemeContextType {
+  theme: string;
+  setTheme: React.Dispatch<React.SetStateAction<string>>;
+}
+
+const ThemeContext = createContext<ThemeContextType>({
+  theme: 'light',
+  setTheme: () => {},
+});

This defines an interface for the context and provides a default implementation.

📝 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.

Suggested change
const ThemeContext = createContext( null );
interface ThemeContextType {
theme: string;
setTheme: React.Dispatch<React.SetStateAction<string>>;
}
const ThemeContext = createContext<ThemeContextType>({
theme: 'light',
setTheme: () => {},
});

// Create a ThemeProvider component
const ThemeProvider = ( { children } ) => {
const [ theme, setTheme ] = useState( 'light' ); // Example theme state

return (
<ThemeContext.Provider value={ { theme, setTheme } }>
{ children }
</ThemeContext.Provider>
);
};

export type DokanRoute = {
id: string;
title?: string;
icon?: JSX.Element | React.ReactNode;
element: JSX.Element | React.ReactNode;
header?: JSX.Element | React.ReactNode;
footer?: JSX.Element | React.ReactNode;
path: string;
exact?: boolean;
order?: number;
parent?: string;
};

interface LayoutProps {
children: React.ReactNode;
route: DokanRoute;
title?: string;
headerComponent?: JSX.Element|React.ReactNode;
footerComponent?: JSX.Element|React.ReactNode;
}

// Create a Layout component that uses the ThemeProvider
const Layout = ( {
children,
route,
title = '',
headerComponent,
footerComponent,
}: LayoutProps ) => {
return (
<ThemeProvider>
<SlotFillProvider>
<div className="dokan-layout">
{ headerComponent ? (
headerComponent
) : (
<Header title={ title } />
) }
<ContentArea>{ children }</ContentArea>
{ footerComponent ? footerComponent : <Footer /> }
</div>
<PluginArea scope={route.id} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add a null check for route to prevent potential runtime errors.

In <PluginArea scope={route.id} />, if route or route.id is undefined or null, it could lead to a runtime error. It's advisable to include a null check to ensure that route is defined before attempting to access its properties.

Apply this diff to add a safety check:

-<PluginArea scope={route.id} />
+{ route?.id && <PluginArea scope={route.id} /> }

This change ensures that the PluginArea is only rendered when route and route.id are valid.

📝 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.

Suggested change
<PluginArea scope={route.id} />
{ route?.id && <PluginArea scope={route.id} /> }

</SlotFillProvider>
</ThemeProvider>
);
};

// Custom hook to use the ThemeContext
export const useTheme = () => {
return useContext( ThemeContext );
};

export default Layout;
32 changes: 32 additions & 0 deletions src/Routing/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import NotFound from "../Layout/404";
import {__} from "@wordpress/i18n";
import {DokanRoute} from "../Layout";

const getRoutes = () => {
let routes : Array<DokanRoute> = [];

routes.push(
{
id: 'dokan-base',
title: __( 'Dashboard', 'dokan-lite' ),
element: <h1>Dashboard body</h1>,
path: '/',
exact: true,
order: 10,
}
);

// @ts-ignore
routes = wp.hooks.applyFilters('dokan-dashboard-routes', routes) as Array<DokanRoute>;
routes.push(
{
id: 'dokan-404',
element: <NotFound />,
path: '*',
}
);

return routes;
}

export default getRoutes;
21 changes: 21 additions & 0 deletions src/base-tailwind.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
@tailwind base;
@tailwind components;
@tailwind utilities;

// Reset unwanted table styles but keep structure intact
@layer base {
.dokan-layout {
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;
}
}
}
2 changes: 2 additions & 0 deletions webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const entryPoint = {
// Dokan tailwind css
'dokan-tailwind': './src/tailwind.css',

'frontend': './src/Dashboard/index.tsx',
'vue-frontend': './src/frontend/main.js',
'vue-admin': './src/admin/main.js',
'vue-bootstrap': './src/utils/Bootstrap.js',
Expand Down Expand Up @@ -66,6 +67,7 @@ const updatedConfig = {
},

resolve: {
...defaultConfig.resolve,
alias: {
'vue$': 'vue/dist/vue.esm.js',
'@': path.resolve('./src/'),
Expand Down
Loading