-
Notifications
You must be signed in to change notification settings - Fork 216
enhance: Full-width template support for Dokan dashboard page. #2938
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
enhance: Full-width template support for Dokan dashboard page. #2938
Conversation
WalkthroughAdds a full-width vendor dashboard: PHP rewrites the dashboard template for vendor users, registers/enqueues a new React/CSS bundle and injects a filtered Changes
Sequence Diagram(s)sequenceDiagram
participant WP as WordPress (PHP)
participant Dash as Shortcodes/Dashboard
participant Template as fullwidth-dashboard.php
participant Browser as Browser/DOM
participant Bundle as vendor-dashboard JS
participant React as React Layout
rect rgba(232,246,255,0.6)
WP->>Dash: Render dashboard page
Dash->>Dash: Build vendorDashboardLayoutConfig (site,user,vendor,nav,flags)
Dash->>WP: Rewrite to fullwidth-dashboard.php for vendor users
Dash->>Browser: Register assets & wp_add_inline_script(config)
end
rect rgba(246,255,240,0.6)
Browser->>Bundle: DOM ready -> find '#dokan-vendor-dashboard-layout-root'
Bundle->>Bundle: Detect wpadminbar, mount React root
Bundle->>React: Render <Layout/> (reads localStorage & config)
React->>Sidebar: Render sidebar (collapsed/expanded, popovers)
React->>Header: Render header nav & user menu
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (1)
includes/Shortcodes/Dashboard.php (1)
32-47: Add a filter to allow template path customization.The template path is hardcoded, limiting extensibility. Consider adding a filter to allow developers to customize or override the template path.
public function load_fullwidth_template( $template ) { // Check if we should load the fullwidth template if ( ! dokan_is_seller_dashboard() ) { return $template; } // Path to custom template $custom_template = DOKAN_DIR . '/templates/dashboard/full-dashboard.php'; + + // Allow developers to customize the template path + $custom_template = apply_filters( 'dokan_fullwidth_dashboard_template', $custom_template ); // Check if custom template exists if ( file_exists( $custom_template ) ) { return $custom_template; } return $template; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/Shortcodes/Dashboard.php(1 hunks)templates/dashboard/full-dashboard.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
includes/Shortcodes/Dashboard.php (2)
includes/Rewrites.php (1)
__construct(18-33)includes/functions.php (1)
dokan_is_seller_dashboard(148-166)
🪛 GitHub Actions: Inspections
templates/dashboard/full-dashboard.php
[error] 1-1: PHPCS: Doc comment short description must start with a capital letter. (vendor/bin/phpcs ...)
includes/Shortcodes/Dashboard.php
[error] 1-1: PHPCS: Doc comment short description must start with a capital letter. (vendor/bin/phpcs ...)
🪛 GitHub Check: Run PHPCS inspection
templates/dashboard/full-dashboard.php
[failure] 55-55:
Doc comment short description must start with a capital letter
[failure] 37-37:
Doc comment short description must start with a capital letter
[failure] 27-27:
Doc comment short description must start with a capital letter
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
🔇 Additional comments (4)
templates/dashboard/full-dashboard.php (2)
24-24: Review the noindex/nofollow meta tag.The
noindex, nofollowdirectives prevent search engines from indexing the dashboard page, which is appropriate for a seller dashboard. However, verify this aligns with your SEO requirements.
44-52: LGTM! Content rendering preserves WordPress loop.The implementation correctly wraps
the_content()within the WordPress loop, ensuring proper post context and content filters are applied.includes/Shortcodes/Dashboard.php (2)
34-36: LGTM! Proper context check using existing function.The use of
dokan_is_seller_dashboard()ensures the fullwidth template is only loaded in the appropriate context, preventing interference with other pages.
38-44: Confirm template file presence: Thetemplates/dashboard/full-dashboard.phpfile is present (62 lines); no further action needed.
…mplate hooks documentation
…proach and replace full-dashboard template with block-dashboard template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
templates/dashboard/block-dashboard.php (1)
8-8: Replace the DOKAN_SINCE placeholder with the actual version number.The
@sincetag contains a placeholder that should be replaced with the actual version number when this feature is released.includes/Shortcodes/Dashboard.php (1)
30-30: Replace the DOKAN_SINCE placeholder with the actual version number.The
@sincetag contains a placeholder that should be replaced with the actual version number when this feature is released.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/Shortcodes/Dashboard.php(1 hunks)templates/dashboard/block-dashboard.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
includes/Shortcodes/Dashboard.php (1)
includes/functions.php (1)
dokan_is_seller_dashboard(148-166)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (5)
templates/dashboard/block-dashboard.php (4)
19-36: LGTM!The HTML head structure is correct, with proper meta tags, and the
wp_head()hook is correctly positioned to ensure all enqueued scripts and styles load.
37-50: LGTM!The body tag setup is correct, with proper use of
body_class()for styling hooks and a defensivefunction_exists()check forwp_body_open()to ensure compatibility with older WordPress versions.
54-57: Verify the loop is ready before rendering.The template assumes the WordPress loop has posts and hasn't been consumed. If the loop was already iterated elsewhere before this template loads, or if the query wasn't set up properly, this will render nothing silently.
Consider adding
rewind_posts()before the loop or checkinghave_posts()with an else clause:<?php // Render the page content (which contains the shortcode) + if ( have_posts() ) { + rewind_posts(); // Ensure loop is reset - while ( have_posts() ) : - the_post(); - the_content(); - endwhile; + while ( have_posts() ) : + the_post(); + the_content(); + endwhile; + } else { + echo '<p>' . esc_html__( 'No content found.', 'dokan-lite' ) . '</p>'; + } ?>
60-72: LGTM!The footer structure is correct with
wp_footer()properly positioned before the closing body tag to ensure all footer scripts load.includes/Shortcodes/Dashboard.php (1)
11-22: LGTM!The constructor correctly implements an opt-out filter (
dokan_vendor_dashboard_enable_full_width_page) as suggested in previous review comments, allowing developers to disable the full-width template feature. Thetemplate_redirecthook with priority 1 ensures early interception before theme template selection.
* add: implement React-based layout for vendor dashboard with Tailwind integration and updated asset handling * fix: update "Visit Store" link to use dynamic store URL instead of placeholder anchor link * add: enhance React-based vendor dashboard layout with dynamic site and vendor info integration, tooltips, and subscription details; update asset handling and add scroll styling for sidebar * add: enhance vendor dashboard layout with user-specific profile and store details, dynamic edit links, tooltips, and improved avatar handling * add: improve vendor dashboard sidebar with dynamic navigation, collapsible submenus, and enhanced focus/outline styling * refactor: replace individual icon imports in vendor dashboard with dynamic Lucide icon handling and update navigation configuration with icon names * chore: update `lucide-react` to v0.546.0 and refactor icon handling in vendor dashboard layout for dynamic parent-state-based styling * add: enhance vendor dashboard with header navigation, updated icons, and improved layout styling * add: enhance vendor dashboard header with dynamic popover menu, updated navigation links, and improved user dropdown styling * add: enhance vendor dashboard with dynamic admin access in header navigation menu, WPLogo component, and improved layout styling * add: enhance vendor dashboard with dynamic admin link handling, improved sidebar collapse behavior, and updated styling logic * fix: update vendor dashboard hover styles for header popover links and icons * refactor: simplify vendor dashboard layout by moving components to `Layout` and removing unused imports, optimize avatar hover styling * add: implement `Submenu` and `CountBubble` components, enhance sidebar with dynamic popover handling and improved submenu logic * refactor: improve `Submenu` and `Sidebar` styling and icon handling, add dynamic popover-based icon color logic * enhance: improve sidebar dynamic width handling, collapsed state behavior, and active menu item logic in vendor dashboard * refactor: rename `Submenu` to `SubmenuPopover`, enhance animations and transitions, and optimize related styles in vendor dashboard * enhance: update the subscription content color based on the color customizer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
42-42: Duplicate dependency: tailwind-merge in both devDependencies and dependenciesThis can cause version skew and larger installs/bundles. Keep it in one section (likely dependencies) and align on a single version.
Apply after choosing target section/version:
- "tailwind-merge": "^2.6.0", + // (moved to dependencies at a single version)and
- "tailwind-merge": "^2.5.5", + "tailwind-merge": "^2.6.0",Also applies to: 96-96
🧹 Nitpick comments (14)
src/vendor-dashboard/layout/components/CountBubble.tsx (1)
10-17: A11y and positioning nits
- Provide an accessible label or hide purely decorative counts from SRs.
- Absolute positioning when collapsed assumes a relatively positioned parent.
- <span + <span + aria-label={`Count ${count}`} + // Or: aria-hidden if decorative className={ twMerge( 'sidebar-menu-bubble ml-auto inline-flex items-center justify-center min-w-[20px] h-5 px-1.5 py-0.5 text-[10px] font-semibold leading-none rounded-md text-white', isCollapsed && 'absolute -top-1 -right-2' ) } >Ensure the parent of CountBubble has
relativewhenisCollapsedis true.src/vendor-dashboard/layout/components/Header.tsx (2)
62-72: Improve keyboard/mouse accessibility for avatar menuAdd click and keyboard handlers; current hover‑only flow is unusable on touch and incomplete for keyboard.
- onMouseEnter={ () => setIsMenuOpen( true ) } - onMouseLeave={ () => setIsMenuOpen( false ) } + onMouseEnter={ () => setIsMenuOpen( true ) } + onMouseLeave={ () => setIsMenuOpen( false ) } + onClick={ () => setIsMenuOpen( (v) => !v ) } + onKeyDown={ (e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); setIsMenuOpen((v)=>!v); + } else if (e.key === 'Escape') { + setIsMenuOpen(false); + } + } }
111-136: Unstable React list keysAvoid
idxas key; prefer a stable unique key (e.g.,item.urloritem.id).- { headerNav?.map( ( item: any, idx: number ) => ( - <li key={ idx }> + { headerNav?.map( ( item: any ) => ( + <li key={ item?.id || item?.url || item?.label }>package.json (1)
89-89: 0.546.0 bump is safe; tree-shaking issue exists in code pattern, not the versionlucide-react 0.546.0 contains only minor icon/docs fixes and no breaking changes. However, the codebase has a pre-existing tree-shaking limitation: multiple files (Header.tsx, DynamicIcon.tsx, Sidebar.tsx, vendors.tsx, withdraw/index.tsx) use
import * as LucideIcons from 'lucide-react'with runtime dynamic lookups like(LucideIcons as any)[iconName], which prevents bundlers from eliminating unused icons. This is not introduced by the version bump—it's a code pattern optimization opportunity. Consider replacing star imports with explicit icon imports in Header.tsx, DynamicIcon.tsx, and other affected files to enable proper tree-shaking.src/vendor-dashboard/layout/components/SubmenuPopover.tsx (1)
13-13: Define a proper TypeScript interface for the submenu prop.Using
anyreduces type safety and makes the component harder to maintain. Based on usage (lines 51-77), the submenu structure should haveurl,title, andcountsproperties.Consider defining an interface:
+interface SubmenuItem { + url: string; + title: string; + counts?: number; +} + +interface SubmenuItems { + [key: string]: SubmenuItem; +} + const SubmenuPopover = ( { submenu, anchorRef, onMouseEnter, onMouseLeave, onClose, }: { - submenu: any; + submenu: SubmenuItems; anchorRef: RefObject< HTMLElement >; onMouseEnter: () => void; onMouseLeave: () => void; onClose: () => void; } ) => {src/vendor-dashboard/layout/index.tsx (1)
15-16: Consider using classList API for better readability.String concatenation for className works but the classList API is more idiomatic and less error-prone.
- rootEl.className += - ' sticky z-10 ' + ( wpAdminBarEl ? 'top-8' : 'top-0' ); + rootEl.classList.add( 'sticky', 'z-10', wpAdminBarEl ? 'top-8' : 'top-0' );src/vendor-dashboard/layout/components/Layout.tsx (1)
8-11: Simplify localStorage boolean parsing.The current approach using
Boolean(parseInt(...))is verbose and could be simplified.const [ collapsed, setCollapsed ] = useState( () => { const isSidebarCollapsed = window?.localStorage?.getItem( STORAGE_KEY ); - return Boolean( parseInt( isSidebarCollapsed || '0' ) ); + return isSidebarCollapsed === '1'; } );includes/Shortcodes/Dashboard.php (2)
96-101: Clarify variable naming for admin access logic.The variable
$no_accessis misleading—it actually represents whether admin access should be checked, not whether access is denied. When HPOS is enabled, it's set to 'on' (meaning no admin access), which is counterintuitive.$user_name = wp_get_current_user()->display_name ?? ''; $admin_access = dokan_get_option( 'admin_access', 'dokan_general', 'on' ); - $no_access = OrderUtil::is_hpos_enabled() ? 'on' : $admin_access; + $admin_access_restricted = OrderUtil::is_hpos_enabled() ? 'on' : $admin_access;And update line 128:
- } elseif ( 'on' !== $no_access ) { + } elseif ( 'on' !== $admin_access_restricted ) {
69-167: Consider extracting header navigation construction into a separate method.The
register_vendor_dashboard_assets()method is doing multiple things: detecting assets, registering scripts/styles, setting translations, AND building complex frontend navigation logic (lines 96-137). This violates the single responsibility principle.Extract the header navigation construction into a dedicated method:
protected function build_header_navigation(): array { $user_name = wp_get_current_user()->display_name ?? ''; $is_admin = current_user_can( 'manage_options' ); $admin_access = dokan_get_option( 'admin_access', 'dokan_general', 'on' ); $admin_access_restricted = OrderUtil::is_hpos_enabled() ? 'on' : $admin_access; // Build base navigation $header_nav = [ [ 'label' => esc_html__( 'My Account', 'dokan-lite' ), 'icon' => 'UserRound', 'url' => dokan_get_page_url( 'myaccount', 'woocommerce' ), ], [ 'label' => esc_html__( 'Log out', 'dokan-lite' ), 'icon' => 'LogOut', 'url' => esc_url_raw( wp_logout_url( home_url() ) ), ], ]; // Insert conditional admin links if ( $is_admin ) { array_splice( /* ... */ ); } elseif ( 'on' !== $admin_access_restricted ) { array_splice( /* ... */ ); } return $header_nav; }Then call it from
register_vendor_dashboard_assets():$header_nav = $this->build_header_navigation();src/vendor-dashboard/layout/components/Sidebar.tsx (5)
4-4: Consider tree-shakeable icon imports to reduce bundle size.The wildcard import loads the entire Lucide icon library. While line 157 uses dynamic lookup
(LucideIcons as any)[iconName]which requires runtime access, this significantly increases the bundle size.Consider either:
- Import only the specific icons used if the icon set is fixed
- Implement code-splitting/lazy loading for dynamic icons
- Document the bundle size tradeoff if dynamic lookup is essential
20-24: Add TypeScript type definitions to improve type safety.Multiple uses of
anythroughout the component (lines 22, 115, 121, 122, 157, etc.) reduce type safety. Consider defining interfaces for:
window.vendorDashboardLayoutConfigstructureSidebarNavItem(with properties: title, url, icon_name, counts, submenu)SubmenuitemsActivePopoverstateThis would provide better IDE support, catch errors at compile time, and improve maintainability.
Also applies to: 115-115, 121-122, 157-157
31-36: Consider using React'screateRef()for ref creation.Line 33 manually creates ref objects with
{ current: null }. While functional, it's more idiomatic to use React'screateRef()or store refs differently:const getMenuItemRef = (key: string) => { if (!menuItemRefs.current[key]) { menuItemRefs.current[key] = React.createRef<HTMLLIElement>(); } return menuItemRefs.current[key]; };The current approach works but using React's utilities improves clarity and consistency.
367-376: Simplify nested spans for submenu item indentation.Lines 367-368 use nested
<span>elements withml-4andml-3to achieve indentation. This can be simplified to a single span or adjusted parent padding:<span className="ml-7"> {subitem.title} {subitem.counts > 0 && ( <CountBubble count={subitem.counts} /> )} </span>The double nesting adds no semantic value and increases DOM complexity.
208-208: Consider using named constants for truncate lengths or CSS overflow.Hardcoded truncate lengths (9 for site title at line 208, 10 for subscription name at line 430) appear arbitrary. Consider:
- Define as named constants with explanatory comments:
const MAX_SIDEBAR_TITLE_CHARS = 9; // Fits in collapsed sidebar- Or use CSS
text-overflow: ellipsisfor responsive text truncation without JavaScriptThis improves maintainability and adapts better to font/size changes.
Also applies to: 429-432
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
assets/src/less/dashboard.less(1 hunks)assets/src/less/site-navigation.less(2 hunks)includes/Shortcodes/Dashboard.php(1 hunks)includes/functions-dashboard-navigation.php(1 hunks)package.json(1 hunks)src/vendor-dashboard/icons/WPLogo.tsx(1 hunks)src/vendor-dashboard/layout/components/CountBubble.tsx(1 hunks)src/vendor-dashboard/layout/components/Header.tsx(1 hunks)src/vendor-dashboard/layout/components/Layout.tsx(1 hunks)src/vendor-dashboard/layout/components/Sidebar.tsx(1 hunks)src/vendor-dashboard/layout/components/SubmenuPopover.tsx(1 hunks)src/vendor-dashboard/layout/index.tsx(1 hunks)src/vendor-dashboard/layout/style.scss(1 hunks)src/vendor-dashboard/layout/tailwind.config.js(1 hunks)templates/dashboard/block-dashboard.php(1 hunks)webpack-entries.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/vendor-dashboard/layout/style.scss
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/dashboard/block-dashboard.php
🧰 Additional context used
🧬 Code graph analysis (6)
src/vendor-dashboard/layout/tailwind.config.js (1)
base-tailwind.config.js (1)
baseConfig(9-291)
src/vendor-dashboard/layout/components/SubmenuPopover.tsx (1)
src/vendor-dashboard/reports/lib/in-app-purchase.js (1)
window(15-15)
src/vendor-dashboard/layout/components/Sidebar.tsx (1)
src/utilities/Truncate.ts (1)
truncate(1-6)
includes/Shortcodes/Dashboard.php (5)
includes/Utilities/OrderUtil.php (2)
OrderUtil(9-212)is_hpos_enabled(20-26)includes/Utilities/VendorUtil.php (2)
VendorUtil(4-55)get_vendor_default_avatar_url(38-54)includes/Abstracts/DokanShortcode.php (1)
DokanShortcode(5-22)includes/functions.php (4)
dokan_is_seller_dashboard(148-166)dokan_get_option(1014-1024)dokan_get_page_url(922-936)dokan_get_navigation_url(1983-2002)includes/Vendor/Vendor.php (1)
get_shop_name(332-334)
src/vendor-dashboard/layout/index.tsx (1)
src/vendor-dashboard/reports/index.js (1)
root(31-31)
includes/functions-dashboard-navigation.php (1)
includes/functions.php (1)
dokan_get_navigation_url(1983-2002)
🪛 GitHub Check: Run PHPCS inspection
includes/Shortcodes/Dashboard.php
[failure] 136-136:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 130-130:
Opening parenthesis of a multi-line function call must be the last content on the line
[failure] 127-127:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 120-120:
Opening parenthesis of a multi-line function call must be the last content on the line
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (10)
src/vendor-dashboard/layout/tailwind.config.js (1)
1-7: Code is correct as-is; review comment misunderstands Tailwind's glob resolution behavior.The review incorrectly assumes that the content glob resolves relative to the config file's location. Tailwind's default behavior resolves paths relative to the project root, not the config file. The glob
'./src/vendor-dashboard/layout/**/*.{js,jsx,ts,tsx}'correctly resolves tosrc/vendor-dashboard/layout/**from the project root—matching the pattern used successfully across all 12 other nested tailwind configs in this project (withdraw, intelligence, ProFeatures, admin/header, etc.).The missing
.jsextension is also consistent with the project's established pattern; all nested configs omit extensions, and the webpack/wp-scripts build tool resolves them correctly without requiring explicit extensions. The codebase already mixes ESM (nested configs) and CJS (root config) syntax without issues.No changes are needed.
Likely an incorrect or invalid review comment.
src/vendor-dashboard/layout/components/SubmenuPopover.tsx (1)
53-55: Verify URL matching precision to avoid false positives.The
startsWithcomparison may incorrectly mark items as active when URLs share a common prefix. For example, ifcurrentUrlis/products-archive, it would match a submenu item with URL/products.Consider making the comparison more precise:
const isSubActive = subitem?.url && - currentUrl.startsWith( subitem.url ); + ( currentUrl === subitem.url || currentUrl.startsWith( subitem.url + '/' ) || currentUrl.startsWith( subitem.url + '?' ) );Alternatively, verify that the URL structure guarantees no overlapping prefixes.
assets/src/less/dashboard.less (1)
73-73: LGTM! Responsive width calculation aligns with the new layout system.The use of the CSS variable
--dokan-sidebar-widthwith a 250px fallback enables responsive sidebar behavior while maintaining backward compatibility.webpack-entries.js (1)
74-74: LGTM! Webpack entry follows existing patterns.The new entry point for the vendor dashboard layout is correctly configured and consistent with other entries in the file.
assets/src/less/site-navigation.less (2)
5-7: LGTM! Responsive sidebar width with smooth transitions.The CSS variable approach with a transition provides a good user experience when the sidebar is collapsed/expanded.
345-361: Verify React mount success to prevent permanently hidden content.The CSS hides content until React mounts and restores visibility. If the React app fails to mount (e.g., JavaScript error, missing dependencies), the dashboard content will remain hidden, breaking the user experience.
Consider adding:
- A fallback timeout to restore visibility if React doesn't mount within a reasonable time
- Error boundary handling in the React app to restore visibility on errors
- Server-side detection to skip this CSS when React assets fail to load
Example fallback approach in JavaScript:
// Add to index.tsx or similar setTimeout(() => { const content = document.querySelector('.dokan-dashboard-content'); if (content && content.style.visibility === 'hidden') { content.style.visibility = 'visible'; console.warn('React mount timeout - forcing content visibility'); } }, 5000); // 5 second timeoutincludes/functions-dashboard-navigation.php (1)
38-38: LGTM! Icon metadata enriches navigation for the new layout.The addition of
icon_namefields provides structured icon data for the React-based vendor dashboard layout while maintaining backward compatibility with the existingiconfield.Also applies to: 46-46, 54-54, 62-62, 69-69
includes/Shortcodes/Dashboard.php (2)
56-56: Verify the necessity of wp_print_media_templates() call.Calling
wp_print_media_templates()outputs media modal templates typically used for the media library. Confirm this is required for the vendor dashboard functionality, as it may be unnecessary overhead.If media library functionality isn't needed in the vendor dashboard, consider removing this call:
include_once $dashboard_template; - wp_print_media_templates(); exit;
149-151: Add null safety check for vendor object.The
$vendorobject from line 97 can be null or false if the user is not a vendor, which would cause a fatal error when calling$vendor->get_shop_name().'vendor' => [ - 'name' => $vendor ? $vendor->get_shop_name() : $user_name, + 'name' => ( $vendor && is_a( $vendor, 'WeDevs\Dokan\Vendor\Vendor' ) ) ? $vendor->get_shop_name() : $user_name, 'avatar' => VendorUtil::get_vendor_default_avatar_url(), ],Likely an incorrect or invalid review comment.
src/vendor-dashboard/layout/components/Sidebar.tsx (1)
127-127: Verify URL matching logic handles edge cases correctly.The active menu detection uses
currentUrl.startsWith(item.url)throughout. Ensure this handles:
- Similar URL prefixes (e.g.,
/dashboard/productvs/dashboard/products)- Query parameters and URL fragments
- Trailing slashes
If URL structure allows ambiguous prefixes, consider exact matching or more robust URL comparison.
Also applies to: 233-233, 242-243, 356-356
…vigation handling, and adjust WPLogo accessibility attributes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
includes/Shortcodes/Dashboard.php(1 hunks)src/vendor-dashboard/icons/WPLogo.tsx(1 hunks)src/vendor-dashboard/layout/components/Header.tsx(1 hunks)src/vendor-dashboard/layout/components/Sidebar.tsx(1 hunks)templates/dashboard/fullwidth-dashboard.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/vendor-dashboard/layout/components/Header.tsx
- src/vendor-dashboard/layout/components/Sidebar.tsx
- src/vendor-dashboard/icons/WPLogo.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
includes/Shortcodes/Dashboard.php (3)
includes/Utilities/OrderUtil.php (2)
OrderUtil(9-212)is_hpos_enabled(20-26)includes/Utilities/VendorUtil.php (2)
VendorUtil(4-55)get_vendor_default_avatar_url(38-54)includes/functions.php (4)
dokan_is_seller_dashboard(148-166)dokan_get_option(1014-1024)dokan_get_page_url(922-936)dokan_get_navigation_url(1983-2002)
🔇 Additional comments (4)
templates/dashboard/fullwidth-dashboard.php (2)
19-50: LGTM: Standard WordPress template structure.The HTML structure follows WordPress best practices with proper wp_head(), wp_body_open(), and backward compatibility checks. The noindex/nofollow meta tag is appropriate for dashboard pages.
51-60: No coordination issues detected—architecture is intentional and properly separated.Based on verification, the React app at
src/vendor-dashboard/layout/index.tsx(lines 7–9) successfully mounts to thedokan-vendor-dashboard-layout-rootelement and renders the Layout component (navigation/sidebar). The WordPress loop then renders the shortcode content viathe_content()in the separate document flow. This dual-rendering is a valid pattern: React provides page structure and navigation, WordPress provides the actual dashboard content. No conflicts or duplicate rendering detected.includes/Shortcodes/Dashboard.php (2)
65-173: LGTM: Comprehensive asset registration with proper WordPress patterns.The asset registration follows WordPress best practices including:
- Dynamic dependency loading from build artifacts
- Script translations support
- Proper inline config injection with filtering for extensibility
- Conditional navigation items based on user capabilities
The multi-line
array_spliceformatting at Lines 116-128 and 131-142 now complies with PHPCS standards (each argument on its own line, closing parenthesis on a new line).
182-189: LGTM: Efficient conditional asset enqueuing.The method correctly guards asset enqueuing to only the seller dashboard for logged-in users, preventing unnecessary script loading on other pages.
…d vendor layout option in settings, and enhance submenu handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/vendor-dashboard/layout/components/Sidebar.tsx (1)
413-413: Replace transparent icon spacer with proper CSS spacing.Using an invisible Settings icon (
!text-transparent) as a layout spacer is a semantic HTML violation and creates unnecessary DOM elements. Use CSS padding or margin instead.Apply this approach:
-<LucideIcons.Settings className="w-5 h-5 !text-transparent" /> -<span className="ml-2"> +<span className="ml-7"> { subitem.title } </span>Or use a utility class that matches the icon width (w-5 = 1.25rem):
<a href={ subitem.url } - className={ twMerge( - 'group skip-color-module flex items-center py-2.5 px-3 text-sm font-medium rounded-md focus:!outline-none', + className={ twMerge( + 'group skip-color-module flex items-center py-2.5 px-3 pl-10 text-sm font-medium rounded-md focus:!outline-none', isSubActive && 'active' ) } > - <LucideIcons.Settings className="w-5 h-5 !text-transparent" /> - <span className="ml-2"> + <span>
🧹 Nitpick comments (5)
includes/Admin/Settings.php (1)
816-826: Verify the default value won't disrupt existing vendor workflows.The default is set to
'latest', which means all existing installations will automatically switch to the new UI upon upgrade. This could be a breaking change if vendors are accustomed to the legacy dashboard layout.Additionally, consider adding a
tooltipto help administrators understand the implications of this choice, as similar critical settings in this file include tooltips (e.g., lines 457, 529, 659).Please confirm:
- Is it intentional for existing sites to default to the new UI?
- Are there migration notes or user communication planned for this change?
- Would a tooltip describing the difference between "New UI" and "Legacy UI" improve the admin experience?
src/vendor-dashboard/layout/components/Sidebar.tsx (4)
26-30: Add proper TypeScript typing foractivePopoverstate.The
submenuproperty is typed asany, which bypasses type safety. Define a proper interface for the submenu structure based on the data shape fromwindow.vendorDashboardLayoutConfig.sidebarNav.Consider creating interfaces like:
interface SubmenuItem { title: string; url: string; counts?: number; } interface ActivePopoverState { key: string; submenu: Record<string, SubmenuItem>; anchorRef: RefObject<HTMLElement>; }Then update the state:
-const [ activePopover, setActivePopover ] = useState< { - key: string; - submenu: any; - anchorRef: RefObject< HTMLElement >; -} | null >( null ); +const [ activePopover, setActivePopover ] = useState<ActivePopoverState | null>( null );
119-120: Define TypeScript interface forwindow.vendorDashboardLayoutConfig.Casting to
anybypasses type checking. Create a proper interface to ensure type safety and better developer experience.Define an interface at the top of the file or in a shared types file:
interface VendorDashboardLayoutConfig { siteInfo: { siteTitle: string; siteIcon?: string; }; vendor: { name: string; avatar?: string; }; subscription?: { name: string; status: string; }; sidebarNav: Record<string, MenuItem>; } declare global { interface Window { vendorDashboardLayoutConfig?: VendorDashboardLayoutConfig; } }Then remove the
as anycast:-const { siteInfo, vendor, subscription, sidebarNav } = - ( window as any )?.vendorDashboardLayoutConfig || {}; +const { siteInfo, vendor, subscription, sidebarNav } = + window.vendorDashboardLayoutConfig || {};
213-213: Consider increasing the truncation length for site titles.Truncating the site title to only 9 characters seems overly aggressive and may cut off most site names. The sidebar width of 250px when expanded should accommodate longer titles.
Consider increasing the truncation length:
-{ truncate( sideBarTitle, 9 ) } +{ truncate( sideBarTitle, 20 ) }Alternatively, use CSS
text-overflow: ellipsisfor more flexible truncation that adapts to the available space.
182-516: Consider adding accessibility features for keyboard navigation and screen readers.The sidebar lacks ARIA attributes and keyboard navigation support, which impacts users who rely on assistive technologies or keyboard-only navigation.
Consider these improvements:
- Add ARIA roles and attributes:
<nav role="navigation" aria-label={ __( 'Vendor Dashboard Menu', 'dokan-lite' ) }> <ul role="menu" className="flex flex-col gap-1.5"> <li role="none"> <a role="menuitem" aria-expanded={ hasSub ? isExpanded : undefined } aria-current={ isParentActive ? 'page' : undefined } // ... other props >
- Implement keyboard navigation:
- Arrow keys to navigate between menu items
- Enter/Space to activate items or toggle submenus
- Escape to close popovers
- Tab to move focus in/out of sidebar
- Focus management:
const menuItemRef = useRef<HTMLAnchorElement>(null); useEffect(() => { if (isParentActive && menuItemRef.current) { menuItemRef.current.focus(); } }, [isParentActive]);
- Announce state changes to screen readers:
<div role="status" aria-live="polite" className="sr-only"> { collapsed ? __( 'Sidebar collapsed', 'dokan-lite' ) : __( 'Sidebar expanded', 'dokan-lite' ) } </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
assets/src/less/dashboard.less(1 hunks)assets/src/less/site-navigation.less(1 hunks)includes/Admin/Settings.php(1 hunks)includes/Shortcodes/Dashboard.php(1 hunks)src/vendor-dashboard/layout/components/Layout.tsx(1 hunks)src/vendor-dashboard/layout/components/Sidebar.tsx(1 hunks)src/vendor-dashboard/layout/components/SubmenuPopover.tsx(1 hunks)src/vendor-dashboard/layout/style.scss(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- includes/Shortcodes/Dashboard.php
- src/vendor-dashboard/layout/components/Layout.tsx
- assets/src/less/site-navigation.less
- src/vendor-dashboard/layout/components/SubmenuPopover.tsx
- src/vendor-dashboard/layout/style.scss
- assets/src/less/dashboard.less
🧰 Additional context used
🧬 Code graph analysis (1)
src/vendor-dashboard/layout/components/Sidebar.tsx (1)
src/utilities/Truncate.ts (1)
truncate(1-6)
🪛 ast-grep (0.39.6)
src/vendor-dashboard/layout/components/Sidebar.tsx
[warning] 321-321: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/vendor-dashboard/layout/components/Sidebar.tsx
[error] 322-322: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (4)
includes/Admin/Settings.php (1)
810-832: Note: AI summary is inconsistent with the actual implementation.The AI summary states this change "renamed appearance_options to vendor_layout_options," but the code shows that
vendor_layout_optionsis a new section (lines 810-815) andappearance_optionsstill exists with a relabeled title "Store Appearance" (lines 827-832). Both sections coexist in the settings structure.src/vendor-dashboard/layout/components/Sidebar.tsx (3)
91-101: Past duplicatesetActivePopover(null)issue has been resolved.The previous review flagged a duplicate call to
setActivePopover(null). The current code correctly contains only a single call (line 93), so this issue has been addressed.
474-489: Remove unnecessaryRawHTMLwrapper for subscription name text.The
subscriptionNameis plain text data from the backend configuration that should be rendered withoutRawHTML. Since the data is not expected to contain HTML markup, usingRawHTMLis unnecessary and violates security best practices. Replace the wrapped content with plain text rendering:<Tooltip content={ subscriptionName } > <span> { truncate( subscriptionName, 10 ) } </span> </Tooltip>If subscription names can legitimately contain HTML markup, explicitly sanitize them using the existing
SanitizeHtmlutility fromsrc/utilities/SanitizeHtml.ts.
132-132: Review comment is correct in principle but implementation is safe—document this decision.The
startsWith()concern is theoretically valid but not a practical issue here. Menu URLs are constructed as/dashboard/{slug}/with distinct first-level paths (products, orders, withdraw). These don't create overlapping prefixes. The settings menu—which could have nested paths like/dashboard/settings/store/—uses submenus and doesn't applystartsWith()to parent matching.However, this pattern is fragile. If future menu items are added without careful naming (e.g., a "product" item before "products"), false positives could occur. The implementation is currently safe but consider documenting why
startsWith()works here, or use more robust matching (e.g., exact path segment matching) to prevent regressions when the sidebar navigation evolves.
…date layout styles, and enhance sidebar behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/vendor-dashboard/layout/components/Header.tsx (1)
1-21: Use explicit lucide imports to keep the bundle lean.The wildcard
* as LucideIconsimport prevents tree‑shaking and drags the whole icon set into this bundle. Please switch to named imports plus a small whitelist map so only referenced icons ship.-import * as LucideIcons from 'lucide-react'; +import { + ChevronDown, + Globe, + Menu, + User, +} from 'lucide-react'; … - const Icon = - ( LucideIcons as any )[ iconName || '' ] || LucideIcons.User; + const icons = { User, Globe, Menu, ChevronDown } as const; + const Icon = ( iconName && icons[ iconName as keyof typeof icons ] ) || User; … - <LucideIcons.Menu /> + <Menu /> … - <LucideIcons.Globe size={ 16 } className="text-[#828282]" /> + <Globe size={ 16 } className="text-[#828282]" /> … - <LucideIcons.ChevronDown + <ChevronDownsrc/vendor-dashboard/layout/components/Sidebar.tsx (1)
329-337: Block XSS: sanitize dashboard icon HTML.
item.iconcomes from the runtime config and is injected withdangerouslySetInnerHTMLwithout any client-side sanitization. A compromised or misconfigured value can ship arbitrary markup/scripts straight into the vendor dashboard, so we lose our XSS guarantees. Please sanitize (e.g., DOMPurify with an allowlist) or parse the attributes and render a plain<img>element instead of trusting raw HTML.+import DOMPurify from 'dompurify'; … - <span - dangerouslySetInnerHTML={ { - __html: item.icon, - } } - /> + <span + dangerouslySetInnerHTML={ { + __html: DOMPurify.sanitize( item.icon, { + ALLOWED_TAGS: [ 'img' ], + ALLOWED_ATTR: [ 'src', 'alt', 'class', 'width', 'height' ], + } ), + } } + />
🧹 Nitpick comments (1)
assets/src/less/dashboard.less (1)
558-594: Consider nesting media queries within the.dokan-dashboardscope for consistency and maintainability.The new media queries are positioned at the root level (lines 558 and 588) rather than being nested within the
.dokan-dashboardblock, which deviates from typical LESS organization patterns and the existing file structure. This can make future maintenance harder and obscures the relationship between the media queries and their parent scope.Additionally, verify that the responsive behavior at the 768px breakpoint correctly handles the sidebar visibility and layout restructuring—setting
.dokan-dashboard-contenttowidth: 100%may need coordination with sidebar visibility logic to avoid overlapping elements.Refactor the media queries to nest them within the
.dokan-dashboardscope:div.chart-container > div.chart-legend-container > table { td { padding: 0; padding-left: 5px; padding-right: 5px; } } #dokan-dashboard-fullwidth-wrapper { .dokan-dashboard-content { width: calc(100% - var(--dokan-sidebar-width, 250px)); } } + + @media screen and ( max-width: 768px ) { + #dokan-dashboard-fullwidth-wrapper { + .dokan-dashboard-content { + width: 100%; + } + + .dokan-frontend-layout { + header { + padding: 0 22px; + min-height: 60px; + height: 60px; + } + + .visit-store, + .header-avatar { + padding: 0; + } + + .header-avatar { + padding-left: 20px; + } + + .visit-store { + font-size: 12px; + padding-right: 20px; + } + } + } + } + + @media screen and ( max-width: 600px ) { + #dokan-dashboard-fullwidth-wrapper { + #dokan-vendor-dashboard-layout-root { + top: 0; + } + } + } } -@media screen and ( max-width: 768px ) { - #dokan-dashboard-fullwidth-wrapper { - .dokan-dashboard-content { - width: 100%; - } - - .dokan-frontend-layout { - header { - padding: 0 22px; - min-height: 60px; - height: 60px; - } - - .visit-store, - .header-avatar { - padding: 0; - } - - .header-avatar { - padding-left: 20px; - } - - .visit-store { - font-size: 12px; - padding-right: 20px; - } - } - } -} - -@media screen and ( max-width: 600px ) { - #dokan-dashboard-fullwidth-wrapper { - #dokan-vendor-dashboard-layout-root { - top: 0; - } - } -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
assets/src/less/dashboard.less(2 hunks)assets/src/less/site-navigation.less(2 hunks)src/vendor-dashboard/layout/components/Header.tsx(1 hunks)src/vendor-dashboard/layout/components/Layout.tsx(1 hunks)src/vendor-dashboard/layout/components/Sidebar.tsx(1 hunks)src/vendor-dashboard/layout/style.scss(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/vendor-dashboard/layout/components/Layout.tsx
- assets/src/less/site-navigation.less
🧰 Additional context used
🧬 Code graph analysis (1)
src/vendor-dashboard/layout/components/Sidebar.tsx (1)
src/utilities/Truncate.ts (1)
truncate(1-6)
🪛 ast-grep (0.39.7)
src/vendor-dashboard/layout/components/Sidebar.tsx
[warning] 333-333: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/vendor-dashboard/layout/components/Sidebar.tsx
[error] 334-334: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
🔇 Additional comments (1)
assets/src/less/dashboard.less (1)
537-542: Confirm definition of --dokan-sidebar-width (used but not found in repository).Search found usages in assets/src/less/dashboard.less (line ~539: calc(100% - var(--dokan-sidebar-width, 250px))) and assets/src/less/site-navigation.less (lines ~310–311: width/max-width: var(--dokan-sidebar-width, 250px)), but no declaration or JS setProperty for --dokan-sidebar-width. Either add a definitive declaration (e.g. :root { --dokan-sidebar-width: 250px; }) or ensure runtime code sets/updates this CSS variable when the sidebar width changes — otherwise the layout will always fall back to 250px and may misalign.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/vendor-dashboard/layout/components/Header.tsx (1)
17-21: Dynamic icon lookup prevents tree-shaking (previously flagged)This issue was already identified in a previous review. The dynamic lookup
(LucideIcons as any)[iconName]pulls in the entire lucide-react bundle, preventing tree-shaking and potentially adding hundreds of KB to the bundle.
🧹 Nitpick comments (7)
src/vendor-dashboard/layout/components/Header.tsx (2)
10-12: Consider separate const declarations for clarityThe comma-separated destructuring is valid but uncommon. Separate const statements would improve readability.
- const { user, headerNav } = - ( window as any )?.vendorDashboardLayoutConfig || {}, - { name: userName, avatar: userAvatar } = user || {}; + const { user, headerNav } = + ( window as any )?.vendorDashboardLayoutConfig || {}; + const { name: userName, avatar: userAvatar } = user || {};
93-95: Improve type safety and key usageConsider defining a proper interface for menu items and using a stable identifier as the key instead of array index.
Example interface:
interface HeaderNavItem { url?: string; label?: string; icon?: string; isSvg?: boolean; }Then use a stable key if available:
- { headerNav?.map( - ( item: any, idx: number ) => ( - <li key={ idx }> + { ( headerNav as HeaderNavItem[] )?.map( + ( item, idx ) => ( + <li key={ item.url || idx }>src/vendor-dashboard/layout/style.scss (1)
37-41: Use CSS variable for consistency.Line 38 uses a hardcoded
#fffvalue while the rest of the file consistently uses CSS custom properties with fallbacks for theming. This creates inconsistency.Apply this diff to maintain theming consistency:
.sidebar-menu-bubble { - background-color: #fff; + background-color: var(--dokan-sidebar-hover-text-color, #ffffff); color: var(--dokan-sidebar-hover-background-color, #7047EB); }src/vendor-dashboard/layout/components/Sidebar.tsx (4)
32-36: Define proper TypeScript interfaces for type safety.The
activePopoverstate usesanyfor thesubmenuproperty, which bypasses TypeScript's type checking and can lead to runtime errors.Define and use proper interfaces:
+interface SubmenuItem { + title: string; + url: string; + counts?: number; + icon?: string; + icon_name?: string; +} + +interface MenuItem { + title: string; + url: string; + icon?: string; + icon_name?: string; + counts?: number; + submenu?: Record<string, SubmenuItem>; +} + const [ activePopover, setActivePopover ] = useState< { key: string; - submenu: any; + submenu: Record<string, SubmenuItem>; anchorRef: RefObject< HTMLElement >; } | null >( null );
125-127: Augment Window interface for type safety.Accessing
window.vendorDashboardLayoutConfigwithanycast bypasses type checking. Define proper types for the global configuration object.Create a type definition file to augment the Window interface:
// types/global.d.ts interface VendorDashboardLayoutConfig { siteInfo: { siteTitle: string; siteIcon?: string; }; vendor: { name: string; avatar?: string; }; subscription?: { name: string; status: string; }; sidebarNav: Record<string, MenuItem>; } declare global { interface Window { vendorDashboardLayoutConfig?: VendorDashboardLayoutConfig; } }Then use it without casting:
-const { siteInfo, vendor, subscription, sidebarNav } = - ( window as any )?.vendorDashboardLayoutConfig || {}; +const { siteInfo, vendor, subscription, sidebarNav } = + window.vendorDashboardLayoutConfig || {};
432-432: Replace transparent icon spacer with CSS.Using a transparent icon component as a spacer is inefficient and non-semantic. Use CSS margin or an empty span element instead.
Apply this diff to use proper spacing:
-<LucideIcons.Settings className="w-5 h-5 !text-transparent" /> -<span className="ml-2"> +<span className="ml-7"> { subitem.title }Or use a proper spacer element:
-<LucideIcons.Settings className="w-5 h-5 !text-transparent" /> +<span className="w-5 h-5" aria-hidden="true" /> <span className="ml-2">
190-243: Add ARIA attributes for accessibility.The sidebar lacks proper ARIA attributes for screen readers, which impacts accessibility for users with disabilities.
Apply these improvements:
<aside + role="navigation" + aria-label={ __( 'Vendor Dashboard Navigation', 'dokan-lite' ) } style={ { top: windowWidth > 600 ? adminBar : 0 } } className={ twMerge(For menu items with submenus (around line 303):
<a href={ item.url } + aria-expanded={ hasSub && ! collapsed ? isExpanded : undefined } + aria-haspopup={ hasSub ? 'true' : undefined } onClick={ ( e ) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/vendor-dashboard/layout/components/Header.tsx(1 hunks)src/vendor-dashboard/layout/components/Sidebar.tsx(1 hunks)src/vendor-dashboard/layout/style.scss(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/vendor-dashboard/layout/components/Sidebar.tsx (1)
src/utilities/Truncate.ts (1)
truncate(1-6)
🪛 ast-grep (0.39.7)
src/vendor-dashboard/layout/components/Sidebar.tsx
[warning] 340-340: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/vendor-dashboard/layout/components/Sidebar.tsx
[error] 341-341: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (2)
src/vendor-dashboard/layout/style.scss (1)
115-125: LGTM! Comprehensive cross-browser scrollbar hiding.The implementation correctly hides scrollbars across WebKit and non-WebKit browsers while preserving scrollability.
src/vendor-dashboard/layout/components/Sidebar.tsx (1)
497-509: Verify server-side sanitization of subscription name and whether HTML rendering is necessary.The
RawHTMLcomponent bypasses React's XSS protection. WhilesubscriptionNameoriginates from server-side data inwindow.vendorDashboardLayoutConfig, we cannot confirm server-side sanitization from the frontend code. If plain text suffices for subscription names, use direct rendering instead for defense-in-depth:<Tooltip - content={ - <RawHTML> - { subscriptionName } - </RawHTML> - } + content={ subscriptionName } > <span> - <RawHTML> - { truncate( - subscriptionName, - 10 - ) } - </RawHTML> + { truncate( subscriptionName, 10 ) } </span> </Tooltip>
… for improved layout management
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
includes/Admin/Settings.php (1)
832-832: Grammar issue still present.The past review comment regarding "appearances" (plural) vs "appearance" (singular) remains unaddressed.
🧹 Nitpick comments (1)
includes/Admin/Settings.php (1)
1178-1184: Add defensive type check.The method accesses
$option_value['vendor_layout_style']without verifying that$option_valueis an array, which could cause a type error if an unexpected value is passed.Apply this diff to add a defensive check:
public function set_vendor_latest_layout( $option_value, $option_name ) { - if ( 'dokan_appearance' === $option_name && empty( $option_value['vendor_layout_style'] ) ) { + if ( 'dokan_appearance' === $option_name && is_array( $option_value ) && empty( $option_value['vendor_layout_style'] ) ) { $option_value['vendor_layout_style'] = 'latest'; } return $option_value; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/Admin/Settings.php(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (2)
includes/Admin/Settings.php (2)
38-38: LGTM!The filter hook registration follows the established pattern and correctly registers the new
set_vendor_latest_layoutmethod with appropriate priority.
811-827: Well-structured vendor dashboard settings.The new vendor layout options provide a clear interface for administrators to configure the dashboard appearance. The settings structure follows Dokan conventions and properly separates vendor dashboard configuration from store appearance settings.
…ements to tooltip and dynamic updates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/vendor-dashboard/layout/components/Sidebar.tsx (1)
346-364: CRITICAL: XSS vulnerability remains unresolved from previous review.This issue was flagged in a previous review but has not been addressed. The code injects
item.iconHTML directly without sanitization usingdangerouslySetInnerHTML, creating a potential XSS vulnerability. Even if the PHP backend sanitizes input, defense-in-depth requires client-side sanitization.Install and use DOMPurify to sanitize the HTML:
npm install dompurify @types/dompurifyThen apply this diff:
+import DOMPurify from 'dompurify'; { item.icon && item.icon.startsWith( '<img' ) ? ( <span dangerouslySetInnerHTML={ { - __html: item.icon, + __html: DOMPurify.sanitize( item.icon, { + ALLOWED_TAGS: [ 'img' ], + ALLOWED_ATTR: [ 'src', 'alt', 'class', 'width', 'height', 'style' ] + } ), } } />Alternatively (preferred), parse the image src and render a proper React
<img>element to avoiddangerouslySetInnerHTMLentirely.
🧹 Nitpick comments (2)
src/vendor-dashboard/layout/components/Sidebar.tsx (2)
184-196: LGTM: Hash-based navigation detection is appropriate.The effect correctly detects hash-based URL changes. If the dashboard also uses
pushState/replaceStatenavigation, consider adding apopstatelistener as well.
442-442: Consider using proper CSS spacing instead of a transparent icon.Using
<LucideIcons.Settings className="w-5 h-5 !text-transparent" />as a spacer is not semantic. Consider using a CSS-based spacer or margin instead.Apply this diff:
-<LucideIcons.Settings className="w-5 h-5 !text-transparent" /> -<span className="ml-2"> +<span className="ml-7"> { subitem.title } </span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/ReverseWithdrawal/Hooks.php(1 hunks)src/vendor-dashboard/layout/components/Sidebar.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/vendor-dashboard/layout/components/Sidebar.tsx (1)
src/utilities/Truncate.ts (1)
truncate(1-6)
🪛 ast-grep (0.40.0)
src/vendor-dashboard/layout/components/Sidebar.tsx
[warning] 350-350: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/vendor-dashboard/layout/components/Sidebar.tsx
[error] 351-351: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (13)
src/vendor-dashboard/layout/components/Sidebar.tsx (13)
1-23: LGTM: Clean imports and component signature.The imports are well-organized and the component props are clearly typed.
24-42: LGTM: State management setup is appropriate.The component state and refs are well-structured for managing the sidebar's expanded/collapsed behavior and popover interactions.
44-95: LGTM: Popover state management is well-implemented.The ref management and timeout handling for popover show/hide logic is correct, with proper cleanup on unmount.
98-108: Confirmed: DuplicatesetActivePopovercall has been fixed.The previous review concern about duplicate
setActivePopover(null)calls has been addressed. The effect now correctly resets the popover state once when the sidebar is expanded.
113-121: LGTM: Admin bar height detection is correctly implemented.The effect properly measures the WordPress admin bar height and updates on window resize with appropriate cleanup.
129-150: LGTM: Active menu auto-expansion logic is sound.The effect correctly identifies which parent menu items should be expanded based on the current URL matching submenu items.
155-182: LGTM: Helper functions are correctly implemented.The icon rendering with fallback and submenu toggle logic are straightforward and appropriate.
198-244: LGTM: Sidebar header rendering is well-structured.The responsive behavior, icon fallback, and tooltip integration for the site title are all correctly implemented.
254-290: LGTM: Active state detection logic is comprehensive.The code correctly identifies active parent items, active children, and handles the collapsed state appropriately.
403-463: LGTM: Inline submenu rendering with smooth animation.The submenu expansion/collapse animation using CSS classes is well-implemented, and the submenu items correctly detect their active state.
482-495: LGTM: Avatar rendering with appropriate fallback.The store avatar rendering handles both custom images and a default gradient icon properly.
551-560: LGTM: Popover rendering is correctly implemented.The conditional rendering and prop passing for the
SubmenuPopovercomponent is appropriate.
497-546: Use appropriate rendering method for user-controlled content instead ofRawHTML.While store names are sanitized with
sanitize_text_field()at save time (Registration.php, Dashboard/Templates/Settings.php, Admin/UserProfile.php), using<RawHTML>to display them is architecturally incorrect.RawHTMLrenders arbitrary HTML and should only be used for trusted content. For plain text display, use standard React text rendering instead. Additionally, thesubscriptionNameorigin could not be confirmed in the config—verify its source and sanitization. Consider removingRawHTMLusage and using appropriate text rendering methods.
| 'icon' => '<i class="fas fa-dollar-sign"></i>', | ||
| 'url' => dokan_get_navigation_url( 'reverse-withdrawal' ), | ||
| 'pos' => 71, | ||
| 'icon_name' => 'Redo2', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify the icon name exists in the icon library.
The addition of 'icon_name' => 'Redo2' looks appropriate for the reverse withdrawal navigation item. Please confirm that the 'Redo2' icon exists in the icon library used by the new React dashboard layout and renders correctly.
🏁 Script executed:
#!/bin/bash
# Description: Search for the Redo2 icon definition or usage in the React components
# Search for Redo2 icon in component files, icon definitions, or imports
rg -n --type=js --type=ts -C3 "Redo2"
# Also search in any icon registry or mapping files
fd -e js -e ts -e jsx -e tsx | xargs rg -l "icon.*registry|iconMap|icon.*metadata" | head -5 | xargs catLength of output: 1700
'Redo2' icon name is not defined in the icon registry and will fail at runtime.
The 'icon_name' => 'Redo2' references an icon that does not exist in the codebase's iconMap. When the component receives this key, it will:
- Log a console warning: "Icon with key 'Redo2' not found in icon map"
- Return null (render nothing)
The available icon keys are: 'whats-new', 'support', 'facebook', 'documentation', 'faq', 'settings', 'feature-request', 'setup-wizard', 'import-data', 'custom-icon'.
Either use an existing icon name from this list, or add the 'Redo2' icon component to the registry (import it and add it to iconMap).
🤖 Prompt for AI Agents
In includes/ReverseWithdrawal/Hooks.php around line 256, the item uses
'icon_name' => 'Redo2' which is not present in the app's iconMap and will cause
a runtime warning and render nothing; fix it by either replacing 'Redo2' with
one of the available keys (e.g., 'custom-icon' or another listed:
'whats-new','support','facebook','documentation','faq','settings','feature-request','setup-wizard','import-data')
or add the Redo2 icon to the registry by importing/defining the Redo2 component
and adding it to the iconMap with the corresponding key.
…for menu interaction in Header component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/vendor-dashboard/layout/components/Header.tsx (1)
17-21: Dynamic icon lookup still prevents tree-shaking.This issue was flagged in a previous review but remains unresolved. The dynamic lookup
(LucideIcons as any)[iconName]prevents tree-shaking and includes the entire lucide-react bundle.
🧹 Nitpick comments (1)
src/vendor-dashboard/layout/components/Header.tsx (1)
15-15: Improve type safety for popoverAnchor.The
anytype reduces type safety. Use a more specific type for the popover anchor element.Apply this diff:
- const [ popoverAnchor, setPopoverAnchor ] = useState< any >(); + const [ popoverAnchor, setPopoverAnchor ] = useState< HTMLDivElement | null >( null );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/vendor-dashboard/layout/components/Header.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (1)
src/vendor-dashboard/layout/components/Header.tsx (1)
108-110: WPLogo color handling corrected.The WPLogo now correctly uses
text-[#828282]to set the icon color viacurrentColor, addressing the previous review feedback.
| <button | ||
| type="button" | ||
| onClick={ onToggleSidebar } | ||
| aria-label="Toggle sidebar menu" | ||
| className="p-2 rounded hover:bg-gray-100 focus:ring-0 focus:!outline-none" | ||
| > | ||
| <LucideIcons.Menu /> | ||
| </button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing focus indicators breaks keyboard navigation.
Line 33 uses focus:ring-0 focus:!outline-none, which removes all visible focus indicators. This prevents keyboard users from seeing which element has focus.
Apply this diff to restore focus indicators:
<button
type="button"
onClick={ onToggleSidebar }
aria-label="Toggle sidebar menu"
- className="p-2 rounded hover:bg-gray-100 focus:ring-0 focus:!outline-none"
+ className="p-2 rounded hover:bg-gray-100 focus:ring-2 focus:ring-blue-500 focus:outline-none"
>📝 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.
| <button | |
| type="button" | |
| onClick={ onToggleSidebar } | |
| aria-label="Toggle sidebar menu" | |
| className="p-2 rounded hover:bg-gray-100 focus:ring-0 focus:!outline-none" | |
| > | |
| <LucideIcons.Menu /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={ onToggleSidebar } | |
| aria-label="Toggle sidebar menu" | |
| className="p-2 rounded hover:bg-gray-100 focus:ring-2 focus:ring-blue-500 focus:outline-none" | |
| > | |
| <LucideIcons.Menu /> | |
| </button> |
🤖 Prompt for AI Agents
In src/vendor-dashboard/layout/components/Header.tsx around lines 29 to 36, the
button currently strips visible focus indicators via the classes "focus:ring-0
focus:!outline-none"; remove those classes and restore accessible focus styling
— delete "focus:ring-0" and "focus:!outline-none" and add a visible focus style
such as "focus-visible:ring-2 focus-visible:ring-indigo-500
focus-visible:outline-none" (or your design system's equivalent) so keyboard
users can see focus while preserving visual design.
| <a | ||
| target="_blank" | ||
| href={ window.dokan?.urls?.storeUrl || '#' } | ||
| className="visit-store group skip-color-module flex items-center text-sm gap-2 font-medium text-[#25252D] focus:!outline-none py-4 px-5" | ||
| > | ||
| <LucideIcons.Globe size={ 16 } className="text-[#828282]" /> | ||
| { __( 'Visit Store', 'dokan-lite' ) } | ||
| </a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix accessibility and security issues in Visit Store link.
Multiple issues:
- Line 44:
focus:!outline-noneremoves focus indicators for keyboard navigation. - Line 43: Fallback to
'#'creates a non-functional link. - Line 42:
target="_blank"withoutrel="noopener noreferrer"is a security risk.
Apply this diff:
<a
target="_blank"
+ rel="noopener noreferrer"
href={ window.dokan?.urls?.storeUrl || '#' }
- className="visit-store group skip-color-module flex items-center text-sm gap-2 font-medium text-[#25252D] focus:!outline-none py-4 px-5"
+ className="visit-store group skip-color-module flex items-center text-sm gap-2 font-medium text-[#25252D] focus:ring-2 focus:ring-blue-500 focus:outline-none py-4 px-5"
>Consider disabling the link entirely when storeUrl is unavailable rather than using '#'.
📝 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.
| <a | |
| target="_blank" | |
| href={ window.dokan?.urls?.storeUrl || '#' } | |
| className="visit-store group skip-color-module flex items-center text-sm gap-2 font-medium text-[#25252D] focus:!outline-none py-4 px-5" | |
| > | |
| <LucideIcons.Globe size={ 16 } className="text-[#828282]" /> | |
| { __( 'Visit Store', 'dokan-lite' ) } | |
| </a> | |
| <a | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| href={ window.dokan?.urls?.storeUrl || '#' } | |
| className="visit-store group skip-color-module flex items-center text-sm gap-2 font-medium text-[#25252D] focus:ring-2 focus:ring-blue-500 focus:outline-none py-4 px-5" | |
| > | |
| <LucideIcons.Globe size={ 16 } className="text-[#828282]" /> | |
| { __( 'Visit Store', 'dokan-lite' ) } | |
| </a> |
🤖 Prompt for AI Agents
In src/vendor-dashboard/layout/components/Header.tsx around lines 41 to 48, the
"Visit Store" anchor removes keyboard focus outlines, uses a non-functional '#'
fallback, and opens a new tab without rel attributes; change it to not render or
render a disabled element when window.dokan?.urls?.storeUrl is absent (or set
aria-disabled="true" and remove href), remove the utility that force-hides focus
(delete focus:!outline-none) so visible focus rings remain, only include
target="_blank" and add rel="noopener noreferrer" when a real storeUrl exists,
and ensure the element conveys disabled state to assistive tech (aria-disabled
and appropriate styling) instead of linking to '#'.
| <div | ||
| // eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions | ||
| onMouseEnter={ () => setIsMenuOpen( true ) } | ||
| onMouseLeave={ () => setIsMenuOpen( false ) } | ||
| onClick={ () => setIsMenuOpen( ! isMenuOpen ) } | ||
| className="header-avatar flex items-center gap-2.5 cursor-pointer py-4 px-5" | ||
| role="button" | ||
| tabIndex={ 0 } | ||
| ref={ setPopoverAnchor } | ||
| aria-haspopup="menu" | ||
| aria-expanded={ isMenuOpen } | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add keyboard event handler for button role.
Lines 51-52 disable ESLint warnings for keyboard accessibility, but the element has role="button" and tabIndex={0} without a keyboard event handler. Users navigating by keyboard can focus this element but cannot activate it with Enter or Space keys.
Add an onKeyDown handler:
<div
// eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions
onMouseEnter={ () => setIsMenuOpen( true ) }
onMouseLeave={ () => setIsMenuOpen( false ) }
onClick={ () => setIsMenuOpen( ! isMenuOpen ) }
+ onKeyDown={ ( e ) => {
+ if ( e.key === 'Enter' || e.key === ' ' ) {
+ e.preventDefault();
+ setIsMenuOpen( ! isMenuOpen );
+ }
+ } }
className="header-avatar flex items-center gap-2.5 cursor-pointer py-4 px-5"
role="button"
tabIndex={ 0 }📝 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.
| <div | |
| // eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions | |
| onMouseEnter={ () => setIsMenuOpen( true ) } | |
| onMouseLeave={ () => setIsMenuOpen( false ) } | |
| onClick={ () => setIsMenuOpen( ! isMenuOpen ) } | |
| className="header-avatar flex items-center gap-2.5 cursor-pointer py-4 px-5" | |
| role="button" | |
| tabIndex={ 0 } | |
| ref={ setPopoverAnchor } | |
| aria-haspopup="menu" | |
| aria-expanded={ isMenuOpen } | |
| > | |
| <div | |
| // eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions | |
| onMouseEnter={ () => setIsMenuOpen( true ) } | |
| onMouseLeave={ () => setIsMenuOpen( false ) } | |
| onClick={ () => setIsMenuOpen( ! isMenuOpen ) } | |
| onKeyDown={ ( e ) => { | |
| if ( e.key === 'Enter' || e.key === ' ' ) { | |
| e.preventDefault(); | |
| setIsMenuOpen( ! isMenuOpen ); | |
| } | |
| } } | |
| className="header-avatar flex items-center gap-2.5 cursor-pointer py-4 px-5" | |
| role="button" | |
| tabIndex={ 0 } | |
| ref={ setPopoverAnchor } | |
| aria-haspopup="menu" | |
| aria-expanded={ isMenuOpen } | |
| > |
🤖 Prompt for AI Agents
In src/vendor-dashboard/layout/components/Header.tsx around lines 50 to 61, the
focusable div has role="button" and tabIndex={0} but no keyboard handler; add an
onKeyDown handler that listens for Enter and Space keys and toggles
setIsMenuOpen (matching the onClick behavior), call preventDefault for Space to
avoid page scrolling, and stop propagation as appropriate; remove the need to
disable the jsx-a11y rules by keeping the handler and ensure aria-expanded is
kept in sync.
| <a | ||
| href={ decodeEntities( | ||
| item?.url || '#' | ||
| ) } | ||
| className="skip-color-module group flex items-center gap-2.5 px-4 py-2.5 text-sm text-[#828282] focus:!outline-none transition-colors duration-150" | ||
| onClick={ () => | ||
| setIsMenuOpen( false ) | ||
| } | ||
| role="menuitem" | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove focus outline suppression on menu items.
Line 102 uses focus:!outline-none which removes focus indicators for keyboard users navigating the menu.
Apply this diff:
<a
href={ decodeEntities(
item?.url || '#'
) }
- className="skip-color-module group flex items-center gap-2.5 px-4 py-2.5 text-sm text-[#828282] focus:!outline-none transition-colors duration-150"
+ className="skip-color-module group flex items-center gap-2.5 px-4 py-2.5 text-sm text-[#828282] focus:ring-2 focus:ring-inset focus:ring-blue-500 focus:outline-none transition-colors duration-150"
onClick={ () =>
setIsMenuOpen( false )
}🤖 Prompt for AI Agents
In src/vendor-dashboard/layout/components/Header.tsx around lines 98 to 107, the
anchor uses the Tailwind utility "focus:!outline-none" which suppresses keyboard
focus indicators; remove that utility and replace it with accessible focus
styling such as "focus:outline-none focus-visible:ring-2
focus-visible:ring-offset-2 focus-visible:ring-primary" (or your design system's
equivalent) so keyboard users retain a visible focus indicator; ensure any
existing CSS specificity is adjusted (drop the "!") and test keyboard navigation
to confirm the visible focus state appears on the menu items.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/vendor-dashboard/layout/components/Sidebar.tsx (5)
33-37: Improve type safety for popover state.The
submenuproperty usesanytype, reducing type safety. Consider defining a proper interface for the submenu structure based on thevendorDashboardLayoutConfigshape.+interface SubmenuItem { + url: string; + title: string; + counts?: number; +} + +interface PopoverState { + key: string; + submenu: Record<string, SubmenuItem>; + anchorRef: RefObject<HTMLElement>; +} + -const [ activePopover, setActivePopover ] = useState< { - key: string; - submenu: any; - anchorRef: RefObject< HTMLElement >; -} | null >( null ); +const [ activePopover, setActivePopover ] = useState< PopoverState | null >( null );
126-150: Define TypeScript interfaces for vendor dashboard config.Multiple uses of
anytype (lines 127, 133, 134, 138) reduce type safety. Consider defining proper interfaces for thevendorDashboardLayoutConfigstructure to catch potential runtime errors at compile time.Add type definitions (can be in a separate types file):
interface VendorDashboardLayoutConfig { siteInfo: { siteTitle: string; siteIcon?: string; }; vendor: { name: string; avatar?: string; }; subscription?: { name: string; status: string; }; sidebarNav: Record<string, SidebarNavItem>; } interface SidebarNavItem { title: string; url: string; icon?: string; icon_name?: string; counts?: number; submenu?: Record<string, SubmenuItem>; } interface SubmenuItem { title: string; url: string; counts?: number; }Then update line 127:
-const { siteInfo, vendor, subscription, sidebarNav } = - ( window as any )?.vendorDashboardLayoutConfig || {}; +const { siteInfo, vendor, subscription, sidebarNav } = + (window as any as { vendorDashboardLayoutConfig?: VendorDashboardLayoutConfig })?.vendorDashboardLayoutConfig || {};
442-442: Replace invisible icon hack with proper spacing.Using a transparent Settings icon as a spacer is fragile and creates unnecessary DOM elements. Use proper margin/padding instead.
-<LucideIcons.Settings className="w-5 h-5 !text-transparent" /> -<span className="ml-2"> +<span className="ml-7"> { subitem.title } </span>The
ml-7(28px) accounts for the parent icon width (20px) + its right margin (8px) for proper alignment.
254-467: Consider memoizing menu rendering for better performance.The menu structure is recalculated on every render. While unlikely to cause issues with typical menu sizes, consider using
useMemoto optimize complex calculations.const menuItems = useMemo( () => { return Object.entries( sidebarNav || {} ).map( ( [ key, item ]: any ) => { const hasSub = !! item?.submenu && Object.keys( item.submenu ).length > 0; // ... rest of menu item logic return { key, item, hasSub, /* other computed values */ }; } ); }, [ sidebarNav, currentUrl, collapsed, expanded ] );Then iterate over
menuItemsinstead of recalculating each render. However, this is a low-priority optimization given typical menu sizes.
1-565: Extract magic numbers to named constants for maintainability.Several magic numbers throughout the component would benefit from being named constants.
Add constants at the top of the file:
const SIDEBAR_CONFIG = { POPOVER_HIDE_DELAY: 200, // ms WIDTH_COLLAPSED: 96, // px WIDTH_EXPANDED: 250, // px MOBILE_BREAKPOINT: 600, // px TRUNCATE_TITLE: 9, // characters TRUNCATE_STORE_NAME: 16, TRUNCATE_SUBSCRIPTION: 10, } as const;Then use throughout the component:
hideTimeoutRef.current = setTimeout( () => { setActivePopover( null ); -}, 200 ); +}, SIDEBAR_CONFIG.POPOVER_HIDE_DELAY );document.documentElement.style.setProperty( '--dokan-sidebar-width', - `${ collapsed ? 96 : 250 }px` + `${ collapsed ? SIDEBAR_CONFIG.WIDTH_COLLAPSED : SIDEBAR_CONFIG.WIDTH_EXPANDED }px` );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/vendor-dashboard/layout/components/Header.tsx(1 hunks)src/vendor-dashboard/layout/components/Sidebar.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/vendor-dashboard/layout/components/Header.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/vendor-dashboard/layout/components/Sidebar.tsx (1)
src/utilities/Truncate.ts (1)
truncate(1-6)
🪛 ast-grep (0.40.0)
src/vendor-dashboard/layout/components/Sidebar.tsx
[warning] 350-350: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
src/vendor-dashboard/layout/components/Sidebar.tsx
[error] 351-351: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (2, 3)
🔇 Additional comments (3)
src/vendor-dashboard/layout/components/Sidebar.tsx (3)
1-15: LGTM: Imports are well-organized.The imports are appropriate for the component's functionality. The wildcard import of lucide-react icons is acceptable for dynamic icon rendering.
98-108: Previous duplicate call issue has been resolved.The duplicate
setActivePopover(null)call that was flagged in previous reviews has been fixed. The effect now correctly resets the popover state once when the sidebar is expanded.
497-546: MAJOR: Sanitize user-generated content before rendering with RawHTML.The
storeNameandsubscriptionNameare rendered usingRawHTMLcomponent without sanitization. Since these are user-generated content (vendor store names), this creates an XSS vulnerability if a malicious vendor sets their store name to include script tags or other HTML.Recommended solutions:
Option 1 (Preferred): Remove RawHTML if HTML rendering isn't needed
If store names are expected to be plain text, remove
RawHTML:<div className="store-name text-sm font-semibold"> { storeName ? ( <Tooltip - content={ - <RawHTML>{ storeName }</RawHTML> - } + content={ storeName } > - <span> - <RawHTML> - { truncate( - storeName, - 16 - ) } - </RawHTML> - </span> + <span>{ truncate( storeName, 16 ) }</span> </Tooltip>Apply similar changes for
subscriptionNameat lines 520-536.Option 2: Sanitize with DOMPurify if HTML support is required
+import DOMPurify from 'dompurify'; <Tooltip - content={ - <RawHTML>{ storeName }</RawHTML> - } + content={ DOMPurify.sanitize( storeName ) } > - <span> - <RawHTML> - { truncate( storeName, 16 ) } - </RawHTML> - </span> + <span dangerouslySetInnerHTML={{ + __html: DOMPurify.sanitize( truncate( storeName, 16 ) ) + }} /> </Tooltip>Verify whether
storeNameandsubscriptionNameare expected to contain HTML entities or plain text by checking the PHP backend:
| return ( | ||
| <> | ||
| <aside | ||
| style={ { top: windowWidth > 600 ? adminBar : 0 } } | ||
| className={ twMerge( | ||
| 'dokan-frontend-sidebar text-white fixed left-0 bottom-0 z-20 flex flex-col transition-all duration-200', | ||
| collapsed | ||
| ? windowWidth <= 768 | ||
| ? 'w-0 max-w-0' | ||
| : 'w-24 max-w-24' | ||
| : 'w-[250px] max-w-[250px]' | ||
| ) } | ||
| > | ||
| { /* === HEADER === */ } | ||
| <div | ||
| className={ twMerge( | ||
| 'flex items-center gap-3.5 min-h-20', | ||
| collapsed ? 'px-5 justify-center' : 'px-8' | ||
| ) } | ||
| > | ||
| { siteIcon ? ( | ||
| <img | ||
| src={ siteIcon } | ||
| className="h-8 w-8 rounded-md" | ||
| alt={ __( 'Vendor Dashboard Logo', 'dokan-lite' ) } | ||
| /> | ||
| ) : ( | ||
| <span | ||
| className={ twMerge( | ||
| 'grid h-8 w-8 place-items-center rounded-md site-icon', | ||
| collapsed && windowWidth <= 768 | ||
| ? 'hidden' | ||
| : 'w-8' | ||
| ) } | ||
| > | ||
| <LucideIcons.Globe size={ 20 } color="#FFF" /> | ||
| </span> | ||
| ) } | ||
|
|
||
| { ! collapsed && ( | ||
| <Tooltip content={ sideBarTitle }> | ||
| <span className="text-2xl font-bold text-white"> | ||
| { truncate( sideBarTitle, 9 ) } | ||
| </span> | ||
| </Tooltip> | ||
| ) } | ||
| </div> | ||
|
|
||
| { /* === MENU === */ } | ||
| <div | ||
| className={ twMerge( | ||
| 'flex-1 overflow-y-auto dokan-vendor-sidebar-scroll', | ||
| collapsed && windowWidth <= 768 ? 'p-0' : 'p-5' | ||
| ) } | ||
| > | ||
| <nav> | ||
| <ul className="flex flex-col gap-1.5 list-none p-0"> | ||
| { Object.entries( sidebarNav || {} ).map( | ||
| ( [ key, item ]: any ) => { | ||
| const hasSub = | ||
| !! item?.submenu && | ||
| Object.keys( item.submenu ).length > 0; | ||
|
|
||
| // Parent is active if it has no submenu and its URL matches current | ||
| const isParentActive = | ||
| ! hasSub && | ||
| item?.url && | ||
| currentUrl.startsWith( item.url ); | ||
|
|
||
| // Detect if any child submenu item is active | ||
| let hasActiveChild = false; | ||
| if ( hasSub ) { | ||
| Object.values( item.submenu ).forEach( | ||
| ( sub: any ) => { | ||
| if ( | ||
| sub?.url && | ||
| currentUrl.startsWith( | ||
| sub.url | ||
| ) | ||
| ) { | ||
| hasActiveChild = true; | ||
| } | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| // In collapsed state, mark parent as active when a child is active | ||
| const isParentActiveCollapsed = | ||
| collapsed && hasSub && hasActiveChild; | ||
|
|
||
| const isExpanded = Boolean( | ||
| expanded[ key ] | ||
| ); | ||
|
|
||
| const menuItemRef = getMenuItemRef( key ); | ||
|
|
||
| return ( | ||
| <li | ||
| key={ key } | ||
| ref={ menuItemRef as any } | ||
| className={ twMerge( | ||
| 'relative', | ||
| collapsed && | ||
| 'flex justify-center' | ||
| ) } | ||
| onMouseEnter={ () => { | ||
| if ( hasSub ) { | ||
| showPopover( | ||
| key, | ||
| item.submenu | ||
| ); | ||
| } | ||
| } } | ||
| onMouseLeave={ hidePopover } | ||
| > | ||
| <a | ||
| href={ item.url } | ||
| onClick={ ( e ) => { | ||
| if ( | ||
| hasSub && | ||
| ! collapsed | ||
| ) { | ||
| e.preventDefault(); | ||
| toggleSubmenu( key ); | ||
| } | ||
| } } | ||
| className={ twMerge( | ||
| 'group skip-color-module relative flex items-center rounded-md font-medium focus:!outline-none py-2.5', | ||
| collapsed | ||
| ? windowWidth <= 768 | ||
| ? 'w-0 max-w-0' | ||
| : 'w-10 max-w-10 justify-center' | ||
| : 'text-sm px-3', | ||
| ( isParentActive || | ||
| isParentActiveCollapsed ) && | ||
| 'active' | ||
| ) } | ||
| > | ||
| { /* Icon: turn white when its popover is visible */ } | ||
| <span | ||
| className={ twMerge( | ||
| activePopover?.key === | ||
| key && collapsed | ||
| ? 'text-white' | ||
| : '' | ||
| ) } | ||
| > | ||
| { /* Render custom images or icons from the sidebar config */ } | ||
| { item.icon && | ||
| item.icon.startsWith( | ||
| '<img' | ||
| ) ? ( | ||
| <span | ||
| dangerouslySetInnerHTML={ { | ||
| __html: item.icon, | ||
| } } | ||
| /> | ||
| ) : ( | ||
| getIcon( | ||
| item.icon_name, | ||
| Boolean( | ||
| activePopover?.key === | ||
| key && | ||
| collapsed | ||
| ) | ||
| ) | ||
| ) } | ||
| </span> | ||
| { ! collapsed && ( | ||
| <span className="ml-2"> | ||
| { item.title } | ||
| </span> | ||
| ) } | ||
| { item.counts > 0 && ( | ||
| <CountBubble | ||
| count={ item.counts } | ||
| isCollapsed={ | ||
| collapsed | ||
| } | ||
| /> | ||
| ) } | ||
| { hasSub && | ||
| ! collapsed && | ||
| ( isExpanded ? ( | ||
| <LucideIcons.ChevronUp | ||
| className={ twMerge( | ||
| 'ml-2 w-4 h-4 text-[#A5A5A5] group-hover:text-white transition-transform duration-200', | ||
| item.counts > 0 | ||
| ? 'ml-2' | ||
| : 'ml-auto' | ||
| ) } | ||
| /> | ||
| ) : ( | ||
| <LucideIcons.ChevronDown | ||
| className={ twMerge( | ||
| 'ml-2 w-4 h-4 text-[#A5A5A5] group-hover:text-white transition-transform duration-200', | ||
| item.counts > 0 | ||
| ? 'ml-2' | ||
| : 'ml-auto' | ||
| ) } | ||
| /> | ||
| ) ) } | ||
| </a> | ||
|
|
||
| { /* Expanded submenu (inline) with smooth animation */ } | ||
| { ! collapsed && hasSub && ( | ||
| <div | ||
| className={ twMerge( | ||
| 'overflow-hidden', | ||
| isExpanded | ||
| ? 'max-h-full opacity-100' | ||
| : 'max-h-0 opacity-0 pointer-events-none' | ||
| ) } | ||
| > | ||
| <ul className="mt-2 mx-0 space-y-1.5 list-none p-0"> | ||
| { Object.entries( | ||
| item.submenu | ||
| ).map( | ||
| ( [ | ||
| subkey, | ||
| subitem, | ||
| ]: any ) => { | ||
| const isSubActive = | ||
| subitem?.url && | ||
| currentUrl.startsWith( | ||
| subitem.url | ||
| ); | ||
|
|
||
| return ( | ||
| <li | ||
| key={ | ||
| subkey | ||
| } | ||
| > | ||
| <a | ||
| href={ | ||
| subitem.url | ||
| } | ||
| className={ twMerge( | ||
| 'group skip-color-module flex items-center py-2.5 px-3 text-sm font-medium rounded-md focus:!outline-none', | ||
| isSubActive && | ||
| 'active' | ||
| ) } | ||
| > | ||
| <LucideIcons.Settings className="w-5 h-5 !text-transparent" /> | ||
| <span className="ml-2"> | ||
| { | ||
| subitem.title | ||
| } | ||
| </span> | ||
| { subitem.counts > | ||
| 0 && ( | ||
| <CountBubble | ||
| count={ | ||
| subitem.counts | ||
| } | ||
| /> | ||
| ) } | ||
| </a> | ||
| </li> | ||
| ); | ||
| } | ||
| ) } | ||
| </ul> | ||
| </div> | ||
| ) } | ||
| </li> | ||
| ); | ||
| } | ||
| ) } | ||
| </ul> | ||
| </nav> | ||
| </div> | ||
|
|
||
| <hr className="m-0 w-full h-[1px]" /> | ||
|
|
||
| { /* === FOOTER === */ } | ||
| <div | ||
| className={ twMerge( | ||
| 'py-4', | ||
| ! collapsed ? 'px-8' : 'px-6 flex justify-center' | ||
| ) } | ||
| > | ||
| <div className="flex items-center gap-2.5 focus:!outline-none"> | ||
| { storeAvatar ? ( | ||
| <img | ||
| src={ storeAvatar } | ||
| alt={ | ||
| storeName || | ||
| __( 'Store Image', 'dokan-lite' ) | ||
| } | ||
| className="h-10 w-10 rounded-full" | ||
| /> | ||
| ) : ( | ||
| <div className="grid h-10 w-10 place-items-center rounded-full bg-gradient-to-br from-orange-500 to-orange-600"> | ||
| <span className="h-2.5 w-2.5 rotate-45 rounded-sm bg-white/90" /> | ||
| </div> | ||
| ) } | ||
|
|
||
| { ! collapsed && ( | ||
| <div className="sidebar-footer leading-tight space-y-1"> | ||
| <div className="store-name text-sm font-semibold"> | ||
| { storeName ? ( | ||
| <Tooltip | ||
| content={ | ||
| <RawHTML>{ storeName }</RawHTML> | ||
| } | ||
| > | ||
| <span> | ||
| <RawHTML> | ||
| { truncate( | ||
| storeName, | ||
| 16 | ||
| ) } | ||
| </RawHTML> | ||
| </span> | ||
| </Tooltip> | ||
| ) : ( | ||
| __( 'Your Store', 'dokan-lite' ) | ||
| ) } | ||
| </div> | ||
| { subscriptionName && ( | ||
| <div className="subscription-info text-xs flex items-center gap-1"> | ||
| <Tooltip | ||
| content={ | ||
| <RawHTML> | ||
| { subscriptionName } | ||
| </RawHTML> | ||
| } | ||
| > | ||
| <span> | ||
| <RawHTML> | ||
| { truncate( | ||
| subscriptionName, | ||
| 10 | ||
| ) } | ||
| </RawHTML> | ||
| </span> | ||
| </Tooltip> | ||
| { subscriptionStatus && | ||
| sprintf( | ||
| /* translators: %1$s: Subscription status */ | ||
| __( '(%1$s)', 'dokan-lite' ), | ||
| subscriptionStatus | ||
| ) } | ||
| </div> | ||
| ) } | ||
| </div> | ||
| ) } | ||
| </div> | ||
| </div> | ||
| </aside> | ||
|
|
||
| { /* === POPOVER === */ } | ||
| { collapsed && activePopover && ( | ||
| <SubmenuPopover | ||
| submenu={ activePopover.submenu } | ||
| anchorRef={ activePopover.anchorRef } | ||
| onMouseEnter={ keepPopoverVisible } | ||
| onMouseLeave={ hidePopover } | ||
| onClose={ closePopover } | ||
| /> | ||
| ) } | ||
| </> | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add keyboard navigation and ARIA attributes for accessibility.
The sidebar lacks keyboard navigation support and proper ARIA attributes, making it inaccessible to keyboard and screen reader users.
Required improvements:
- Keyboard navigation for popovers - In collapsed mode, hovering shows popovers but there's no keyboard alternative. Add keyboard handlers:
onKeyDown={ ( e ) => {
if ( hasSub && collapsed && ( e.key === 'Enter' || e.key === ' ' ) ) {
e.preventDefault();
showPopover( key, item.submenu );
}
if ( e.key === 'Escape' ) {
closePopover();
}
} }- ARIA attributes - Add proper semantic attributes:
<a
href={ item.url }
+ role="button"
+ aria-expanded={ hasSub && !collapsed ? isExpanded : undefined }
+ aria-haspopup={ hasSub ? 'true' : undefined }
onClick={ ( e ) => {- Focus management - Replace
focus:!outline-nonewith visible focus indicators:
-'group skip-color-module relative flex items-center rounded-md font-medium focus:!outline-none py-2.5',
+'group skip-color-module relative flex items-center rounded-md font-medium focus:outline-2 focus:outline-offset-2 focus:outline-white py-2.5',- Navigation landmark - Already uses
<nav>but consider addingaria-label:
-<nav>
+<nav aria-label={ __( 'Vendor Dashboard Navigation', 'dokan-lite' ) }>🧰 Tools
🪛 ast-grep (0.40.0)
[warning] 350-350: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
[error] 351-351: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
In src/vendor-dashboard/layout/components/Sidebar.tsx around lines 198 to 562,
the sidebar lacks keyboard navigation and ARIA semantics; add keyboard handlers
and ARIA attributes so keyboard and screen-reader users can interact with
popovers and submenus: attach an onKeyDown handler to the menu item anchor or li
that, when collapsed and hasSub, opens the popover on Enter or Space
(preventDefault), closes it on Escape (calling closePopover), and moves focus to
the first focusable submenu item when opening; add aria-haspopup="true" and
aria-expanded={isExpanded || (collapsed && activePopover?.key === key)} to items
with submenus, give the <nav> an aria-label (e.g., "Vendor sidebar"), set
role="menu" on the nav and role="menuitem" on actionable menu entries (and
role="menu" on inline/popover submenu containers), ensure submenu items are
focusable (tabIndex={-1} when hidden, 0 when shown) and that popover container
traps/returns focus on close; remove focus:!outline-none and replace it with
visible focus styles (e.g., class that shows a clear outline or ring) so
keyboard focus is always visible.
| { item.icon && | ||
| item.icon.startsWith( | ||
| '<img' | ||
| ) ? ( | ||
| <span | ||
| dangerouslySetInnerHTML={ { | ||
| __html: item.icon, | ||
| } } | ||
| /> | ||
| ) : ( | ||
| getIcon( | ||
| item.icon_name, | ||
| Boolean( | ||
| activePopover?.key === | ||
| key && | ||
| collapsed | ||
| ) | ||
| ) | ||
| ) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrabbani we are concern about such issue?
@MdAsifHossainNadim, please have a look here..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| { item.icon && | ||
| item.icon.startsWith( | ||
| '<img' | ||
| ) ? ( | ||
| <span | ||
| dangerouslySetInnerHTML={ { | ||
| __html: item.icon, | ||
| } } | ||
| /> | ||
| ) : ( | ||
| getIcon( | ||
| item.icon_name, | ||
| Boolean( | ||
| activePopover?.key === | ||
| key && | ||
| collapsed | ||
| ) | ||
| ) | ||
| ) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrabbani we are concern about such issue?
@MdAsifHossainNadim, please have a look here..
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
New Features
Style
Settings
Navigation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.