-
Notifications
You must be signed in to change notification settings - Fork 77
WIP: React migration #174
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
base: develop
Are you sure you want to change the base?
WIP: React migration #174
Conversation
…tions, quick amount buttons, and price parsing fixes
…ality with new hooks and components
… CustomerModal, enhance Cart component with customer selection functionality
…ed list/grid views for better product display
…ness and styling consistency, enhancing user experience across the application
WalkthroughThis PR migrates the POS frontend from Vue.js to React with TypeScript and Tailwind CSS, introduces WordPress data stores and React components, removes legacy CSS/JS assets, adds new configuration and documentation, and updates build workflows to pnpm and wp-scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant WP as WordPress
participant RA as ReactAssets
participant B as Browser
WP->>RA: wp_enqueue_scripts
RA->>RA: is_dev_mode()
alt dev mode
RA->>B: enqueue HMR runtime + main script from dev server
else production
RA->>B: enqueue built JS & CSS assets
end
RA->>B: localize script with REST & settings data
sequenceDiagram
participant User as Shopper
participant Home as HomePage
participant APIo as posAPI.orders
participant WC as WooCommerce
participant APIp as posAPI.payment
participant Receipt as ReceiptModal
User->>Home: Click "Checkout"
Home->>APIo: createOrder(orderData)
APIo->>WC: POST /orders
WC-->>APIo: orderResponse
Home->>APIp: processPayment(paymentData)
APIp->>WC: POST /payments
WC-->>APIp: paymentResponse
APIp-->>Home: paymentResult
Home->>Receipt: show receipt with data
Receipt->>User: display & print dialog
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 56
🧹 Nitpick comments (47)
bin/zip.sh (1)
42-43
: Verify if both build scripts are needed
You’re running bothpnpm run dev:build
andpnpm run build
. Ifbuild
already encompassesdev:build
, you can simplify; otherwise ensure the order is intentional.src/frontend/App.tsx (1)
1-17
: LGTM! Consider adding error boundaries and lazy loading.The routing setup is clean and follows React Router v6 best practices. The component structure and TypeScript typing are correct.
Consider these enhancements for production readiness:
import React from 'react'; -import { Routes, Route } from 'react-router-dom'; -import HomePage from './pages/Home'; -import OrdersPage from './pages/Orders'; -import CustomersPage from './pages/Customers'; +import { Routes, Route } from 'react-router-dom'; +import { Suspense, lazy } from 'react'; + +// Lazy load pages for better performance +const HomePage = lazy(() => import('./pages/Home')); +const OrdersPage = lazy(() => import('./pages/Orders')); +const CustomersPage = lazy(() => import('./pages/Customers')); const App: React.FC = () => { return ( + <Suspense fallback={<div>Loading...</div>}> <Routes> <Route path="/" element={<HomePage />} /> <Route path="/orders" element={<OrdersPage />} /> <Route path="/customers" element={<CustomersPage />} /> </Routes> + </Suspense> ); };src/frontend/store/products/store.ts (1)
8-16
: LGTM! Consider adding error handling for store registration.The store creation and registration is properly implemented using WordPress data patterns.
Consider adding error handling for store registration:
// Register the store -register(store); +try { + register(store); +} catch (error) { + console.error('Failed to register products store:', error); +}src/frontend/components/SearchBar.tsx (1)
15-17
: Make search icon clickable and accessible.The search icon should be interactive and accessible.
- <span className="text-wepos-primary hover:text-wepos-primary-hover absolute top-1/2 right-3 -translate-y-1/2 transform cursor-pointer transition-colors"> + <button + type="button" + => onSearch?.(query)} + className="text-wepos-primary hover:text-wepos-primary-hover absolute top-1/2 right-3 -translate-y-1/2 transform cursor-pointer transition-colors focus:outline-none focus:ring-2 focus:ring-wepos-primary/20 rounded" + aria-label={__('Search', 'wepos')} + > <Search className="h-4 w-4" /> - </span> + </button>src/frontend/pages/Orders.tsx (1)
5-30
: LGTM! Well-structured placeholder page.The page structure is clean and follows good React patterns. The placeholder content clearly indicates this is a work-in-progress component.
Since this is a placeholder for order management functionality, would you like me to generate a more comprehensive orders page structure with:
- Order list table
- Order filtering/search
- Order status management
- Integration with the WordPress data stores
This could help accelerate the development of the orders management feature.
src/frontend/index.tsx (1)
3-3
: Remove unused import.
BrowserRouter
is imported but not used. OnlyHashRouter
is utilized in the code.-import { BrowserRouter, HashRouter } from 'react-router-dom'; +import { HashRouter } from 'react-router-dom';src/frontend/hooks/useCart.ts (1)
95-119
: Consider consolidating addToCart and addToCartItem functions.These two functions have very similar logic and could potentially be unified to reduce code duplication.
Consider creating a unified function that handles both cases:
+const addItemToCart = useCallback((item: POSProduct | POSCartItem, isProduct = false) => { + if (isProduct && !hasStock(item as POSProduct)) { + onError?.('Product is out of stock!'); + return; + } + + const productId = 'product_id' in item ? item.product_id : item.id; + const variationId = 'variation_id' in item ? item.variation_id : undefined; + + const existingItemIndex = cartData.line_items.findIndex( + cartItem => cartItem.product_id === productId && + cartItem.variation_id === variationId + ); + + // ... rest of unified logic +}, [cartData.line_items, setCartData]);src/frontend/store/cart/index.ts (1)
9-10
: Clarify or implement the auto-registration functionality.The comment mentions "Auto-register the store" but there's no actual registration code visible. This could be misleading for other developers.
Either implement the auto-registration or clarify the comment:
-// Auto-register the store +// Default export for store registration elsewhere export default store;Or if auto-registration should happen here:
// Auto-register the store +import { register } from '@wordpress/data'; +register(store); export default store;src/frontend/components/Layout.tsx (1)
8-8
: Consider extracting localStorage key as a constant.The storage key is well-defined and could potentially be shared across the application if other components need to read this state.
Consider moving storage keys to a constants file if they might be reused:
+// In src/frontend/constants/storage.ts +export const STORAGE_KEYS = { + SIDEBAR_COLLAPSED: 'wepos-sidebar-collapsed', +} as const; // Then import and use: +import { STORAGE_KEYS } from '../constants/storage'; -const SIDEBAR_STORAGE_KEY = 'wepos-sidebar-collapsed'; +const SIDEBAR_STORAGE_KEY = STORAGE_KEYS.SIDEBAR_COLLAPSED;Also applies to: 25-27
src/frontend/components/ProductViewToggle.tsx (2)
18-26
: Improve toggle logic to be more explicit about view switching.Both buttons use the same
onToggle
handler, which means the parent component needs to implement the actual toggle logic. Consider making the interface more explicit about which view to switch to.Consider passing the specific view type to make the intent clearer:
interface ProductViewToggleProps { productView: ProductViewType; - onToggle: () => void; + onViewChange: (view: ProductViewType) => void; } // In the buttons: -> + => onViewChange('grid')} -> + => onViewChange('list')}This makes the component's behavior more predictable and reduces coupling with parent component logic.
Also applies to: 27-35
19-19
: Consider extracting long className strings for better maintainability.The className strings are quite long and contain duplicated styles. Consider extracting them into constants or using a utility like
clsx
.+const baseButtonClass = "cursor-pointer border border-gray-300 bg-white px-3 py-2 text-gray-500 transition-all duration-200 select-none hover:bg-gray-50"; +const activeButtonClass = "text-wepos-primary bg-wepos-primary/5 border-wepos-primary"; +const firstButtonClass = "first:rounded-l-lg"; +const lastButtonClass = "last:rounded-r-lg"; // Then use: -className={`cursor-pointer border border-gray-300 bg-white px-3 py-2 text-gray-500 transition-all duration-200 select-none first:rounded-l-lg hover:bg-gray-50 ${productView === 'grid' ? 'text-wepos-primary bg-wepos-primary/5 border-wepos-primary' : ''}`} +className={`${baseButtonClass} ${firstButtonClass} ${productView === 'grid' ? activeButtonClass : ''}`}Also applies to: 28-28
.gitignore (1)
49-49
: Remove duplicate pattern*~
The pattern
*~
is defined twice - once in the Linux section (line 49) and again in the Vim section (line 118). Consider removing one occurrence to avoid duplication.Also applies to: 118-118
src/frontend/components/CategoryFilter.tsx (1)
17-17
: Remove redundant width classThe className contains
w-64 w-full md:w-64
where the initialw-64
is immediately overridden byw-full
.- <div className="w-64 w-full md:w-64"> + <div className="w-full md:w-64">src/frontend/hooks/usePOSData.ts (1)
83-93
: Consider adding error state managementThe current error handling only logs to console without providing user feedback or allowing the UI to react to errors. Consider adding error states to track and display failures.
Consider implementing error states in the store or local state:
const [errors, setErrors] = useState<{ products?: Error; gateways?: Error; settings?: Error; categories?: Error; }>({}); // In catch blocks: catch (error) { console.error('Error fetching products:', error); setErrors(prev => ({ ...prev, products: error as Error })); setProductsLoading(false); }This would allow components to display appropriate error messages or retry options.
Also applies to: 95-104, 106-115, 117-126
src/frontend/components/HelpModal.tsx (2)
1-60
: Well-implemented help modal with room for configuration improvements.The component follows React best practices with proper conditional rendering, i18n usage, and WordPress comp 8000 onent integration.
Consider making the keyboard shortcuts configurable rather than hardcoded:
- <ul className="space-y-3"> - <li className="text-gray-700 flex items-center gap-4 py-2"> - <span className="code">F3</span> - <span>{__('Toggle between grid and list view', 'wepos')}</span> - </li> + <ul className="space-y-3"> + {shortcuts.map(({ key, description }) => ( + <li key={key} className="text-gray-700 flex items-center gap-4 py-2"> + <span className="code">{key}</span> + <span>{__(description, 'wepos')}</span> + </li> + ))}
17-17
: Consider consistent styling approach.The component mixes Tailwind CSS classes with custom CSS classes like
wepos-help-modal
. For better maintainability, consider using either pure Tailwind or a consistent custom class naming convention.src/frontend/components/ProductVariationSelector.tsx (5)
70-90
: Consider extracting cart item construction logic.The cart item construction logic is complex and could benefit from being extracted to a separate utility function for better testability and reusability.
Extract to a utility function:
const createCartItemFromVariation = ( product: POSProduct, variation: ProductVariation, selectedAttributes: SelectedAttributes ): CartItem => { // Move the current cart item construction logic here };
71-71
: Consider using a more robust ID generation method.Using
Date.now()
for generating cart item IDs could potentially create collisions if items are added rapidly. Consider using a more robust method like UUID or a proper ID generation utility.- id: Date.now(), // Generate a temporary ID + id: `temp_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`, // More robust temporary ID
10-10
: Remove unused prop from interface.The
anchor
prop is defined in the interface but never used in the component implementation.- anchor?: HTMLElement | null;
64-64
: Clarify variation attribute ID handling.The comment suggests the ID will be set by the system, but it's hardcoded to 0. This might cause issues if the system expects unique IDs.
Consider either removing the hardcoded value or updating the comment to reflect the actual behavior.
119-119
: Optimize callback to prevent unnecessary re-renders.The inline function creates a new reference on every render, which could impact performance.
- => setIsVisible(false)} + >This reuses the existing
handleClose
callback which is already memoized.src/frontend/components/CustomerModal.tsx (3)
126-129
: Consider more robust form validation.The current validation only checks for non-empty trimmed values. Consider adding email format validation and other field-specific validation rules.
Add email validation:
const isFormValid = customerForm.first_name.trim() !== '' && customerForm.last_name.trim() !== '' && - customerForm.email.trim() !== ''; + customerForm.email.trim() !== '' && + /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(customerForm.email);
126-129
: Enhance form validation.The current validation only checks for non-empty required fields. Consider adding email format validation and other business rules.
// Check if form is valid + const isValidEmail = (email: string) => { + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + return emailRegex.test(email); + }; + const isFormValid = customerForm.first_name.trim() !== '' && customerForm.last_name.trim() !== '' && - customerForm.email.trim() !== ''; + customerForm.email.trim() !== '' && + isValidEmail(customerForm.email);
189-189
: Address the TODO comment for error notification.Error handling is incomplete and users won't see feedback when operations fail.
The TODO comment indicates missing error notification functionality. This should be implemented to provide proper user feedback.
Would you like me to help generate a proper error notification implementation using WordPress notices or a toast notification system?
src/frontend/store/products/selectors.ts (2)
26-34
: Improve stock filtering logic for edge cases.The stock filtering logic could be improved to handle edge cases more robustly, particularly for products with undefined stock quantities.
Consider enhancing the logic for better clarity and safety:
getInStockProducts: (state: ProductsState) => { return state.products.filter( (product) => product.stock_status === 'instock' || (product.manage_stock && - product.stock_quantity && - product.stock_quantity > 0), + typeof product.stock_quantity === 'number' && + product.stock_quantity > 0), ); },
3-35
: Consider memoization for performance optimization.The selectors perform filtering operations that could benefit from memoization, especially for large product catalogs. Consider using a memoization library like
reselect
.+ import { createSelector } from '@reduxjs/toolkit'; + export const selectors = { getProducts: (state: ProductsState) => state.products, // ... other basic selectors - getProductsByCategory: (state: ProductsState, categoryId?: number) => { - if (!categoryId || categoryId === 0) { - return state.products; - } - return state.products.filter((product) => - product.categories?.some((cat) => cat?.id === categoryId) || false, - ); - }, + getProductsByCategory: createSelector( + [(state: ProductsState) => state.products, (state: ProductsState, categoryId?: number) => categoryId], + (products, categoryId) => { + if (!categoryId || categoryId === 0) { + return products; + } + return products.filter((product) => + product.categories?.some((cat) => cat?.id === categoryId) || false, + ); + } + ), - getInStockProducts: (state: ProductsState) => { - return state.products.filter( - (product) => - product.stock_status === 'instock' || - (product.manage_stock && - typeof product.stock_quantity === 'number' && - product.stock_quantity > 0), - ); - }, + getInStockProducts: createSelector( + [(state: ProductsState) => state.products], + (products) => products.filter( + (product) => + product.stock_status === 'instock' || + (product.manage_stock && + typeof product.stock_quantity === 'number' && + product.stock_quantity > 0), + ) + ), };src/frontend/components/CustomerNote.tsx (3)
20-26
: Improve accessibility and timing for focus management.The focus management implementation has a hardcoded timeout which may not be reliable across different devices and browsers. Consider using a more robust approach.
// Focus textarea when popover opens useEffect(() => { if (isVisible && textareaRef.current) { - setTimeout(() => { - textareaRef.current?.focus(); - }, 100); + // Use requestAnimationFrame for better timing + const focusElement = () => { + if (textareaRef.current) { + textareaRef.current.focus(); + } else { + requestAnimationFrame(focusElement); + } + }; + requestAnimationFrame(focusElement); } }, [isVisible]);
49-58
: Enhance button accessibility.The button implementation could benefit from better accessibility attributes and ARIA labels.
<button ref={buttonRef} type="button" + aria-expanded={isVisible} + aria-haspopup="dialog" + aria-describedby="customer-note-description" className="focus:ring-opacity-20 rounded-lg border border-gray-200 bg-gray-50 px-3 py-2 text-sm font-medium text-gray-600 transition-colors hover:bg-gray-100 focus:ring-2 focus:ring-gray-500" => setIsVisible(!isVisible)} > {__('Add Note', 'wepos')} </button> + + <span id="customer-note-description" className="sr-only"> + {__('Opens a dialog to add a customer note', 'wepos')} + </span>
76-85
: Add proper validation feedback.The textarea could benefit from validation feedback and better error handling for edge cases.
Consider adding validation state and feedback:
+ const maxLength = 500; // Define maximum note length + const isValid = noteText.trim().length > 0 && noteText.length <= maxLength; + <textarea ref={textareaRef} id="customer-note" value={noteText} => setNoteText(e.target.value)} + maxLength={maxLength} + aria-invalid={!isValid && noteText.length > 0} + aria-describedby="note-help note-error" className="w-full resize-none rounded-lg border border-gray-300 px-3 py-2 focus:border-blue-500 focus:ring-2 focus:ring-blue-500" rows={4} placeholder={__('Enter note for this order...', 'wepos')} /> + + <div id="note-help" className="mt-1 text-xs text-gray-500"> + {noteText.length}/{maxLength} {__('characters', 'wepos')} + </div> + + {noteText.length > maxLength && ( + <div id="note-error" className="mt-1 text-xs text-red-600"> + {__('Note is too long. Please keep it under 500 characters.', 'wepos')} + </div> + )}src/frontend/store/cart/reducer.ts (1)
62-62
: Consider using a more robust ID generation methodUsing
Date.now()
for generating unique IDs could potentially produce duplicates if multiple discounts or fees are added in rapid succession (within the same millisecond). Consider using a UUID library or implementing a counter-based approach.+import { v4 as uuidv4 } from 'uuid'; // In ADD_DISCOUNT case: - const discountId = Date.now(); + const discountId = uuidv4(); // In ADD_FEE case: - const feeId = Date.now(); + const feeId = uuidv4();Alternatively, if you want to avoid external dependencies:
+let idCounter = 0; +const generateId = () => `${Date.now()}_${++idCounter}`; // In ADD_DISCOUNT case: - const discountId = Date.now(); + const discountId = generateId(); // In ADD_FEE case: - const feeId = Date.now(); + const feeId = generateId();Also applies to: 81-81
src/frontend/components/Cart.tsx (1)
29-561
: Consider splitting this large component for better maintainabilityAt 500+ lines, this component handles many responsibilities. Consider extracting sub-components for better maintainability and testability.
Suggested component breakdown:
CartHeader
- Customer search and quick menu (lines 140-198)CartItemList
- Product table and item management (lines 203-393)CartTotals
- Subtotal, discounts, fees section (lines 399-546)CartActions
- Discount/Fee/Note buttons (lines 487-507)This would improve:
- Code reusability
- Testing isolation
- Performance (with React.memo on sub-components)
- Team collaboration
src/frontend/pages/Home.tsx (2)
101-101
: Remove unuseditemsWrapperRef
.The ref is declared but never used in the component.
- const itemsWrapperRef = useRef<HTMLDivElement>(null);
Also remove it from the ProductGrid props on line 432:
- itemsWrapperRef={itemsWrapperRef}
390-402
: Replacewindow.confirm
with a proper modal component.Using browser's native confirm dialog provides poor UX. Consider using a React modal component for better user experience.
- if ( - window.confirm( - 'Order successful! Would you like to print the receipt?', - ) - ) { - window.print(); - } + // Consider using a modal component like: + // setShowPrintConfirmModal(true); + // And handle printing in the modal's confirm actionWould you like me to help create a PrintConfirmModal component that provides a better user experience?
src/frontend/components/FeeKeypad.tsx (1)
136-142
: Consider using an icon component for the backspace button.The Unicode character ⌫ might not render consistently across different systems. Consider using an SVG icon or icon component.
<button type="button" className="h-12 rounded-lg bg-red-100 text-sm font-medium text-red-700 transition-colors hover:bg-red-200" + aria-label="Clear input" > - ⌫ + <svg className="w-5 h-5 mx-auto" fill="none" stroke="currentColor" viewBox="0 0 24 24"> + <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M12 14l2-2m0 0l2-2m-2 2l-2-2m2 2l2 2M3 12l6.414 6.414a2 2 0 001.414.586H19a2 2 0 002-2V7a2 2 0 00-2-2h-8.172a2 2 0 00-1.414.586L3 12z" /> + </svg> </button>src/frontend/components/CustomerSearch.tsx (1)
270-274
: Add error handling for customer avatarsConsider adding error handling for broken avatar images to improve UX.
<img src={customer.avatar_url} alt={`${customer.first_name} ${customer.last_name}`} className="h-8 w-8 flex-shrink-0 rounded-full object-cover" + => { + e.currentTarget.src = '/path/to/default-avatar.png'; + }} />src/frontend/store/cart/selectors.ts (1)
42-46
: Missing tax calculation implementationThe tax calculation is currently a placeholder returning 0.
Would you like me to help implement the tax calculation logic or create an issue to track this TODO?
src/frontend/README.md (1)
1-622
: Excellent comprehensive documentation for the React migration!This README provides outstanding coverage of the Vue.js to React migration, including architecture decisions, API integration patterns, and detailed troubleshooting guidance. The migration notes section (lines 178-196) is particularly valuable for developers understanding the transition patterns.
Minor formatting improvements needed:
-``` +```bash src/frontend/ ├── components/ │ └── Home.tsx # Main POS interface (single component approach)Also consider adding language specification to the directory structure block at line 57.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~169-~169: You might be missing the article “the” here.
Context: ...sessions - Order data maintained during checkout process - Data restored on page reload ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~229-~229: You might be missing the article “the” here.
Context: ...erformance - Bundle size optimized for WordPress environment - Lazy loading ready for fu...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~307-~307: You might be missing the article “the” here.
Context: ...ocessing results and print data - Check browser network tab for detailed API error resp...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~314-~314: Possible missing comma found.
Context: ...e payment processing follows a two-step approach matching the original Vue.js implementa...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~485-~485: Loose punctuation mark.
Context: ...r the layout system: -.wepos-sidebar
: Main sidebar container - `.wepos-sideba...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~496-~496: You might be missing the article “a” here.
Context: ...d layouts - Mobile: Sidebar becomes horizontal navigation bar ## Modal System ### Wo...(AI_EN_LECTOR_MISSING_DETERMINER_A)
🪛 markdownlint-cli2 (0.17.2)
57-57: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
includes/ReactAssets.php (1)
56-56
: Consider making development server URL configurable.The hardcoded
http://localhost:8887
URLs work for standard setups but could be problematic in containerized or custom development environments.Consider adding a filter or constant for the development server URL:
-$script_url = $is_dev ? 'http://localhost:8887/wepos-react.js' : WEPOS_URL . '/build/wepos-react.js'; +$dev_server_url = apply_filters('wepos_dev_server_url', 'http://localhost:8887'); +$script_url = $is_dev ? $dev_server_url . '/wepos-react.js' : WEPOS_URL . '/build/wepos-react.js';Also applies to: 67-67, 88-88
src/frontend/components/README.md (1)
1-227
: Excellent component documentation with clear architecture explanation.This README effectively documents the CustomerSearch and CustomerModal components with clear interfaces, usage examples, and architectural benefits. The separation of concerns between search/selection and data entry is well explained.
Minor formatting improvements for consistency:
-### In CustomerSearch Component: +### In CustomerSearch Component -### In CustomerModal Component: +### In CustomerModal ComponentAlso consider adding language specification to the component relationship diagram at line 139.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~174-~174: Loose punctuation mark.
Context: ...sibility Principle** -CustomerSearch
: Handles search, selection, and customer...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~207-~207: Loose punctuation mark.
Context: ...ustomerSearch -@wordpress/components
: Spinner -lucide-react
: Users, Plus, ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~213-~213: Loose punctuation mark.
Context: ...CustomerModal -@wordpress/components
: Modal, TextControl, SelectControl, Butt...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~220-~220: Loose punctuation mark.
Context: ...ints: -getCustomers(search?: string)
: Search for customers - `createCustomer(...(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
139-139: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
157-157: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
163-163: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
src/frontend/components/ProductGrid.tsx (1)
135-172
: Consider extracting the card rendering logic to reduce duplication.The grid view has significant code duplication between simple and variable product cards. Consider extracting the card layout to a separate component or shared render function.
Consider creating a
ProductCard
component to reduce duplication:interface ProductCardProps { product: POSProduct; hasStock: boolean; getProductImage: (product: POSProduct) => string; truncateTitle: (text: string, length: number) => string; children: React.ReactNode; // For the action button/selector } const ProductCard: React.FC<ProductCardProps> = ({ product, hasStock, getProductImage, truncateTitle, children }) => ( <div className="hover:border-wepos-primary/30 group min-w-[160px] cursor-pointer overflow-hidden rounded-xl border border-gray-200 bg-white transition-all duration-200 hover:-translate-y-1 hover:shadow-lg..."> {/* Card content */} {children} </div> );Also applies to: 174-216
🧰 Tools
🪛 Biome (1.9.4)
[error] 166-166: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
plan.md (3)
82-82
: Minor: Consider hyphenating the section heading.For consistency with technical writing standards.
-### 3.1 Page 1: Product Selection/Catalog +### 3.1 Page-1: Product Selection/Catalog🧰 Tools
🪛 LanguageTool
[grammar] ~82-~82: ‘1 Page’ is missing a hyphen.
Context: ...se 3: Incremental Page Migration ### 3.1 Page 1: Product Selection/Catalog - [ ] Conv...(STARS_AND_STEPS)
169-169
: Use an en dash for the range.-**Total Estimated Timeline**: 9-12 weeks +**Total Estimated Timeline**: 9–12 weeks🧰 Tools
🪛 LanguageTool
[typographical] ~169-~169: If specifying a range, consider using an en dash instead of a hyphen.
Context: ... Polish) Total Estimated Timeline: 9-12 weeks ## Success Criteria for Each Pag...(HYPHEN_TO_EN)
196-196
: Consider adding "the" for better readability.-- [ ] Successful integration with WordPress ecosystem +- [ ] Successful integration with the WordPress ecosystem🧰 Tools
🪛 LanguageTool
[uncategorized] ~196-~196: You might be missing the article “the” here.
Context: ...iance - [ ] Successful integration with WordPress ecosystem - [ ] Positive user feedback ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
src/frontend/types/index.ts (1)
586-586
: Consider using the existingProductVariation
type instead ofany[]
.- variations?: any[]; + variations?: ProductVariation[];src/frontend/api/index.ts (3)
56-56
: Extract magic numbers to constants.Hard-coded values for pagination should be extracted to named constants for better maintainability.
Add constants at the top of the file:
// API Constants const API_CONFIG = { PRODUCTS_PER_PAGE: 30, CATEGORIES_PER_PAGE: 100, } as const;Then update the usage:
- path: `${API_BASE.WEPOS}/products?status=publish&per_page=30&page=1`, + path: `${API_BASE.WEPOS}/products?status=publish&per_page=${API_CONFIG.PRODUCTS_PER_PAGE}&page=1`,Also applies to: 82-82, 168-168
459-475
: Define proper types for report data.Both report methods return
any
type, which reduces type safety. Consider defining interfaces for the report data structures.// Add these interfaces to your types file interface SalesReport { total: number; orders: number; period: string; // ... other fields } interface ProductReport { product_id: number; name: string; quantity: number; total: number; // ... other fields } // Then update the methods getSalesReport: async (period: string = 'today'): Promise<SalesReport> => { // ... }, getTopProducts: async (period: string = 'today'): Promise<ProductReport[]> => { // ... }
1-487
: Consider adding error handling and request management utilities.The API client could benefit from additional utilities for improved reliability and user experience:
- Error handling wrapper: Standardize error handling across all API calls
- Request cancellation: Support for AbortController to cancel in-flight requests
- Retry logic: Automatic retry for failed requests with exponential backoff
- Response caching: Cache frequently accessed data like products and categories
Example error handling wrapper:
const apiWrapper = async <T>( apiCall: () => Promise<T>, options?: { retries?: number; retryDelay?: number } ): Promise<T> => { try { return await apiCall(); } catch (error) { // Log error, show user notification, retry if needed console.error('API Error:', error); throw error; } };Would you like me to create an issue to track these architectural improvements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
assets/css/admin.css.map
is excluded by!**/*.map
assets/css/bootstrap.css.map
is excluded by!**/*.map
assets/css/frontend.css.map
is excluded by!**/*.map
assets/css/style.css.map
is excluded by!**/*.map
assets/css/vendor.css.map
is excluded by!**/*.map
assets/js/admin.min.js
is excluded by!**/*.min.js
assets/js/bootstrap.min.js
is excluded by!**/*.min.js
assets/js/frontend.min.js
is excluded by!**/*.min.js
assets/js/jquery.min.js
is excluded by!**/*.min.js
assets/js/select2.min.js
is excluded by!**/*.min.js
assets/js/style.min.js
is excluded by!**/*.min.js
assets/js/vendor.min.js
is excluded by!**/*.min.js
assets/js/wphook.min.js
is excluded by!**/*.min.js
package-lock.json
is excluded by!**/package-lock.json
tests/pw/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (66)
.cursor/rules/overview.mdc
(1 hunks).editorconfig
(1 hunks).github/workflows/e2e_api_tests.yml
(7 hunks).gitignore
(1 hunks).pnpmrc
(1 hunks).prettierrc
(1 hunks)assets/css/admin.css
(0 hunks)assets/css/admin.min.css
(0 hunks)assets/css/bootstrap.css
(0 hunks)assets/css/bootstrap.min.css
(0 hunks)assets/css/flaticon.css
(0 hunks)assets/css/fonts.css
(0 hunks)assets/css/frontend.css
(0 hunks)assets/css/frontend.min.css
(0 hunks)assets/css/select2.min.css
(0 hunks)assets/css/styl 8000 e.css
(0 hunks)assets/css/style.min.css
(0 hunks)assets/css/vendor.css
(0 hunks)assets/js/jed.js
(0 hunks)assets/js/style.js
(0 hunks)assets/js/wphook.js
(0 hunks)bin/zip.sh
(1 hunks)includes/ReactAssets.php
(1 hunks)package.json
(1 hunks)plan.md
(1 hunks)postcss.config.js
(1 hunks)src/frontend/App.tsx
(1 hunks)src/frontend/README.md
(1 hunks)src/frontend/api/index.ts
(1 hunks)src/frontend/components/Cart.tsx
(1 hunks)src/frontend/components/CategoryFilter.tsx
(1 hunks)src/frontend/components/CustomerModal.tsx
(1 hunks)src/frontend/components/CustomerNote.tsx
(1 hunks)src/frontend/components/CustomerSearch.tsx
(1 hunks)src/frontend/components/FeeKeypad.tsx
(1 hunks)src/frontend/components/HelpModal.tsx
(1 hunks)src/frontend/components/Layout.tsx
(1 hunks)src/frontend/components/PaymentModal.tsx
(1 hunks)src/frontend/components/ProductGrid.tsx
(1 hunks)src/frontend/components/ProductVariationSelector.tsx
(1 hunks)src/frontend/components/ProductViewToggle.tsx
(1 hunks)src/frontend/components/README.md
(1 hunks)src/frontend/components/ReceiptModal.tsx
(1 hunks)src/frontend/components/SearchBar.tsx
(1 hunks)src/frontend/components/Sidebar.tsx
(1 hunks)src/frontend/hooks/useCart.ts
(1 hunks)src/frontend/hooks/usePOSData.ts
(1 hunks)src/frontend/index.tsx
(1 hunks)src/frontend/pages/Customers.tsx
(1 hunks)src/frontend/pages/Home.tsx
(1 hunks)src/frontend/pages/Orders.tsx
(1 hunks)src/frontend/store/cart/actions.ts
(1 hunks)src/frontend/store/cart/index.ts
(1 hunks)src/frontend/store/cart/reducer.ts
(1 hunks)src/frontend/store/cart/selectors.ts
(1 hunks)src/frontend/store/cart/store.ts
(1 hunks)src/frontend/store/cart/types.ts
(1 hunks)src/frontend/store/index.ts
(1 hunks)src/frontend/store/products/actions.ts
(1 hunks)src/frontend/store/products/index.ts
(1 hunks)src/frontend/store/products/reducer.ts
(1 hunks)src/frontend/store/products/selectors.ts
(1 hunks)src/frontend/store/products/store.ts
(1 hunks)src/frontend/store/products/types.ts
(1 hunks)src/frontend/styles/main.css
(1 hunks)src/frontend/types/index.ts
(1 hunks)
💤 Files with no reviewable changes (15)
- assets/js/style.js
- assets/css/fonts.css
- assets/css/bootstrap.min.css
- assets/css/flaticon.css
- assets/css/select2.min.css
- assets/css/vendor.css
- assets/css/admin.min.css
- assets/css/frontend.css
- assets/css/admin.css
- assets/css/frontend.min.css
- assets/css/style.css
- assets/css/bootstrap.css
- assets/js/wphook.js
- assets/css/style.min.css
- assets/js/jed.js
👮 Files not reviewed due to content moderation or server errors (3)
- src/frontend/components/HelpModal.tsx
- src/frontend/components/ProductVariationSelector.tsx
- src/frontend/components/CustomerModal.tsx
🧰 Additional context used
🧬 Code Graph Analysis (10)
src/frontend/components/SearchBar.tsx (1)
assets/src/utils/i18n.js (1)
__
(42-44)
src/frontend/store/products/types.ts (1)
src/frontend/types/index.ts (5)
POSProduct
(565-587)POSCategory
(555-560)POSGateway
(526-529)POSSettings
(443-450)POSSettings
(531-536)
src/frontend/components/ProductVariationSelector.tsx (2)
src/frontend/types/index.ts (3)
POSProduct
(565-587)CartItem
(221-221)ProductVariation
(223-259)assets/src/utils/i18n.js (1)
__
(42-44)
src/frontend/hooks/usePOSData.ts (7)
src/frontend/types/index.ts (2)
POSCartData
(213-218)POSOrderData
(517-524)src/frontend/store/products/store.ts (2)
store
(18-18)PRODUCTS_STORE_NAME
(6-6)src/frontend/store/index.ts (1)
PRODUCTS_STORE_NAME
(7-7)src/frontend/store/products/index.ts (1)
PRODUCTS_STORE_NAME
(1-1)src/frontend/utils/helpers.ts (2)
getFromLocalStorage
(63-72)setToLocalStorage
(77-85)src/frontend/store/products/actions.ts (8)
setProductsLoading
(32-37)setProducts
(4-9)setGatewaysLoading
(46-51)setGateways
(18-23)setSettingsLoading
(53-58)setSettings
(25-30)setCategoriesLoading
(39-44)setCategories
(11-16)src/frontend/api/index.ts (1)
posAPI
(479-487)
src/frontend/components/HelpModal.tsx (1)
assets/src/utils/i18n.js (1)
__
(42-44)
src/frontend/components/CategoryFilter.tsx (3)
src/frontend/types/index.ts (1)
POSCategory
(555-560)tests/pw/pages/basePage.ts (1)
select
(717-728)assets/src/utils/i18n.js (1)
__
(42-44)
src/frontend/components/ProductViewToggle.tsx (2)
src/frontend/types/index.ts (1)
ProductViewType
(563-563)assets/src/utils/i18n.js (1)
__
(42-44)
src/frontend/components/CustomerNote.tsx (1)
assets/src/utils/i18n.js (1)
__
(42-44)
src/frontend/store/cart/types.ts (1)
src/frontend/types/index.ts (3)
POSCartItem
(162-184)POSDiscountLine
(187-198)POSFeeLine
(200-210)
src/frontend/components/ProductGrid.tsx (4)
src/frontend/types/index.ts (3)
POSProduct
(565-587)ProductViewType
(563-563)CartItem
(221-221)assets/src/utils/i18n.js (1)
__
(42-44)src/frontend/utils/helpers.ts (3)
hasStock
(18-28)getProductImage
(33-37)truncateTitle
(42-44)src/frontend/components/ProductVariationSelector.tsx (1)
ProductVariationSelector
(17-208)
🪛 actionlint (1.7.7)
.github/workflows/e2e_api_tests.yml
152-152: shellcheck reported issue in this script: SC2086:info:1:127: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 LanguageTool
src/frontend/README.md
[uncategorized] ~169-~169: You might be missing the article “the” here.
Context: ...sessions - Order data maintained during checkout process - Data restored on page reload ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~229-~229: You might be missing the article “the” here.
Context: ...erformance - Bundle size optimized for WordPress environment - Lazy loading ready for fu...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~307-~307: You might be missing the article “the” here.
Context: ...ocessing results and print data - Check browser network tab for detailed API error resp...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~314-~314: Possible missing comma found.
Context: ...e payment processing follows a two-step approach matching the original Vue.js implementa...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~485-~485: Loose punctuation mark.
Context: ...r the layout system: - .wepos-sidebar
: Main sidebar container - `.wepos-sideba...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~496-~496: You might be missing the article “a” here.
Context: ...d layouts - Mobile: Sidebar becomes horizontal navigation bar ## Modal System ### Wo...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
plan.md
[grammar] ~82-~82: ‘1 Page’ is missing a hyphen.
Context: ...se 3: Incremental Page Migration ### 3.1 Page 1: Product Selection/Catalog - [ ] Conv...
(STARS_AND_STEPS)
[grammar] ~89-~89: ‘2 Page’ is missing a hyphen.
Context: ...eate UI design with Tailwind CSS ### 3.2 Page 2: Shopping Cart - [ ] Migrate shopping...
(STARS_AND_STEPS)
[grammar] ~92-~92: Using ‘plenty’ without ‘of’ is considered to be informal.
Context: ...e product functionality - [ ] Implement quantity adjustments - [ ] Convert discount and tax calculat...
(PLENTY_OF_NOUNS)
[grammar] ~95-~95: ‘3 Page’ is missing a hyphen.
Context: ...rt discount and tax calculations ### 3.3 Page 3: Checkout Flow - [ ] Convert customer...
(STARS_AND_STEPS)
[grammar] ~101-~101: ‘4 Page’ is missing a hyphen.
Context: ... receipt generation and printing ### 3.4 Page 4: Order Management - [ ] Convert order...
(STARS_AND_STEPS)
[typographical] ~169-~169: If specifying a range, consider using an en dash instead of a hyphen.
Context: ... Polish) Total Estimated Timeline: 9-12 weeks ## Success Criteria for Each Pag...
(HYPHEN_TO_EN)
[uncategorized] ~196-~196: You might be missing the article “the” here.
Context: ...iance - [ ] Successful integration with WordPress ecosystem - [ ] Positive user feedback ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
src/frontend/components/README.md
[uncategorized] ~174-~174: Loose punctuation mark.
Context: ...sibility Principle** - CustomerSearch
: Handles search, selection, and customer...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~207-~207: Loose punctuation mark.
Context: ...ustomerSearch - @wordpress/components
: Spinner - lucide-react
: Users, Plus, ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~213-~213: Loose punctuation mark.
Context: ...CustomerModal - @wordpress/components
: Modal, TextControl, SelectControl, Butt...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~220-~220: Loose punctuation mark.
Context: ...ints: - getCustomers(search?: string)
: Search for customers - `createCustomer(...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
src/frontend/README.md
57-57: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
plan.md
9-9: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
src/frontend/components/README.md
139-139: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
157-157: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
163-163: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 Biome (1.9.4)
src/frontend/components/ProductGrid.tsx
[error] 90-90: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 166-166: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 208-208: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (54)
.pnpmrc (1)
1-7
: Configuration looks solid
The new.pnpmrc
correctly configures peer-dependency handling and hoisting patterns to align with your pnpm migration..prettierrc (1)
1-9
: Prettier configuration approved
The settings (2-space tabs, trailing commas, single quotes) and inclusion ofprettier-plugin-tailwindcss
align with the React + Tailwind migration.src/frontend/store/products/store.ts (1)
2-4
:✅ Verification successful
Verify that all imported modules exist.
The store setup follows WordPress data store patterns correctly. Please ensure that the imported modules (
reducer
,actions
,selectors
) are properly implemented.
🏁 Script executed:
#!/bin/bash # Description: Verify that all imported store modules exist # Expected: All modules should exist and export the expected items echo "Checking for products store modules..." fd -t f "reducer\.(ts|js)" src/frontend/store/products/ fd -t f "actions\.(ts|js)" src/frontend/store/products/ fd -t f "selectors\.(ts|js)" src/frontend/store/products/Length of output: 381
Store modules verified and setup approved
All imported modules in
src/frontend/store/products/
(reducer.ts
,actions.ts
,selectors.ts
) exist as expected. No further action required—the WordPress data store setup is correctly implemented.src/frontend/pages/Orders.tsx (1)
3-3
:✅ Verification successful
Verify Layout component exists.
Ensure the Layout component is properly implemented and exported.
🏁 Script executed:
#!/bin/bash # Description: Verify Layout component exists and is properly exported # Expected: Layout component should exist with proper exports echo "Checking for Layout component..." fd -t f "Layout\.(tsx|ts)" src/frontend/components/Length of output: 163
🏁 Script executed:
#!/bin/bash # Verifying that Layout.tsx has a default export for the Layout component echo "Checking for default export in Layout.tsx..." rg -n "^export\s+default\s+Layout" src/frontend/components/Layout.tsx || { echo "No default export found; listing all exports in Layout.tsx:" rg -n "export" src/frontend/components/Layout.tsx }Length of output: 196
Layout component verification successful
Verified
src/frontend/components/Layout.tsx
exists and contains a default export forLayout
. Placeholder structure approved.src/frontend/store/cart/store.ts (1)
1-19
: LGTM! Clean WordPress data store implementation.The cart store follows WordPress data store conventions correctly with proper imports, store creation, registration, and exports. The namespace
'wepos/cart'
is appropriate and the structure is clean.src/frontend/index.tsx (2)
18-18
: Consider if HashRouter is the appropriate choice.Using
HashRouter
instead ofBrowserRouter
means URLs will have hash fragments (e.g.,/#/customers
). This is typically used when you can't configure the server to handle client-side routing. For a WordPress plugin context, this might be appropriate, but please verify this aligns with your routing requirements.
11-23
: Excellent error handling and React setup.The conditional rendering with error logging for missing container and the use of
React.StrictMode
are both excellent practices for a production React application.src/frontend/pages/Customers.tsx (1)
5-34
: Well-structured placeholder component.The
CustomersPage
component follows React and TypeScript best practices with:
- Clean functional component structure
- Appropriate Tailwind CSS styling
- Good use of Lucide React icons
- Clear placeholder content indicating future functionality
- Proper component export
The layout and styling provide a consistent foundation for future customer management features.
src/frontend/store/index.ts (1)
1-9
: Excellent store aggregation pattern.This central index file provides a clean way to:
- Automatically register all stores through imports
- Centralize store name constants and types for easy consumption
- Maintain a single import location for store-related exports
The pattern supports type safety and makes the store system easy to use across components.
.github/workflows/e2e_api_tests.yml (2)
76-76
: LGTM! Proper pnpm setup and caching configuration.The migration to pnpm is well-executed with appropriate cache configuration and version specification.
Also applies to: 84-87
89-96
: LGTM! Consistent pnpm command usage.The installation and build commands have been properly updated to use pnpm equivalents.
src/frontend/store/products/index.ts (1)
1-5
: LGTM! Clean barrel export pattern.The module exports follow a clean, standard pattern for organizing store-related exports. This provides a single entry point for the products store functionality.
src/frontend/store/products/types.ts (2)
4-15
: LGTM! Well-structured state interface.The
ProductsState
interface is comprehensive and follows good patterns for state management with separate loading flags for each data type.
18-26
: LGTM! Proper discriminated union for actions.The
ProductsAction
type uses a discriminated union pattern which provides excellent type safety and IntelliSense support for action handling.src/frontend/store/products/reducer.ts (1)
18-78
: LGTM! Well-structured reducer implementation.The reducer follows Redux patterns correctly with immutable updates and proper action handling. The automatic loading state management when setting data is a good UX pattern.
src/frontend/store/cart/index.ts (1)
1-8
: Clean barrel export structure.The export structure follows good practices for creating a clean API surface for the cart store module.
src/frontend/components/Layout.tsx (2)
12-20
: Excellent localStorage error handling and lazy initialization.The implementation properly handles localStorage failures and uses lazy state initialization, which is a React best practice.
23-32
: Good persistence pattern with error handling.The useEffect properly persists state changes while gracefully handling localStorage errors.
src/frontend/components/ProductViewToggle.tsx (1)
19-19
: Excellent accessibility implementation.The component properly implements accessibility with descriptive titles, aria-labels, and semantic button elements. The internationalization is also correctly implemented.
Also applies to: 28-28
package.json (1)
16-16
: 🛠️ Refactor suggestionAdd cross-env for cross-platform compatibility
The
NODE_ENV=development
syntax doesn't work on Windows. Consider usingcross-env
for cross-platform compatibility.First, add cross-env to devDependencies:
"devDependencies": { + "cross-env": "^7.0.3", "@tailwindcss/postcss": "^4.1.0",
Then update the script:
- "start": "NODE_ENV=development wp-scripts start --hot", + "start": "cross-env NODE_ENV=development wp-scripts start --hot",Likely an incorrect or invalid review comment.
src/frontend/store/cart/types.ts (1)
1-26
: Excellent TypeScript type definitions.The type definitions are well-structured and comprehensive, covering all necessary cart operations. The use of union types for actions follows Redux best practices and ensures type safety throughout the store implementation.
src/frontend/components/Rec 6D40 eiptModal.tsx (1)
167-178
: LGTM! Good conditional rendering for cash payment details.The conditional rendering for cash payment information (cash received and change) is well-implemented and provides relevant information only when using cash payment gateway.
.cursor/rules/overview.mdc (4)
11-14
: Update documentation to reflect React migration.The architecture overview still references Vue.js components, but this PR is migrating from Vue.js to React with TypeScript. The documentation should be updated to reflect the new React-based architecture.
WePos is a **WordPress plugin** that extends WooCommerce with Point of Sale (POS) functionality. It combines: - **Backend PHP**: WordPress/WooCommerce integration using PSR-4 autoloading - - **Frontend Vue.js SPA**: Modern JavaScript interface for POS operations + - **Frontend React SPA**: Modern TypeScript/React interface for POS operations - **REST API**: Custom endpoints extending WooCommerce REST API - - **Build System**: Webpack-based compilation with Babel, LESS, and Vue support + - **Build System**: wp-scripts-based compilation with TypeScript, Tailwind CSS, and React supportLikely an incorrect or invalid review comment.
19-21
: Update entry points to reflect new React structure.The documented entry points still reference the old Vue.js structure, but the new React structure uses different paths.
Based on the migration, update the entry points:
### 🚀 Entry Points - **Main Plugin**: [wepos.php](mdc:wepos.php) - WordPress plugin entry point with core WePOS class - - **Frontend App**: [assets/src/frontend/main.js](mdc:assets/src/frontend/main.js) - Vue.js frontend application - - **Admin App**: [assets/src/admin/main.js](mdc:assets/src/admin/main.js) - Vue.js admin interface + - **Frontend App**: [src/frontend/index.tsx](mdc:src/frontend/index.tsx) - React frontend application + - **Admin Interface**: Integrated within WordPress admin using React componentsLikely an incorrect or invalid review comment.
34-38
: Update frontend section to reflect React migration.The frontend section references Vue.js directories that no longer exist in the new React structure.
- ### 🎨 Frontend (Vue.js) - - **Source Directory**: [assets/src/](mdc:assets/src) - All Vue.js source code - - **Frontend App**: [assets/src/frontend/](mdc:assets/src/frontend) - POS interface components - - **Admin App**: [assets/src/admin/](mdc:assets/src/admin) - Admin dashboard components - - **Utilities**: [assets/src/utils/](mdc:assets/src/utils) - Shared JavaScript utilities and Vuex store + ### 🎨 Frontend (React) + - **Source Directory**: [src/frontend/](mdc:src/frontend) - All React/TypeScript source code + - **Components**: [src/frontend/components/](mdc:src/frontend/components) - React components + - **Pages**: [src/frontend/pages/](mdc:src/frontend/pages) - Page-level components + - **Store**: [src/frontend/store/](mdc:src/frontend/store) - Redux state management + - **API**: [src/frontend/api/](mdc:src/frontend/api) - API client and utilitiesLikely an incorrect or invalid review comment.
72-76
: Update JavaScript development section for React.The JavaScript development section needs to reflect the new React-based stack with TypeScript and Tailwind CSS.
### JavaScript Development - - **Framework**: Vue.js 2.7 with Vue Router and Vuex - - **Build Process**: Webpack with Babel transpilation - - **Styling**: LESS preprocessor for CSS - - **Components**: Located in respective `components/` directories + - **Framework**: React 18 with TypeScript + - **Build Process**: wp-scripts with TypeScript compilation + - **Styling**: Tailwind CSS with PostCSS + - **State Management**: Redux with WordPress data stores + - **Components**: Located in `src/frontend/components/` directoryLikely an incorrect or invalid review comment.
src/frontend/components/CustomerNote.tsx (1)
40-47
: LGTM! Well-implemented keyboard shortcuts.The keyboard event handling is well-implemented with proper support for Ctrl+Enter to submit and Escape to cancel. This provides excellent user experience for power users.
src/frontend/store/cart/reducer.ts (1)
13-127
: Well-structured reducer with clean immutability patternsThe reducer implementation follows Redux best practices with proper state immutability, clear action types, and comprehensive cart management functionality.
src/frontend/styles/main.css (1)
1-350
: Excellent CSS architecture with comprehensive print supportThe stylesheet is well-organized with:
- Clean Tailwind v4 integration and custom theme configuration
- Proper component encapsulation using
@layer components
- Comprehensive print styles optimized for thermal receipt printers
- Responsive design considerations
The print media queries are particularly well-thought-out with proper handling of thermal printer constraints.
src/frontend/components/Sidebar.tsx (1)
18-121
: Clean component structure with proper TypeScript usageThe Sidebar component is well-structured with:
- Proper TypeScript interfaces
- Clean separation of concerns
- Good use of React Router hooks
- Accessible button implementations with proper
title
attributessrc/frontend/components/Cart.tsx (1)
84-133
: Well-implemented cart logic with proper state managementThe cart operations (quantity updates, item removal, discount/fee calculations) are implemented correctly with proper state updates through the WordPress data store.
src/frontend/store/products/actions.ts (1)
3-59
: Well-structured Redux actions with proper TypeScript typing!The action creators follow Redux best practices with consistent patterns and proper use of
as const
assertions for type safety.src/frontend/components/CustomerSearch.tsx (1)
255-322
: Well-implemented search results UIThe search results dropdown is well-designed with:
- Clear visual feedback for keyboard navigation
- Helpful keyboard shortcuts display
- Proper accessibility considerations
src/frontend/store/cart/actions.ts (1)
1-74
: Well-structured Redux actionsThe action creators are:
- Properly typed with TypeScript
- Following consistent naming conventions
- Using
as const
for type inference- Clean and maintainable
src/frontend/README.md (1)
253-296
: Excellent troubleshooting section with specific fixes documented.The detailed documentation of price calculation bugs and their fixes (lines 274-296) is extremely valuable. This shows careful attention to common issues like string-to-number conversion problems and localStorage data handling.
includes/ReactAssets.php (3)
11-18
: Good separation of admin and frontend asset loading.The constructor properly separates admin and frontend contexts using different hooks. This allows for future admin-specific React components while keeping frontend assets isolated.
27-31
: Smart development mode detection.Using the presence of
runtime.asset.php
to detect development mode is a clever approach that directly correlates with webpack dev server status.
98-134
: Comprehensive localization data provided to React frontend.The localized data object is thorough and provides all necessary context for the React frontend including REST endpoints, WooCommerce settings, and WordPress integration data.
src/frontend/components/README.md (1)
170-194
: Great explanation of architectural benefits.The benefits section clearly articulates why this component structure was chosen, focusing on single responsibility, user experience, maintainability, and code organization. This helps developers understand the design decisions.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~174-~174: Loose punctuation mark.
Context: ...sibility Principle** -CustomerSearch
: Handles search, selection, and customer...(UNLIKELY_OPENING_PUNCTUATION)
src/frontend/components/ProductGrid.tsx (4)
7-18
: Well-defined props interface with appropriate types.The interface clearly defines all required props with proper TypeScript types. The separation of
onAddToCart
for simple products andonAddToCartItem
for variable products is good design.
34-40
: Excellent responsive grid system with comprehensive breakpoints.The responsive grid implementation covers multiple device sizes and maintains proper aspect ratios. The use of CSS Grid with auto-fill and minmax provides flexible layout behavior.
96-123
: Good separation of simple and variable product handling.The conditional rendering properly handles different product types, with simple products getting direct add-to-cart functionality and variable products using the ProductVariationSelector component.
90-93
:❓ Verification inconclusive
Security concern: Multiple uses of dangerouslySetInnerHTML with external data.
Using
dangerouslySetInnerHTML
withproduct.price_html
poses a potential XSS risk, even if the data comes from WooCommerce. Consider safer alternatives for displaying formatted prices.Consider using a safe HTML sanitizer or extracting the price value directly:
-<span - className="text-sm font-medium sm:text-xs" - dangerouslySetInnerHTML={{ - __html: product.price_html, - }} -></span> +<span className="text-sm font-medium sm:text-xs"> + {formatPrice(product.sale_price || product.regular_price)} +</span>Let me verify if WooCommerce's
price_html
field is properly sanitized:Also applies to: 166-169, 208-211
🏁 Script executed:
#!/bin/bash # Search for how price_html is generated and if it's sanitized rg -A 5 -B 5 "price_html" --type phpLength of output: 36
Security: Verify sanitization of
price_html
before using dangerouslySetInnerHTMLI wasn’t able to find any generation or sanitization logic for
price_html
in the repo, so it’s unclear whether WooCommerce strips unsafe markup. Since you’re rendering this field withdangerouslySetInnerHTML
, please manually confirm that it can’t contain malicious content, or switch to a safer approach:• Locations to review:
– src/frontend/components/ProductGrid.tsx (lines 90–93, 166–169, 208–211)• Safe rendering options:
- Extract and format the raw price values instead of injecting HTML:
- <span - className="text-sm font-medium sm:text-xs" - dangerouslySetInnerHTML={{ - __html: product.price_html, - }} - ></span> + <span className="text-sm font-medium sm:text-xs"> + {formatPrice(product.sale_price || product.regular_price)} + </span>- If you must render HTML, sanitize it first with a trusted library (e.g. DOMPurify):
<span className="text-sm font-medium sm:text-xs" dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(product.price_html), }} ></span>🧰 Tools
🪛 Biome (1.9.4)
[error] 90-90: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
plan.md (1)
1-198
: Excellent migration plan! Very comprehensive and well-structured.The phased approach with infrastructure setup, scaffolding, incremental page migration, and testing provides a clear roadmap. The asset switching mechanism for toggling between Vue.js and React is a smart safety feature.
🧰 Tools
🪛 LanguageTool
[grammar] ~82-~82: ‘1 Page’ is missing a hyphen.
Context: ...se 3: Incremental Page Migration ### 3.1 Page 1: Product Selection/Catalog - [ ] Conv...(STARS_AND_STEPS)
[grammar] ~89-~89: ‘2 Page’ is missing a hyphen.
Context: ...eate UI design with Tailwind CSS ### 3.2 Page 2: Shopping Cart - [ ] Migrate shopping...(STARS_AND_STEPS)
[grammar] ~92-~92: Using ‘plenty’ without ‘of’ is considered to be informal.
Context: ...e product functionality - [ ] Implement quantity adjustments - [ ] Convert discount and tax calculat...(PLENTY_OF_NOUNS)
[grammar] ~95-~95: ‘3 Page’ is missing a hyphen.
Context: ...rt discount and tax calculations ### 3.3 Page 3: Checkout Flow - [ ] Convert customer...(STARS_AND_STEPS)
[grammar] ~101-~101: ‘4 Page’ is missing a hyphen.
Context: ... receipt generation and printing ### 3.4 Page 4: Order Management - [ ] Convert order...(STARS_AND_STEPS)
[typographical] ~169-~169: If specifying a range, consider using an en dash instead of a hyphen.
Context: ... Polish) Total Estimated Timeline: 9-12 weeks ## Success Criteria for Each Pag...(HYPHEN_TO_EN)
[uncategorized] ~196-~196: You might be missing the article “the” here.
Context: ...iance - [ ] Successful integration with WordPress ecosystem - [ ] Positive user feedback ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
9-9: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
src/frontend/components/PaymentModal.tsx (1)
128-151
: Well-implemented quick amount calculation logic!The quick amount buttons provide excellent UX by offering exact amount, rounded amounts, and common bill denominations. The logic is clean and user-friendly.
src/frontend/types/index.ts (1)
1-588
: Comprehensive type definitions provide excellent foundation for the React migration!The type definitions cover all major domain entities (products, orders, customers, cart) and POS-specific requirements. This will significantly improve code maintainability and developer experience.
src/frontend/api/index.ts (2)
360-408
: Well-structured customers API implementation.The customers API is properly typed and follows consistent patterns with appropriate use of the
Customer
type throughout.
478-487
: Clean and well-organized API export structure.The aggregated export pattern provides a nice, centralized API interface that's easy to import and use throughout the application.
src/frontend/components/HelpModal.tsx (3)
1-60
: LGTM! Well-structured help modal component.The component follows React best practices with proper TypeScript interfaces, early return pattern for conditional rendering, and good use of WordPress components and i18n. The keyboard shortcuts are clearly presented with consistent styling.
1-60
: LGTM! Well-structured help modal component.The component follows React best practices with proper TypeScript interfaces, early return pattern, and clean separation of concerns. The internationalization is properly implemented and the modal configuration is appropriate.
24-44
:✅ Verification successful
Verify CSS class definition for keyboard shortcut styling.
The component uses a CSS class "code" for styling keyboard shortcuts, but this class doesn't appear to be defined in 9E88 the provided context. Ensure this class is available in your stylesheet or consider using Tailwind classes instead.
🏁 Script executed:
#!/bin/bash # Description: Search for the 'code' CSS class definition in stylesheets # Expected: Find the CSS class definition or confirm it needs to be added echo "Searching for 'code' CSS class definition..." rg -i "\.code\s*\{" --type css rg -i "\.code\s*\{" --type scss rg -i "\.code\s*\{" --type less # Also check for any existing code-related styles echo "Searching for code-related styles..." rg -i "code.*\{" --type css --type scss --type less -A 3Length of output: 466
Verified
.code
CSS class definitionThe
.code
class is defined insrc/frontend/styles/main.css
under the.wepos-help-wrapper .code
selector, so your keyboard shortcut styling is already covered. No further changes needed here.src/frontend/components/ProductVariationSelector.tsx (2)
26-34
:✅ Verification successful
Verify variation matching algorithm.
The variation matching logic assumes that the product variations have an
attributes
array and that each attribute hasname
andoption
properties. This should be verified against the actual API response structure.
🏁 Script executed:
#!/bin/bash # Description: Check the ProductVariation type definition to verify attribute structure # Expected: Confirm that ProductVariation.attributes has name and option properties ast-grep --pattern 'interface ProductVariation { $$$ attributes: $_; $$$ }'Length of output: 2280
🏁 Script executed:
#!/bin/bash # Description: Verify that ProductVariationAttribute defines `name` and `option` properties rg -n "interface ProductVariationAttribute" -A5 src/frontend/types/index.tsLength of output: 197
Confirm variation matching algorithm
I’ve verified thatProductVariation.attributes
is defined asProductVariationAttribute[]
and thatProductVariationAttribute
includes bothname
andoption
properties (seesrc/frontend/types/index.ts:261-265
). The current matching logic correctly aligns with the API’s type definitions—no changes needed.
26-95
: LGTM! Solid variation selection logic.The component implements comprehensive variation matching logic with proper memoization for performance. The attribute selection handling and cart item construction are well-implemented.
src/frontend/components/CustomerModal.tsx (1)
1-353
: LGTM! Comprehensive customer management implementation.Overall, this is a well-structured customer modal with proper form handling, validation, and API integration. The edit/create mode handling is clean and the country/state dependency logic is well implemented.
@@ -36,11 +36,11 @@ mkdir -p "$BUILD_DIR" | |||
|
|||
# Run the build. | |||
# status "Installing dependencies... 📦" | |||
# npm install | |||
# pnpm install |
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
Re-enable dependency installation
The pnpm install
step is commented out, so fresh checkouts won’t have dependencies installed before building.
Proposed diff:
- # pnpm install
+ pnpm install
📝 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.
# pnpm install | |
pnpm install |
🤖 Prompt for AI Agents
In bin/zip.sh at line 39, the command 'pnpm install' is commented out, which
prevents automatic installation of dependencies on fresh checkouts. Uncomment
this line to ensure dependencies are installed before the build process runs.
[*.tsx,*.ts] | ||
indent_style = space | ||
indent_size = 2 | ||
|
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 the glob pattern for TS/TSX files
EditorConfig expects brace expansion rather than comma-separated patterns. The current header won’t match .ts
files.
Suggested change:
-[*.tsx,*.ts]
+[{*.ts,*.tsx}]
Or using brace syntax:
-[*.tsx,*.ts]
+[*.{ts,tsx}]
🤖 Prompt for AI Agents
In the .editorconfig file at lines 14 to 17, the glob pattern for matching TS
and TSX files is incorrect because it uses a comma-separated list instead of
brace expansion. Replace the pattern [*.tsx,*.ts] with the correct brace syntax
[*.{tsx,ts}] to ensure both file types are matched properly by EditorConfig.
module.exports = { | ||
plugins: { | ||
'@tailwindcss/postcss': {}, | ||
} | ||
} |
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
Add autoprefixer and correct plugin usage
Use the official Tailwind plugin name and include Autoprefixer to ensure vendor prefixes are applied.
Example refactor:
-module.exports = {
- plugins: {
- '@tailwindcss/postcss': {},
- }
-}
+module.exports = {
+ plugins: {
+ tailwindcss: {},
+ autoprefixer: {},
+ },
+}
📝 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.
module.exports = { | |
plugins: { | |
'@tailwindcss/postcss': {}, | |
} | |
} | |
module.exports = { | |
plugins: { | |
tailwindcss: {}, | |
autoprefixer: {}, | |
}, | |
} |
🤖 Prompt for AI Agents
In postcss.config.js lines 1 to 5, the Tailwind CSS plugin is incorrectly named
and Autoprefixer is missing. Replace '@tailwindcss/postcss' with the official
'tailwindcss' plugin name and add 'autoprefixer' to the plugins object to enable
vendor prefixing. Ensure both plugins are properly included as keys with empty
configuration objects.
<input | ||
type="text" | ||
id="product-search" | ||
placeholder={__('Search products...', 'wepos')} | ||
className="focus:ring-wepos-primary/20 focus:border-wepos-primary h-10 w-full rounded-lg border border-gray-300 bg-white px-4 py-2 pr-10 transition-colors focus:ring-2" | ||
/> |
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
Add search functionality and event handlers.
The input field is missing essential search functionality. Consider adding state management and event handlers.
+import React, { useState } from 'react';
import { __ } from '@wordpress/i18n';
import { Search } from 'lucide-react';
+interface SearchBarProps {
+ onSearch?: (query: string) => void;
+ placeholder?: string;
+}
-const SearchBar: React.FC = () => {
+const SearchBar: React.FC<SearchBarProps> = ({
+ onSearch,
+ placeholder = __('Search products...', 'wepos')
+}) => {
+ const [query, setQuery] = useState('');
+
+ const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+ const value = e.target.value;
+ setQuery(value);
+ onSearch?.(value);
+ };
return (
<div className="relative w-full flex-1 md:w-auto">
<div className="relative">
<input
type="text"
id="product-search"
- placeholder={__('Search products...', 'wepos')}
+ value={query}
+ >
+ placeholder={placeholder}
className="focus:ring-wepos-primary/20 focus:border-wepos-primary h-10 w-full rounded-lg border border-gray-300 bg-white px-4 py-2 pr-10 transition-colors focus:ring-2"
/>
📝 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.
<input | |
type="text" | |
id="product-search" | |
placeholder={__('Search products...', 'wepos')} | |
className="focus:ring-wepos-primary/20 focus:border-wepos-primary h-10 w-full rounded-lg border border-gray-300 bg-white px-4 py-2 pr-10 transition-colors focus:ring-2" | |
/> | |
import React, { useState } from 'react'; | |
import { __ } from '@wordpress/i18n'; | |
import { Search } from 'lucide-react'; | |
interface SearchBarProps { | |
onSearch?: (query: string) => void; | |
placeholder?: string; | |
} | |
const SearchBar: React.FC<SearchBarProps> = ({ | |
onSearch, | |
placeholder = __('Search products...', 'wepos'), | |
}) => { | |
const [query, setQuery] = useState(''); | |
const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>) => { | |
const value = e.target.value; | |
setQuery(value); | |
onSearch?.(value); | |
}; | |
return ( | |
<div className="relative w-full flex-1 md:w-auto"> | |
<div className="relative"> | |
<input | |
type="text" | |
id="product-search" | |
value={query} | |
onChange={handleInputChange} | |
placeholder={placeholder} | |
className="focus:ring-wepos-primary/20 focus:border-wepos-primary h-10 w-full rounded-lg border border-gray-300 bg-white px-4 py-2 pr-10 transition-colors focus:ring-2" | |
/> | |
</div> | |
</div> | |
); | |
}; | |
export default SearchBar; |
🤖 Prompt for AI Agents
In src/frontend/components/SearchBar.tsx around lines 9 to 14, the input field
lacks search functionality and event handlers. Add state management using
React's useState to track the input value, and implement an onChange event
handler to update this state as the user types. Also, add an onSubmit or
onKeyPress handler to trigger the search action when appropriate, ensuring the
component can handle user input and perform searches.
|
||
# Get Playwright version | ||
- name: Get installed Playwright version | ||
id: playwright-version | ||
working-directory: tests/pw | ||
run: | | ||
echo "PLAYWRIGHT_VERSION=$(npm ls @playwright/test --json | jq --raw-output '.dependencies["@playwright/test"].version')" >> $GITHUB_ENV | ||
echo "PLAYWRIGHT_VERSION=$(pnpm ls @playwright/test --json | jq --raw-output '.dependencies["@playwright/test"].version')" >> $GITHUB_ENV |
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 shell quoting issue to prevent globbing.
The static analysis tool flagged a potential issue with unquoted variables that could cause globbing and word splitting.
Apply this diff to fix the shell quoting issue:
- echo "PLAYWRIGHT_VERSION=$(pnpm ls @playwright/test --json | jq --raw-output '.dependencies["@playwright/test"].version')" >> $GITHUB_ENV
+ echo "PLAYWRIGHT_VERSION=$(pnpm ls @playwright/test --json | jq --raw-output '.dependencies["@playwright/test"].version')" >> "$GITHUB_ENV"
📝 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.
echo "PLAYWRIGHT_VERSION=$(pnpm ls @playwright/test --json | jq --raw-output '.dependencies["@playwright/test"].version')" >> $GITHUB_ENV | |
echo "PLAYWRIGHT_VERSION=$(pnpm ls @playwright/test --json | jq --raw-output '.dependencies[@\"playwright/test\"].version')" >> "$GITHUB_ENV" |
🤖 Prompt for AI Agents
In .github/workflows/e2e_api_tests.yml at line 153, the shell command uses an
unquoted variable which can cause globbing and word splitting issues. To fix
this, enclose the command substitution in double quotes to properly quote the
variable assignment and prevent unintended shell expansions.
const handleBlur = () => { | ||
setTimeout(() => { | ||
setShowResults(false); | ||
onBlur?.(); | ||
}, 200); | ||
}; |
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
Improve blur handler to prevent race conditions
The blur handler uses a hardcoded timeout without cleanup, which could cause race conditions and state updates after unmount.
Track and clean up the blur timeout:
+ const blurTimerRef = useRef<ReturnType<typeof setTimeout>>();
const handleBlur = () => {
- setTimeout(() => {
+ blurTimerRef.current = setTimeout(() => {
setShowResults(false);
onBlur?.();
}, 200);
};
And add to the cleanup effect:
useEffect(() => {
return () => {
if (debounceTimerRef.current) {
clearTimeout(debounceTimerRef.current);
}
if (blurTimerRef.current) {
clearTimeout(blurTimerRef.current);
}
};
}, []);
🤖 Prompt for AI Agents
In src/frontend/components/CustomerSearch.tsx around lines 133 to 138, the blur
handler uses a hardcoded setTimeout without cleanup, risking race conditions and
state updates after unmount. Fix this by storing the timeout ID in a ref (e.g.,
blurTimerRef) when calling setTimeout in handleBlur, and then clear this timeout
in the component's cleanup effect using clearTimeout to prevent any pending blur
actions after unmount.
|
||
// Refs | ||
const searchInputRef = useRef<HTMLInputElement>(null); | ||
const debounceTimerRef = useRef<NodeJS.Timeout>(); |
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 TypeScript type for browser environment
NodeJS.Timeout
is not the correct type for browser environments. Use ReturnType<typeof setTimeout>
for universal compatibility.
- const debounceTimerRef = useRef<NodeJS.Timeout>();
+ const debounceTimerRef = useRef<ReturnType<typeof setTimeout>>();
📝 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 debounceTimerRef = useRef<NodeJS.Timeout>(); | |
const debounceTimerRef = useRef<ReturnType<typeof setTimeout>>(); |
🤖 Prompt for AI Agents
In src/frontend/components/CustomerSearch.tsx at line 33, replace the type
annotation of debounceTimerRef from NodeJS.Timeout to ReturnType<typeof
setTimeout> to ensure compatibility with browser environments. This change will
correctly type the timer reference for universal use across different JavaScript
environments.
getTotalFee: (state: CartState): number => { | ||
const subtotal = selectors.getSubtotal(state); | ||
return state.fee_lines.reduce((total: number, fee: POSFeeLine) => { | ||
if (fee.fee_type === 'percent') { | ||
return total + (subtotal * parseFloat(fee.value)) / 100; | ||
} else { | ||
return total + parseFloat(fee.value); | ||
} | ||
}, 0); | ||
}, |
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 validation for fee value parsing
parseFloat
can return NaN
for invalid strings, which would corrupt the total calculations.
getTotalFee: (state: CartState): number => {
const subtotal = selectors.getSubtotal(state);
return state.fee_lines.reduce((total: number, fee: POSFeeLine) => {
+ const feeValue = parseFloat(fee.value);
+ if (isNaN(feeValue)) {
+ console.error(`Invalid fee value: ${fee.value}`);
+ return total;
+ }
if (fee.fee_type === 'percent') {
- return total + (subtotal * parseFloat(fee.value)) / 100;
+ return total + (subtotal * feeValue) / 100;
} else {
- return total + parseFloat(fee.value);
+ return total + feeValue;
}
}, 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.
getTotalFee: (state: CartState): number => { | |
const subtotal = selectors.getSubtotal(state); | |
return state.fee_lines.reduce((total: number, fee: POSFeeLine) => { | |
if (fee.fee_type === 'percent') { | |
return total + (subtotal * parseFloat(fee.value)) / 100; | |
} else { | |
return total + parseFloat(fee.value); | |
} | |
}, 0); | |
}, | |
getTotalFee: (state: CartState): number => { | |
const subtotal = selectors.getSubtotal(state); | |
return state.fee_lines.reduce((total: number, fee: POSFeeLine) => { | |
const feeValue = parseFloat(fee.value); | |
if (isNaN(feeValue)) { | |
console.error(`Invalid fee value: ${fee.value}`); | |
return total; | |
} | |
if (fee.fee_type === 'percent') { | |
return total + (subtotal * feeValue) / 100; | |
} else { | |
return total + feeValue; | |
} | |
}, 0); | |
}, |
🤖 Prompt for AI Agents
In src/frontend/store/cart/selectors.ts around lines 31 to 40, the code uses
parseFloat on fee.value without validating the result, which can lead to NaN
values corrupting the total fee calculation. To fix this, add validation after
parsing fee.value to check if the result is a valid number before using it in
calculations. If parseFloat returns NaN, treat the fee value as zero or skip
adding it to the total to ensure the total fee remains accurate.
getFeeAmount: (state: CartState, fee: POSFeeLine): number => { | ||
const subtotal = selectors.getSubtotal(state); | ||
if (fee.fee_type === 'percent') { | ||
return (subtotal * parseFloat(fee.value)) / 100; | ||
} else { | ||
return parseFloat(fee.value); | ||
} | ||
}, |
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 validation for fee amount calculation
Similar to getTotalFee
, this function needs validation for parseFloat
.
getFeeAmount: (state: CartState, fee: POSFeeLine): number => {
const subtotal = selectors.getSubtotal(state);
+ const feeValue = parseFloat(fee.value);
+ if (isNaN(feeValue)) {
+ console.error(`Invalid fee value: ${fee.value}`);
+ return 0;
+ }
if (fee.fee_type === 'percent') {
- return (subtotal * parseFloat(fee.value)) / 100;
+ return (subtotal * feeValue) / 100;
} else {
- return parseFloat(fee.value);
+ return feeValue;
}
},
📝 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.
getFeeAmount: (state: CartState, fee: POSFeeLine): number => { | |
const subtotal = selectors.getSubtotal(state); | |
if (fee.fee_type === 'percent') { | |
return (subtotal * parseFloat(fee.value)) / 100; | |
} else { | |
return parseFloat(fee.value); | |
} | |
}, | |
getFeeAmount: (state: CartState, fee: POSFeeLine): number => { | |
const subtotal = selectors.getSubtotal(state); | |
const feeValue = parseFloat(fee.value); | |
if (isNaN(feeValue)) { | |
console.error(`Invalid fee value: ${fee.value}`); | |
return 0; | |
} | |
if (fee.fee_type === 'percent') { | |
return (subtotal * feeValue) / 100; | |
} else { | |
return feeValue; | |
} | |
}, |
🤖 Prompt for AI Agents
In src/frontend/store/cart/selectors.ts around lines 66 to 73, the getFeeAmount
function uses parseFloat on fee.value without validation, which can lead to NaN
or incorrect calculations. Add validation to ensure parseFloat returns a valid
number before using it in calculations, and handle invalid values appropriately,
similar to the approach used in getTotalFee.
// Add rounded up amounts | ||
const rounded5 = Math.ceil(total / 5) * 5; | ||
const rounded10 = Math.ceil(total / 10) * 10; | ||
|
||
if (rounded5 > total) quickAmounts.push(rounded5); | ||
if (rounded10 > total && rounded10 !== rounded5) | ||
quickAmounts.push(rounded10); | ||
|
||
// Add common bill denominations | ||
const bills = [20, 50, 100, 200, 500]; | ||
bills.forEach((bill) => { | ||
if (bill > total && !quickAmounts.includes(bill)) { | ||
quickAmounts.push(bill); | ||
} | ||
}); | ||
|
||
// Take first 6 amounts and sort | ||
return quickAmounts.slice(0, 6).sort((a, b) => a - b); | ||
})().map((amount) => ( | ||
<button | ||
key={amount} | ||
type="button" | ||
F438 className="text-wepos-primary border-wepos-primary hover:bg-wepos-primary/5 focus:ring-wepos-primary/20 rounded-lg border border-transparent bg-white px-4 py-2 text-sm font-medium transition-all duration-200 focus:ring-2 focus:ring-offset-2 focus:outline-none" | ||
=> onCashAmountChange(amount.toString())} | ||
> | ||
{formatPrice(amount)} | ||
</button> | ||
))} | ||
</div> | ||
</div> | ||
|
||
{changeAmount > 0 && ( | ||
<div className="rounded-lg border border-green-200 bg-green-50 p-4"> | ||
<div className="flex items-center justify-between"> | ||
<span className="font-medium text-green-700"> | ||
{__('Change Due:', 'wepos')} | ||
</span> | ||
<span className="text-xl font-bold text-green-800"> | ||
{formatPrice(changeAmount)} | ||
</span> | ||
</div> | ||
</div> | ||
)} | ||
</div> | ||
</div> | ||
)} | ||
</div> | ||
|
||
<div className="space-y-4"> | ||
<div className="rounded-lg bg-gray-50 p-6"> | ||
<h3 className="mb-4 text-lg font-semibold text-gray-800"> | ||
{__('Order Summary', 'wepos')} | ||
</h3> | ||
|
||
<div className="space-y-3"> | ||
<div className="flex items-center justify-between py-2"> | ||
<span className="text-gray-600"> | ||
{__('Subtotal:', 'wepos')} | ||
</span> | ||
<span className="font-medium text-gray-800"> | ||
{formatPrice(total)} | ||
</span> | ||
</div> | ||
</div> | ||
|
||
<div className="mt-4 border-t border-gray-300 pt-4"> | ||
<div className="flex items-center justify-between text-xl font-bold text-gray-800"> | ||
<span>{__('Total:', 'wepos')}</span> | ||
<span className="text-wepos-primary"> | ||
{formatPrice(total)} | ||
</span> | ||
</div> | ||
</div> | ||
</div> | ||
</div> | ||
</div> | ||
|
||
<div className="flex items-center justify-between gap-4 border-t border-gray-200 pt-4"> | ||
<button | ||
className="rounded-lg border border-gray-200 border-transparent bg-gray-100 px-4 py-2 font-medium text-gray-700 transition-all duration-200 hover:bg-gray-200 focus:ring-2 focus:ring-gray-500/20 focus:ring-offset-2 focus:outline-none" | ||
> | ||
type="button" | ||
> | ||
{__('← Back to Sale', 'wepos')} | ||
</button> | ||
<button | ||
className="bg-wepos-primary hover:bg-wepos-primary-hover focus:ring-wepos-primary/20 disabled:hover:bg-wepos-primary rounded-lg border border-transparent px-4 py-2 font-medium text-white transition-all duration-200 focus:ring-2 focus:ring-offset-2 focus:outline-none disabled:cursor-not-allowed disabled:opacity-50" | ||
> | ||
disabled={!ableToProcess} | ||
type="button" | ||
> | ||
{__('Process Payment', 'wepos')} • {formatPrice(total)} | ||
</button> | ||
</div> | ||
</div> | ||
</Modal> | ||
); | ||
}; | ||
|
||
export default PaymentModal; |
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
Consider adding error handling for store operations.
The component doesn't handle potential errors when accessing WordPress data stores. Consider wrapping store operations in try-catch blocks or using error boundaries.
Would you like me to provide an example of how to add proper error handling for the store operations?
🤖 Prompt for AI Agents
In src/frontend/components/PaymentModal.tsx around lines 23 to 233, the calls to
select data from WordPress stores do not handle potential errors, which could
cause the component to fail silently or crash. Wrap the store access logic
inside try-catch blocks within the useSelect hooks to catch any exceptions. In
the catch block, handle errors gracefully by logging them or providing fallback
values to ensure the component remains stable even if store data retrieval
fails.
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.
Review continued from previous batch...
// Get cart data from cart store | ||
const { total } = useSelect((select) => { | ||
const store = select(CART_STORE_NAME) as any; | ||
return { | ||
total: store.getTotal(), | ||
}; | ||
}, []); |
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.
Avoid using as any
type assertion for better type safety.
The as any
assertion bypasses TypeScript's type checking. Consider properly typing the store or using a type guard.
- const { total } = useSelect((select) => {
- const store = select(CART_STORE_NAME) as any;
- return {
- total: store.getTotal(),
- };
- }, []);
+ const { total } = useSelect((select) => {
+ const store = select(CART_STORE_NAME);
+ return {
+ total: store.getTotal(),
+ };
+ }, []);
If the store types aren't available, consider creating proper type definitions for the stores in your types directory.
📝 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.
// Get cart data from cart store | |
const { total } = useSelect((select) => { | |
const store = select(CART_STORE_NAME) as any; | |
return { | |
total: store.getTotal(), | |
}; | |
}, []); | |
// Get cart data from cart store | |
const { total } = useSelect((select) => { | |
const store = select(CART_STORE_NAME); | |
return { | |
total: store.getTotal(), | |
}; | |
}, []); |
🤖 Prompt for AI Agents
In src/frontend/components/PaymentModal.tsx around lines 35 to 41, avoid using
the `as any` type assertion on the store to improve type safety. Instead, import
or define the correct type for the cart store and use it to type the `store`
variable properly. If the store types are not available, create appropriate type
definitions for the cart store in your types directory and apply them here to
ensure TypeScript can check the store's methods and properties correctly.
// Get gateways from products store | ||
const { availableGateways } = useSelect((select) => { | ||
const store = select(PRODUCTS_STORE_NAME) as any; | ||
return { | ||
availableGateways: store.getGateways(), | ||
}; | ||
}, []); |
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.
Type safety issue with as any
assertion.
Same issue as above - avoid bypassing TypeScript's type system.
- const { availableGateways } = useSelect((select) => {
- const store = select(PRODUCTS_STORE_NAME) as any;
- return {
- availableGateways: store.getGateways(),
- };
- }, []);
+ const { availableGateways } = useSelect((select) => {
+ const store = select(PRODUCTS_STORE_NAME);
+ return {
+ availableGateways: store.getGateways(),
+ };
+ }, []);
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/frontend/components/PaymentModal.tsx around lines 43 to 49, the use of
'as any' to cast the store bypasses TypeScript's type safety. To fix this,
replace 'as any' with the correct type for the PRODUCTS_STORE_NAME selector,
ensuring the store variable is properly typed according to the store's
interface. This will maintain type safety and avoid using 'any'.
export interface POSSettings { | ||
barcode_scanner: boolean; | ||
receipt_printer: boolean; | ||
cash_drawer: boolean; | ||
default_customer: number | null; | ||
tax_calculation: 'automatic' | 'manual'; | ||
discount_permission: boolean; | ||
} | ||
|
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.
Critical: Duplicate POSSettings
interface declarations.
You have two conflicting declarations of POSSettings
interface (lines 443-451 and 531-537). This will cause TypeScript compilation errors.
Remove the duplicate declaration at lines 531-537 and merge any unique properties into the first declaration at lines 443-451.
Also applies to: 531-537
🤖 Prompt for AI Agents
In src/frontend/types/index.ts between lines 443-451 and 531-537, there are
duplicate declarations of the POSSettings interface causing TypeScript errors.
Remove the second declaration at lines 531-537 and merge any unique properties
it contains into the first declaration at lines 443-451 to consolidate all
properties into a single interface definition.
} | ||
|
||
export interface POSSettings { | ||
wepos_general: any; |
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
Define proper type for wepos_general
instead of any
.
Create a proper interface for the general settings:
interface WeposGeneralSettings {
// Add specific properties based on your settings structure
barcode_scanner?: boolean;
receipt_printer?: boolean;
// ... other settings
}
export interface POSSettings {
wepos_general: WeposGeneralSettings;
// ... rest of the interface
}
🤖 Prompt for AI Agents
In src/frontend/types/index.ts at line 532, the type of wepos_general is
currently any, which lacks type safety. Define a new interface
WeposGeneralSettings with specific properties like barcode_scanner and
receipt_printer, then update the POSSettings interface to use wepos_general:
WeposGeneralSettings instead of any. This will provide proper typing and improve
code clarity.
billing: any; | ||
shipping: any; |
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.
Replace any
types with proper type definitions.
Using any
defeats TypeScript's type safety. Consider using the already defined BillingAddress
and ShippingAddress
interfaces.
- billing: any;
- shipping: any;
+ billing: BillingAddress;
+ shipping: ShippingAddress;
📝 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.
billing: any; | |
shipping: any; | |
billing: BillingAddress; | |
shipping: ShippingAddress; |
🤖 Prompt for AI Agents
In src/frontend/types/index.ts at lines 522 to 523, replace the `any` types for
`billing` and `shipping` with the appropriate interfaces `BillingAddress` and
`ShippingAddress` respectively to enforce type safety and leverage existing type
definitions.
processPayment: async (orderData: any): Promise<any> => { | ||
const response = await apiFetch({ | ||
path: `/${API_BASE.WEPOS}/payment/process`, | ||
method: 'POST', | ||
data: orderData, | ||
}); | ||
|
||
return response; | ||
}, | ||
|
||
// Get payment gateways | ||
getPaymentGateways: async (): Promise<POSGateway[]> => { | ||
const response = await apiFetch({ | ||
path: `/${API_BASE.WEPOS}/payment/gateways`, | ||
method: 'GET', | ||
}); | ||
|
||
return response as POSGateway[]; | ||
}, |
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 type safety and path consistency issues.
The payment API has the same issues with any
types and inconsistent path formatting.
- processPayment: async (orderData: any): Promise<any> => {
+ processPayment: async (orderData: Partial<Order>): Promise<Order> => {
const response = await apiFetch({
- path: `/${API_BASE.WEPOS}/payment/process`,
+ path: `${API_BASE.WEPOS}/payment/process`,
method: 'POST',
data: orderData,
});
- return response;
+ return response as Order;
},
// Get payment gateways
getPaymentGateways: async (): Promise<POSGateway[]> => {
const response = await apiFetch({
- path: `/${API_BASE.WEPOS}/payment/gateways`,
+ path: `${API_BASE.WEPOS}/payment/gateways`,
method: 'GET',
});
📝 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.
processPayment: async (orderData: any): Promise<any> => { | |
const response = await apiFetch({ | |
path: `/${API_BASE.WEPOS}/payment/process`, | |
method: 'POST', | |
data: orderData, | |
}); | |
return response; | |
}, | |
// Get payment gateways | |
getPaymentGateways: async (): Promise<POSGateway[]> => { | |
const response = await apiFetch({ | |
path: `/${API_BASE.WEPOS}/payment/gateways`, | |
method: 'GET', | |
}); | |
return response as POSGateway[]; | |
}, | |
processPayment: async (orderData: Partial<Order>): Promise<Order> => { | |
const response = await apiFetch({ | |
path: `${API_BASE.WEPOS}/payment/process`, | |
method: 'POST', | |
data: orderData, | |
}); | |
return response as Order; | |
}, | |
// Get payment gateways | |
getPaymentGateways: async (): Promise<POSGateway[]> => { | |
const response = await apiFetch({ | |
path: `${API_BASE.WEPOS}/payment/gateways`, | |
method: 'GET', | |
}); | |
return response as POSGateway[]; | |
}, |
🤖 Prompt for AI Agents
In src/frontend/api/index.ts around lines 413 to 431, the processPayment
function uses 'any' for orderData and return type, and the API paths have
inconsistent formatting. Replace 'any' with appropriate types for orderData and
the response to improve type safety. Also, ensure the API paths use consistent
formatting, such as removing leading slashes if the base path already includes
them, to maintain path consistency across calls.
path: `/${API_BASE.WEPOS}/settings`, | ||
method: 'GET', | ||
}); | ||
|
||
return response as POSSettings; | ||
}, | ||
|
||
updateSettings: async (settings: any): Promise<APIResponse<any>> => { | ||
const response = await apiFetch({ | ||
path: `${API_BASE.WEPOS}/settings`, | ||
method: 'POST', | ||
data: settings, | ||
}); | ||
|
||
return response as APIResponse<any>; |
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
Maintain consistency in API paths and improve type safety.
getSettings: async (): Promise<POSSettings> => {
const response = await apiFetch({
- path: `/${API_BASE.WEPOS}/settings`,
+ path: `${API_BASE.WEPOS}/settings`,
method: 'GET',
});
return response as POSSettings;
},
- updateSettings: async (settings: any): Promise<APIResponse<any>> => {
+ updateSettings: async (settings: Partial<POSSettings>): Promise<APIResponse<POSSettings>> => {
const response = await apiFetch({
path: `${API_BASE.WEPOS}/settings`,
method: 'POST',
data: settings,
});
- return response as APIResponse<any>;
+ return response as APIResponse<POSSettings>;
},
📝 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.
path: `/${API_BASE.WEPOS}/settings`, | |
method: 'GET', | |
}); | |
return response as POSSettings; | |
}, | |
updateSettings: async (settings: any): Promise<APIResponse<any>> => { | |
const response = await apiFetch({ | |
path: `${API_BASE.WEPOS}/settings`, | |
method: 'POST', | |
data: settings, | |
}); | |
return response as APIResponse<any>; | |
getSettings: async (): Promise<POSSettings> => { | |
const response = await apiFetch({ | |
path: `${API_BASE.WEPOS}/settings`, | |
method: 'GET', | |
}); | |
return response as POSSettings; | |
}, | |
updateSettings: async (settings: Partial<POSSettings>): Promise<APIResponse<POSSettings>> => { | |
const response = await apiFetch({ | |
path: `${API_BASE.WEPOS}/settings`, | |
method: 'POST', | |
data: settings, | |
}); | |
return response as APIResponse<POSSettings>; | |
}, |
🤖 Prompt for AI Agents
In src/frontend/api/index.ts around lines 439 to 453, the API path for
updateSettings is inconsistent with the getSettings path and the settings
parameter uses a generic any type. Update the path in updateSettings to use the
same format as getSettings by prefixing with a slash and the API_BASE.WEPOS
constant. Replace the any type for the settings parameter with a more specific
type that matches the expected settings structure to improve type safety.
email: customerForm.email, | ||
first_name: customerForm.first_name, | ||
last_name: customerForm.last_name, | ||
username: isEditMode ? editingCustomer!.username : customerForm.email, |
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
Potential null reference error in edit mode.
The code uses editingCustomer!.username
which could throw a runtime error if editingCustomer
is unexpectedly null in edit mode.
- username: isEditMode ? editingCustomer!.username : customerForm.email,
+ username: isEditMode ? editingCustomer?.username || customerForm.email : customerForm.email,
📝 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.
username: isEditMode ? editingCustomer!.username : customerForm.email, | |
username: isEditMode | |
? editingCustomer?.username || customerForm.email | |
: customerForm.email, |
🤖 Prompt for AI Agents
In src/frontend/components/CustomerModal.tsx at line 141, the use of
`editingCustomer!.username` risks a runtime error if `editingCustomer` is null
in edit mode. To fix this, add a null check for `editingCustomer` before
accessing `username`. You can use optional chaining or a conditional to safely
access `username` only if `editingCustomer` is not null, and provide a fallback
value if it is null.
Replace non-null assertions with proper null checks.
The non-null assertions (!
) could cause runtime errors if editingCustomer
is unexpectedly null.
- username: isEditMode ? editingCustomer!.username : customerForm.email,
+ username: isEditMode ? editingCustomer?.username || customerForm.email : customerForm.email,
- savedCustomer = await posAPI.customers.updateCustomer(
- editingCustomer!.id,
- customerData,
- );
+ if (!editingCustomer?.id) {
+ throw new Error('Cannot update customer: missing customer ID');
+ }
+ savedCustomer = await posAPI.customers.updateCustomer(
+ editingCustomer.id,
+ customerData,
+ );
Also applies to: 173-173
🤖 Prompt for AI Agents
In src/frontend/components/CustomerModal.tsx at lines 141 and 173, replace the
non-null assertions on editingCustomer with proper null checks to avoid
potential runtime errors. Add conditional checks to ensure editingCustomer is
not null before accessing its properties, and handle the null case
appropriately, such as providing a fallback value or rendering logic.
); | ||
|
||
const cartItem: CartItem = { | ||
id: Date.now(), // Generate a temporary ID |
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
Improve temporary ID generation to prevent collisions.
Using Date.now()
for temporary IDs could cause collisions if multiple variations are added rapidly within the same millisecond.
Consider using a more robust ID generation approach:
- id: Date.now(), // Generate a temporary ID
+ id: Date.now() + Math.random(), // Generate a more unique temporary ID
Or better yet, use a proper UUID library or crypto.randomUUID() if available.
📝 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.
id: Date.now(), // Generate a temporary ID | |
id: Date.now() + Math.random(), // Generate a more unique temporary ID |
🤖 Prompt for AI Agents
In src/frontend/components/ProductVariationSelector.tsx at line 71, the
temporary ID generation using Date.now() can cause collisions when multiple
variations are added quickly. Replace Date.now() with a more robust method such
as crypto.randomUUID() if supported, or integrate a UUID library to generate
unique IDs for each variation to ensure uniqueness and prevent collisions.
const resetForm = () => { | ||
setCustomerForm({ | ||
first_name: '', | ||
last_name: '', | ||
email: '', | ||
address_1: '', | ||
address_2: '', | ||
city: '', | ||
state: '', | ||
postcode: '', | ||
country: '', | ||
phone: '', | ||
}); | ||
setSelectedCountry(''); | ||
}; |
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
Fix form reset to include all state variables.
The resetForm
function doesn't reset availableStates
, which could cause stale state issues.
const resetForm = () => {
setCustomerForm({
first_name: '',
last_name: '',
email: '',
address_1: '',
address_2: '',
city: '',
state: '',
postcode: '',
country: '',
phone: '',
});
setSelectedCountry('');
+ setAvailableStates([]);
};
📝 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 resetForm = () => { | |
setCustomerForm({ | |
first_name: '', | |
last_name: '', | |
email: '', | |
address_1: '', | |
address_2: '', | |
city: '', | |
state: '', | |
postcode: '', | |
country: '', | |
phone: '', | |
}); | |
setSelectedCountry(''); | |
}; | |
const resetForm = () => { | |
setCustomerForm({ | |
first_name: '', | |
last_name: '', | |
email: '', | |
address_1: '', | |
address_2: '', | |
city: '', | |
state: '', | |
postcode: '', | |
country: '', | |
phone: '', | |
}); | |
setSelectedCountry(''); | |
setAvailableStates([]); | |
}; |
🤖 Prompt for AI Agents
In src/frontend/components/CustomerModal.tsx around lines 196 to 210, the
resetForm function resets customerForm and selectedCountry but does not reset
availableStates, which may lead to stale state issues. Update resetForm to also
reset availableStates to its initial or empty state to ensure all related state
variables are cleared properly.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Style
Refactor