fix: refactor vendor table with VendorCard, update styles and labels#2940
fix: refactor vendor table with VendorCard, update styles and labels#2940
VendorCard, update styles and labels#2940Conversation
WalkthroughIntroduces a new VendorCard React component for rendering vendor information with avatar, name, and email. Integrates it into the vendors dashboard page, updates vendor table column configuration, modifies action buttons for vendor approval and disabling, adjusts styling for table cells, and adds a CSS rule for popover styling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The vendors.tsx file contains multiple interrelated changes including component integration, button logic restructuring, and styling updates requiring careful verification. The VendorCard component and styling changes are straightforward, but the action button logic swap and confirmation flow mapping need attention to ensure correct vendor approval/disabling behavior. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 3
🧹 Nitpick comments (8)
src/admin/dashboard/components/VendorCard.tsx (5)
4-4: Add explicit TypeScript props and default valuesProps are untyped, reducing safety and DX in TSX.
-import { DokanLink } from '@src/components'; -import { twMerge } from 'tailwind-merge'; - -function VendorCard( { name, avatar, isLoading, loadingClass, onClick, email = '' } ) { +import { DokanLink } from '@src/components'; +import { twMerge } from 'tailwind-merge'; +import type { MouseEventHandler } from 'react'; + +interface VendorCardProps { + name: string; + avatar?: string; + email?: string; + isLoading?: boolean; + loadingClass?: string; + onClick?: MouseEventHandler<HTMLDivElement>; +} + +function VendorCard({ + name, + avatar, + email = '', + isLoading = false, + loadingClass = '', + onClick, +}: VendorCardProps) {
9-13: Remove non-functionalobject-coverfrom a div; ensure overflow/rounding consistency
object-coverhas no effect on a div. Also align rounding with the image.- className={ twMerge( - 'w-[44px] h-[44px] rounded object-cover', - isLoading ? `${ loadingClass }` : '' - ) } + className={ twMerge( + 'w-[44px] h-[44px] rounded-[5px] overflow-hidden', + isLoading ? loadingClass : '' + ) }
14-21: Improve image performance and stabilityAdd intrinsic size to prevent CLS, lazy-load, and decode asap. Keep alt.
- <img - src={ avatar } - alt={ name || 'Store avatar' } - className={ twMerge( - 'w-[44px] h-[44px] rounded-[5px] object-cover border-[1px] border-[#E9E9E9] border-solid', - isLoading ? 'opacity-0' : 'opacity-100' - ) } - /> + <img + src={ avatar } + alt={ name || 'Store avatar' } + width={44} + height={44} + loading="lazy" + decoding="async" + className={ twMerge( + 'w-[44px] h-[44px] rounded-[5px] object-cover border border-[#E9E9E9]', + isLoading ? 'opacity-0' : 'opacity-100' + ) } + />
30-34: Make the clickable name accessible (link semantics + focus styles)
DokanLink as="div"+ onClick yields a non-focusable control. Prefer anchor or button semantics.Option A (anchor): if
DokanLinksupportsto/href-<DokanLink as="div" onClick={ onClick } className="cursor-pointer"> +<DokanLink to="#" onClick={ onClick } className="cursor-pointer focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-dokan-primary">Option B (div): add role, tabIndex, and keyboard handling inside DokanLink or wrapper.
-<DokanLink as="div" onClick={ onClick } className="cursor-pointer"> +<DokanLink + as="div" + onClick={ onClick } + role="link" + tabIndex={0} + onKeyDown={ (e) => { if (e.key === 'Enter' || e.key === ' ') onClick?.(e as any); } } + className="cursor-pointer focus-visible:outline focus-visible:outline-2 focus-visible:outline-dokan-primary" +>Also applies to: 35-43
37-44: Replace hard-coded brand hex with token; add truncation for long names
text-[#7047EB]hardcodes brand; names may overflow the cell.- 'text-[14px] font-[600] text-[#7047EB]', + 'text-[14px] font-semibold text-dokan-primary truncate',Also consider
title={name}on the element to show full name on hover.src/admin/dashboard/pages/vendors.tsx (3)
35-36: Email removed from fields: verify intent vs VendorCard still showing emailYou removed the Email column but still render
Confirm product intent: hide email entirely from the list, or just as a dedicated column? If hide entirely, omit passing
Conditionally pass email based on policy (e.g., a flag or
item.show_email).
17-18: Import style inconsistency confirmed; standardize within fileThe file mixes import styles: lines 12–13 use relative imports (
../../../components/modals/DokanModal,../../../definitions/dokan-vendor), while line 17 uses the alias-based import (admin/dashboard/components/VendorCard). Theadmin/*alias is configured in tsconfig and valid, but inconsistent usage within the same file is brittle.Standardize to one approach: either use the alias for all admin components or revert to relative imports for consistency.
643-651: Optional refactor: Add null check for Icon to improve defensive codingThe 'Check' icon is available in lucide-react v0.488.0, so the current code at line 643–651 will work without issues. However, the
getActionLabelfunction usesIcondirectly as a JSX component without validating it exists, which means passing an invalid icon name would cause a React error. Adding the suggested guard is a good defensive practice:const Icon = LucideIcons[ iconName ]; +if ( !Icon ) { return <span className="inline-flex items-center gap-2.5">{ label }</span>; }Apply the same guard to the similar function in
src/admin/dashboard/pages/withdraw/index.tsx:249.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/admin/dashboard/components/VendorCard.tsx(1 hunks)src/admin/dashboard/pages/vendors.tsx(7 hunks)src/admin/dashboard/style.scss(1 hunks)src/components/AdminTab.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/admin/dashboard/pages/vendors.tsx (1)
src/definitions/dokan-vendor.ts (1)
Vendor(101-160)
⏰ 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 (4)
src/admin/dashboard/pages/vendors.tsx (4)
296-299: LGTM on cell typographyThe phone cell styling (
text-[14px] font-[400] text-[#575757]) aligns with the updated scheme and loading class merge is correct.
322-324: LGTM on registered date stylingConsistent with phone cell; works well with loading skeleton.
455-466: Header tweaks read wellCompact header, semantic h2, and “Add Vendor” label look good.
645-671: Inconsistency confirmed: single-vendor and bulk vendor status payloads use different formatsThe codebase shows an actual mismatch at lines 500–522:
- Single-vendor (lines 500–504): sends
{ status: 'active'|'inactive' }to PUT/dokan/v1/stores/{id}- Bulk (lines 515, 521): sends
{ approved: ids }or{ pending: ids }to POST/dokan/v1/stores/batchSingle-vendor format is confirmed correct via API documentation (expects
'active'|'inactive'). However, the batch endpoint payload format ({ approved/pending: ids }) requires verification—confirm whether this is the correct format POST/dokan/v1/stores/batchexpects, then map consistently (either convert batch responses to single-vendor format or vice versa).
| <VendorCard | ||
| name={ name } | ||
| avatar={ avatar } | ||
| isLoading={ isLoading } | ||
| loadingClass={ loadingClass } | ||
| onClick={ () => { | ||
| navigate( `/vendors/${ item.id }` ); | ||
| } } | ||
| email={ item?.email || '' } | ||
| /> | ||
| ); |
There was a problem hiding this comment.
Respect vendor email visibility preference (privacy)
If Vendor.show_email is false, we should avoid showing email even in the nested card.
- <VendorCard
- name={ name }
- avatar={ avatar }
- isLoading={ isLoading }
- loadingClass={ loadingClass }
- onClick={ () => { navigate( `/vendors/${ item.id }` ); } }
- email={ item?.email || '' }
- />
+ <VendorCard
+ name={ name }
+ avatar={ avatar }
+ isLoading={ isLoading }
+ loadingClass={ loadingClass }
+ onClick={ () => { navigate( `/vendors/${ item.id }` ); } }
+ email={ item?.show_email ? ( item?.email || '' ) : '' }
+ />If admins should always see emails, gate this behind a filter/setting, e.g., applyFilters('dokan_admin_vendors_show_email', true). Based on the Vendor type.
📝 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.
| <VendorCard | |
| name={ name } | |
| avatar={ avatar } | |
| isLoading={ isLoading } | |
| loadingClass={ loadingClass } | |
| onClick={ () => { | |
| navigate( `/vendors/${ item.id }` ); | |
| } } | |
| email={ item?.email || '' } | |
| /> | |
| ); | |
| <VendorCard | |
| name={ name } | |
| avatar={ avatar } | |
| isLoading={ isLoading } | |
| loadingClass={ loadingClass } | |
| onClick={ () => { | |
| navigate( `/vendors/${ item.id }` ); | |
| } } | |
| email={ item?.show_email ? ( item?.email || '' ) : '' } | |
| /> | |
| ); |
🤖 Prompt for AI Agents
In src/admin/dashboard/pages/vendors.tsx around lines 271 to 281, the VendorCard
is always passed item?.email which can violate a vendor's privacy preference;
change the email prop to respect Vendor.show_email by passing item?.email only
when item?.show_email is true, otherwise pass an empty string, and if admins
should override this behavior gate it with
applyFilters('dokan_admin_vendors_show_email', true) (import/use applyFilters
and ensure Vendor type has show_email), so the final email value is
item?.show_email || applyFilters(...) ? item?.email ?? '' : ''.
src/admin/dashboard/style.scss
Outdated
| .css-1bnxq3m-PopoverInnerWrapper { | ||
| padding: 0 !important; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Avoid targeting hashed class names and using !important for Popover padding
.css-1bnxq3m-PopoverInnerWrapper is a generated/emotion hash and can change between builds or WP versions. The override is fragile and global due to !important.
Prefer a stable selector or component-level class:
- If you can add a class to the Popover: style that class’
.components-popover__content. - Otherwise scope to a stable WP selector under our root.
Example refactor:
-.css-1bnxq3m-PopoverInnerWrapper {
- padding: 0 !important;
-}
+:where(#dokan-admin-dashboard).dokan-admin-vendors
+ :where(.components-popover__content){
+ padding: 0;
+}Or add a dedicated marker on the Popover and scope to it:
.dokan-popover--compact .components-popover__content { padding: 0; }Run to ensure no other hashed selectors slipped in:
🏁 Script executed:
#!/bin/bash
rg -nP '\.css-\w+-' -g '!**/dist/**'Length of output: 68
---
🏁 Script executed:
```shell
#!/bin/bash
# Check for Popover imports and usage to identify component source
rg -nP '(import|from).*[Pp]opover' src/admin/dashboard/ -A2 -B2
Length of output: 953
🏁 Script executed:
#!/bin/bash
# Check if .components-popover selector exists anywhere in the codebase
rg -nP '\.components-popover' -g '!**/dist/**'Length of output: 566
🏁 Script executed:
#!/bin/bash
# Check the full context around the problematic selector
cat src/admin/dashboard/style.scss | sed -n '90,100p'Length of output: 116
🏁 Script executed:
#!/bin/bash
# Find Dokan Popover component implementation
fd -n 'Popover' --type f src/ | grep -i 'component' | head -20Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check if the problematic CSS is scoped to #dokan-admin-dashboard
rg -nP '#dokan-admin-dashboard' src/admin/dashboard/style.scssLength of output: 83
🏁 Script executed:
#!/bin/bash
# Check larger context of the problem line to see if it's scoped
cat src/admin/dashboard/style.scss | sed -n '80,100p'Length of output: 294
---
🏁 Script executed:
```shell
#!/bin/bash
# Check MonthPicker and CategorySelector usage context
rg -nP '(MonthPicker|CategorySelector)' src/admin/dashboard/ -B5 -A5 --type tsx --type jsx | head -50
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check if #dokan-admin-dashboard wraps the page (check render structure)
fd 'index.tsx' src/admin/dashboard/pages/dashboard/ -x cat {}Length of output: 53097
🏁 Script executed:
#!/bin/bash
# Check if there are any other !important uses in the file for comparison
rg -nP '!important' src/admin/dashboard/style.scssLength of output: 62
Replace fragile hashed selector with stable .components-popover__content class
The generated emotion hash .css-1bnxq3m-PopoverInnerWrapper is unscoped and changes between builds. The codebase already normalizes Popover padding using .components-popover__content elsewhere (vendor-dashboard, admin-dashboard), making that selector a stable alternative.
Recommended fix:
-.css-1bnxq3m-PopoverInnerWrapper {
- padding: 0 !important;
-}
+:where(#dokan-admin-dashboard) .components-popover__content {
+ padding: 0;
+}Or add a custom class directly to the Popover component and style that.
📝 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.
| .css-1bnxq3m-PopoverInnerWrapper { | |
| padding: 0 !important; | |
| } | |
| :where(#dokan-admin-dashboard) .components-popover__content { | |
| padding: 0; | |
| } |
🤖 Prompt for AI Agents
In src/admin/dashboard/style.scss around lines 95 to 97, replace the fragile
generated selector `.css-1bnxq3m-PopoverInnerWrapper { padding: 0 !important; }`
with the stable selector `.components-popover__content { padding: 0; }` (or add
a custom class to the Popover component and target that class instead) — remove
the reliance on the emotion hash, drop the unnecessary `!important` if
specificity is sufficient, or increase selector specificity if needed to ensure
the rule applies.
| const defaultTabClassName = | ||
| 'border-0 border-b border-solid mr-5 space-x-8 whitespace-nowrap px-1 py-8 text-sm font-medium cursor-pointer hover:bg-transparent focus:outline-none text-gray-500 border-gray-200 hover:text-gray-600 hover:border-gray-300'; | ||
| 'border-0 border-b border-solid mr-5 space-x-8 whitespace-nowrap px-1 py-8 text-sm font-medium cursor-pointer hover:bg-transparent focus:outline-none text-[#575757] border-gray-200 hover:text-gray-600 hover:border-gray-300'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Replace hard-coded hex with a theme token/palette color
text-[#575757] hardcodes color and may diverge from theme/dark mode.
Use a token or Tailwind color already used elsewhere (e.g., text-gray-600), or a CSS var fallback:
- '... text-[#575757] border-gray-200 hover:text-gray-600 ...';
+ '... text-gray-600 border-gray-200 hover:text-gray-700 ...';Check for other hard-coded #575757 to consolidate:
🏁 Script executed:
#!/bin/bash
rg -n "#575757" -g "!**/dist/**"Length of output: 2532
Consolidate hard-coded #575757 across codebase to use Tailwind color tokens
The color text-[#575757] appears 15+ times across 9 files. Replace with text-gray-600 for consistency and theme support:
In src/components/AdminTab.tsx:32:
- '... text-[#575757] border-gray-200 hover:text-gray-600 ...';
+ '... text-gray-600 border-gray-200 hover:text-gray-700 ...';Apply the same consolidation to:
src/components/commission/CommissionInputs.tsx(3 occurrences)src/components/commission/CommissionHeader.tsxsrc/components/commission/CategoryRow.tsx(2 occurrences)src/admin/onboard/screens/WelcomeScreen.tsxsrc/admin/onboard/screens/BasicInfoScreen.tsxsrc/admin/dashboard/pages/withdraw/index.tsx(2 occurrences)src/admin/dashboard/pages/vendors.tsx(2 occurrences)src/admin/pages/ChangeLog.vue
📝 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.
| const defaultTabClassName = | |
| 'border-0 border-b border-solid mr-5 space-x-8 whitespace-nowrap px-1 py-8 text-sm font-medium cursor-pointer hover:bg-transparent focus:outline-none text-gray-500 border-gray-200 hover:text-gray-600 hover:border-gray-300'; | |
| 'border-0 border-b border-solid mr-5 space-x-8 whitespace-nowrap px-1 py-8 text-sm font-medium cursor-pointer hover:bg-transparent focus:outline-none text-[#575757] border-gray-200 hover:text-gray-600 hover:border-gray-300'; | |
| const defaultTabClassName = | |
| 'border-0 border-b border-solid mr-5 space-x-8 whitespace-nowrap px-1 py-8 text-sm font-medium cursor-pointer hover:bg-transparent focus:outline-none text-gray-600 border-gray-200 hover:text-gray-700 hover:border-gray-300'; |
🤖 Prompt for AI Agents
In src/components/AdminTab.tsx around lines 31 to 33, replace the hard-coded
tailwind class text-[#575757] with the token text-gray-600; do the same across
the codebase for all occurrences (CommissionInputs.tsx x3, CommissionHeader.tsx,
CategoryRow.tsx x2, WelcomeScreen.tsx, BasicInfoScreen.tsx, withdraw/index.tsx
x2, vendors.tsx x2, and src/admin/pages/ChangeLog.vue) so they use text-gray-600
for consistency and theme support; ensure you only update the class name
(preserve other classes), run a quick search to confirm all 15+ replacements
were made, and run the app/build to verify no styling regressions.
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