-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Dashboard UI fixes #3715
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
Dashboard UI fixes #3715
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis set of changes primarily refactors and enhances the order history and asset gallery UI components. The order history timeline now groups secondary events, introduces collapsible UI sections, and adds richer iconography and note controls. Asset bulk actions and data table bulk actions menus are now dynamically positioned based on viewport visibility. Several UI components received style updates, and some navigation and formatting logic was adjusted. Additionally, a new hook was added to manage floating bulk actions positioning, and a new date display component with tooltip was introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OrderHistory
participant HistoryEntry
participant UI
User->>OrderHistory: View order history
OrderHistory->>OrderHistory: Group entries into primary/secondary
OrderHistory->>UI: Render primary events individually
OrderHistory->>UI: Render secondary events in collapsible groups
User->>UI: Toggle group (expand/collapse)
UI->>OrderHistory: Update expandedGroups state
OrderHistory->>UI: Re-render group as expanded/collapsed
OrderHistory->>HistoryEntry: Render entry with icon, title, and controls
sequenceDiagram
participant User
participant AssetGallery
participant AssetBulkActions
participant UI
User->>AssetGallery: Select assets
AssetGallery->>AssetBulkActions: Update selection
AssetBulkActions->>UI: Calculate position based on gallery and viewport
UI->>AssetBulkActions: Render floating menu at computed position
User->>UI: Scroll or resize window
UI->>AssetBulkActions: Recalculate and update menu position
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 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: 4
🔭 Outside diff range comments (1)
packages/dashboard/src/lib/components/layout/nav-main.tsx (1)
226-226
: Missing localization for "Administration" labelThe "Administration" label should use localization via the Trans component to be consistent with the coding guidelines.
- <SidebarGroupLabel>Administration</SidebarGroupLabel> + <SidebarGroupLabel><Trans>Administration</Trans></SidebarGroupLabel>
🧹 Nitpick comments (2)
packages/dashboard/src/lib/components/data-table/data-table-bulk-actions.tsx (1)
59-107
: Consider throttling scroll and resize event handlersThe dynamic positioning implementation is well-structured, but the scroll and resize event handlers fire frequently and could impact performance. Consider throttling these handlers to improve performance on lower-end devices.
Add throttling to the event handlers:
+import { useRef, useEffect, useState, useCallback } from 'react'; + +// Add throttle utility at the top of the component +const throttle = (func: Function, delay: number) => { + let timeoutId: NodeJS.Timeout | null = null; + let lastExecTime = 0; + return (...args: any[]) => { + const currentTime = Date.now(); + if (currentTime - lastExecTime > delay) { + func(...args); + lastExecTime = currentTime; + } else { + if (timeoutId) clearTimeout(timeoutId); + timeoutId = setTimeout(() => { + func(...args); + lastExecTime = Date.now(); + }, delay - (currentTime - lastExecTime)); + } + }; +}; // Inside the component: useEffect(() => { if (selection.length === 0) return; const updatePosition = () => { // ... existing logic }; + const throttledUpdatePosition = throttle(updatePosition, 100); updatePosition(); - window.addEventListener('scroll', updatePosition); - window.addEventListener('resize', updatePosition); + window.addEventListener('scroll', throttledUpdatePosition); + window.addEventListener('resize', throttledUpdatePosition); return () => { - window.removeEventListener('scroll', updatePosition); - window.removeEventListener('resize', updatePosition); + window.removeEventListener('scroll', throttledUpdatePosition); + window.removeEventListener('resize', throttledUpdatePosition); }; }, [selection.length]);packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx (1)
15-15
: Remove unuseduseRef
import.The
useRef
hook is imported but not used anywhere in the component.-import { useEffect, useRef, useState } from 'react'; +import { useEffect, useState } from 'react';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
(4 hunks)packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
(3 hunks)packages/dashboard/src/lib/components/data-table/data-table-bulk-actions.tsx
(3 hunks)packages/dashboard/src/lib/components/layout/nav-main.tsx
(2 hunks)packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
(3 hunks)packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx
(2 hunks)packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
(3 hunks)packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx
(1 hunks)packages/dashboard/src/lib/components/shared/history-timeline/history-timeline.tsx
(1 hunks)packages/dashboard/src/lib/framework/defaults.ts
(2 hunks)packages/dashboard/src/lib/hooks/use-local-format.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
packages/dashboard/src/**/*.{tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{tsx,jsx}
: Use React for all UI components in the dashboard package.
Prefer re-using components from /src/lib/components, especially Shadcn components from /src/lib/components/ui.
Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
All labels or user-facing messages should use localization via the Trans component or useLingui().
Files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-timeline.tsx
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/lib/components/data-table/data-table-bulk-actions.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
packages/dashboard/src/lib/components/**/*.{tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
Use Shadcn UI and Tailwind CSS for UI components.
Files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-timeline.tsx
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/lib/components/data-table/data-table-bulk-actions.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
packages/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{ts,tsx}
: Use TanStack Query (useQuery or useMutation) for data fetching; do not use Apollo Client.
When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
When performing mutations, follow the provided useMutation pattern: use graphql() for the mutation document, api.mutate for mutationFn, and do not pass variables at declaration.
Files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/framework/defaults.ts
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-timeline.tsx
packages/dashboard/src/lib/hooks/use-local-format.ts
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/lib/components/data-table/data-table-bulk-actions.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
packages/dashboard/src/**/*.{tsx,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
React component props objects should be typed as Readonly<...>.
Files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/framework/defaults.ts
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-timeline.tsx
packages/dashboard/src/lib/hooks/use-local-format.ts
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/lib/components/data-table/data-table-bulk-actions.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
packages/dashboard/src/app/routes/**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
Use TanStack Router for all routing and navigation in the dashboard app.
Files:
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
packages/dashboard/src/app/routes/**
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
Do not create new directories or files in /src/app/routes/ that do not fit the pattern: route files (except those in /components, /hooks, or ending with .graphql.ts).
Files:
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
🧠 Learnings (10)
📚 Learning: applies to packages/dashboard/src/lib/components/**/*.{tsx,jsx} : use shadcn ui and tailwind css for...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/lib/components/**/*.{tsx,jsx} : Use Shadcn UI and Tailwind CSS for UI components.
Applied to files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-timeline.tsx
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
📚 Learning: applies to packages/dashboard/src/app/routes/**/*.tsx : use tanstack router for all routing and navi...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/app/routes/**/*.tsx : Use TanStack Router for all routing and navigation in the dashboard app.
Applied to files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : use react for all ui components in the dashboard ...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : Use React for all UI components in the dashboard package.
Applied to files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : prefer re-using components from /src/lib/componen...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : Prefer re-using components from /src/lib/components, especially Shadcn components from /src/lib/components/ui.
Applied to files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-timeline.tsx
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : all labels or user-facing messages should use loc...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : All labels or user-facing messages should use localization via the Trans component or useLingui().
Applied to files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : prefer using the formfieldwrapper component for f...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
Applied to files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,ts} : react component props objects should be typed as r...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,ts} : React component props objects should be typed as Readonly<...>.
Applied to files:
packages/dashboard/src/lib/components/layout/nav-main.tsx
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{ts,tsx} : use tanstack query (usequery or usemutation) for d...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : Use TanStack Query (useQuery or useMutation) for data fetching; do not use Apollo Client.
Applied to files:
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{ts,tsx} : when performing mutations, follow the provided use...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : When performing mutations, follow the provided useMutation pattern: use graphql() for the mutation document, api.mutate for mutationFn, and do not pass variables at declaration.
Applied to files:
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{ts,tsx} : when creating usequery calls, follow the provided ...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
Applied to files:
packages/dashboard/src/lib/components/data-input/rich-text-input.tsx
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx
🧬 Code Graph Analysis (2)
packages/dashboard/src/lib/components/layout/nav-main.tsx (1)
packages/dashboard/src/lib/components/layout/nav-item-wrapper.tsx (1)
NavItemWrapper
(25-107)
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx (4)
packages/dashboard/src/lib/hooks/use-page-block.tsx (1)
usePageBlock
(12-18)packages/dashboard/src/lib/lib/trans.tsx (1)
Trans
(6-8)packages/dashboard/src/lib/components/ui/dropdown-menu.tsx (2)
DropdownMenu
(205-205)DropdownMenuTrigger
(219-219)packages/dashboard/src/lib/components/ui/button.tsx (1)
Button
(58-58)
🔇 Additional comments (14)
packages/dashboard/src/lib/framework/defaults.ts (1)
128-128
: Navigation menu reordering looks good.The order values are correctly swapped to move Settings (order: 100) before System (order: 200) in the bottom navigation menu.
Also applies to: 156-156
packages/dashboard/src/lib/components/shared/asset/asset-gallery.tsx (1)
235-235
: Layout enhancements support dynamic positioning.The addition of the
relative
class provides positioning context for child components, and thedata-asset-gallery
attribute enables the AssetBulkActions component to calculate proper positioning relative to the gallery container.Also applies to: 294-294
packages/dashboard/src/lib/hooks/use-local-format.ts (1)
80-103
: Improved relative date formatting with proper rounding.The addition of
Math.floor()
ensures that relative time values are always whole numbers, preventing fractional displays like "1.5 hours ago" and providing consistent user-friendly formatting.packages/dashboard/src/lib/components/data-input/rich-text-input.tsx (1)
29-29
: Excellent internal update tracking implementation.The
isInternalUpdate
ref prevents unnecessary editor content resets and cursor jumps by distinguishing between internal editor changes and external prop updates. The conditional logic inuseLayoutEffect
ensures the editor only updates when the externalvalue
prop changes, while preserving selection state.Also applies to: 38-38, 49-57
packages/dashboard/src/lib/components/shared/history-timeline/history-entry.tsx (1)
1-2
: Excellent component refactor with improved separation of concerns.The refactor successfully:
- Removes complex editing controls, simplifying the component's responsibility to display only
- Adds semantic color coding via
getIconColor()
for better visual feedback on entry types- Implements conditional styling with the
isPrimary
prop for visual hierarchy- Provides proper fallback actor name resolution with
getActorName()
- Maintains proper TypeScript typing with
Readonly<HistoryEntryProps>
The color mapping logic appropriately categorizes success states (payments settled, orders delivered) and destructive states (cancellations) while using neutral colors for other entries.
Also applies to: 17-30, 41-76, 78-123
packages/dashboard/src/lib/components/layout/nav-main.tsx (1)
153-153
: Consistent offset prop usageThe addition of
offset={true}
to NavItemWrapper components in the bottom section aligns with the UI layout improvements mentioned in the PR objectives.Also applies to: 170-170
packages/dashboard/src/lib/components/shared/history-timeline/history-timeline.tsx (1)
46-51
: Well-executed timeline UI improvementsThe visual enhancements including reduced padding, gradient timeline, and proper spacing between entries improve the timeline's visual hierarchy and readability.
packages/dashboard/src/lib/components/shared/history-timeline/history-note-input.tsx (1)
22-32
: Clean and modern UI refinementsThe styling updates create a more subtle and polished interface with improved focus states and consistent sizing.
packages/dashboard/src/lib/components/data-table/data-table-bulk-actions.tsx (1)
30-30
: Good defensive programming practicesUsing optional chaining for
blockId
access and the querySelector with multiple fallback selectors shows good defensive programming.Also applies to: 64-64
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx (2)
61-85
: Well-structured event grouping logicThe
isPrimaryEvent
function and the grouping algorithm effectively separate primary from secondary events and create logical groups for the collapsible UI. This enhances the user experience by reducing visual clutter.Also applies to: 88-126
137-225
: Comprehensive event type handlingThe enhanced
getTimelineIcon
andgetTitle
functions provide rich, context-specific icons and localized titles for all event types, significantly improving the timeline's visual communication.packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx (3)
98-100
: Good defensive rendering approach.Preventing render until position is calculated avoids visual glitches.
124-132
: Well-implemented floating menu positioning.The fixed positioning with dynamic coordinates and transform centering provides a smooth floating UI experience. Good use of Tailwind classes for styling.
138-138
: Appropriate shadow removal.Removing the button's shadow when the container already has
shadow-2xl
prevents visual clutter.
{entry.type === 'ORDER_STATE_TRANSITION' && entry.data.from !== 'Created' && ( | ||
<p className="text-xs text-muted-foreground"> | ||
<Trans> | ||
From {entry.data.from} to {entry.data.to} | ||
</Trans> | ||
</p> | ||
)} | ||
{entry.type === 'ORDER_PAYMENT_TRANSITION' && ( | ||
<p className="text-xs text-muted-foreground"> | ||
<Trans> | ||
Payment #{entry.data.paymentId} transitioned to {entry.data.to} | ||
</Trans> | ||
</p> | ||
)} | ||
{entry.type === 'ORDER_REFUND_TRANSITION' && ( | ||
<p className="text-xs text-muted-foreground"> | ||
<Trans> | ||
Refund #{entry.data.refundId} transitioned to {entry.data.to} | ||
</Trans> | ||
</p> | ||
)} | ||
{entry.type === 'ORDER_FULFILLMENT_TRANSITION' && entry.data.from !== 'Created' && ( | ||
<p className="text-xs text-muted-foreground"> | ||
<Trans> | ||
Fulfillment #{entry.data.fulfillmentId} from {entry.data.from} to {entry.data.to} | ||
</Trans> | ||
</p> | ||
)} | ||
{entry.type === 'ORDER_FULFILLMENT' && ( | ||
<p className="text-xs text-muted-foreground"> | ||
<Trans> | ||
Fulfillment #{entry.data.fulfillmentId} created | ||
</Trans> | ||
</p> | ||
)} | ||
{entry.type === 'ORDER_MODIFIED' && ( | ||
<p className="text-xs text-muted-foreground"> | ||
<Trans> | ||
Order modification #{entry.data.modificationId} | ||
</Trans> | ||
</p> | ||
)} | ||
{entry.type === 'ORDER_CUSTOMER_UPDATED' && ( | ||
<p className="text-xs text-muted-foreground"> | ||
<Trans> | ||
Customer information updated | ||
</Trans> | ||
</p> | ||
)} | ||
{entry.type === 'ORDER_CANCELLATION' && ( | ||
<p className="text-xs text-muted-foreground"> | ||
<Trans> | ||
Order cancelled | ||
</Trans> | ||
</p> | ||
)} |
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
Significant code duplication in event detail rendering
The rendering logic for event details (ORDER_STATE_TRANSITION, ORDER_PAYMENT_TRANSITION, etc.) is duplicated between primary and secondary events. This violates the DRY principle and makes maintenance harder.
Extract the common rendering logic into a separate component or function:
+const renderEventDetails = (entry: HistoryEntryItem) => {
+ switch (entry.type) {
+ case 'ORDER_STATE_TRANSITION':
+ if (entry.data.from !== 'Created') {
+ return (
+ <p className="text-xs text-muted-foreground">
+ <Trans>From {entry.data.from} to {entry.data.to}</Trans>
+ </p>
+ );
+ }
+ return null;
+ case 'ORDER_PAYMENT_TRANSITION':
+ return (
+ <p className="text-xs text-muted-foreground">
+ <Trans>Payment #{entry.data.paymentId} transitioned to {entry.data.to}</Trans>
+ </p>
+ );
+ case 'ORDER_REFUND_TRANSITION':
+ return (
+ <p className="text-xs text-muted-foreground">
+ <Trans>Refund #{entry.data.refundId} transitioned to {entry.data.to}</Trans>
+ </p>
+ );
+ case 'ORDER_FULFILLMENT_TRANSITION':
+ if (entry.data.from !== 'Created') {
+ return (
+ <p className="text-xs text-muted-foreground">
+ <Trans>Fulfillment #{entry.data.fulfillmentId} from {entry.data.from} to {entry.data.to}</Trans>
+ </p>
+ );
+ }
+ return null;
+ case 'ORDER_FULFILLMENT':
+ return (
+ <p className="text-xs text-muted-foreground">
+ <Trans>Fulfillment #{entry.data.fulfillmentId} created</Trans>
+ </p>
+ );
+ case 'ORDER_MODIFIED':
+ return (
+ <p className="text-xs text-muted-foreground">
+ <Trans>Order modification #{entry.data.modificationId}</Trans>
+ </p>
+ );
+ case 'ORDER_CUSTOMER_UPDATED':
+ return (
+ <p className="text-xs text-muted-foreground">
+ <Trans>Customer information updated</Trans>
+ </p>
+ );
+ case 'ORDER_CANCELLATION':
+ return (
+ <p className="text-xs text-muted-foreground">
+ <Trans>Order cancelled</Trans>
+ </p>
+ );
+ default:
+ return null;
+ }
+};
// Then in both primary and secondary rendering sections, replace the duplicated code with:
+{renderEventDetails(entry)}
Also applies to: 366-434
🤖 Prompt for AI Agents
In
packages/dashboard/src/app/routes/_authenticated/_orders/components/order-history/order-history.tsx
around lines 289 to 344, the rendering logic for different event types is
duplicated, violating the DRY principle. To fix this, extract the repeated JSX
for event detail rendering into a reusable component or function that takes the
event entry as a prop or argument and returns the appropriate JSX based on the
event type. Then replace the duplicated code blocks with calls to this new
component or function. Apply the same refactoring to the similar duplicated code
between lines 366 to 434.
const pageBlock = usePageBlock(); | ||
const blockId = pageBlock?.blockId; |
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.
Use optional flag for consistent error handling.
The code uses optional chaining for blockId
, suggesting pageBlock
might be undefined. However, usePageBlock()
will throw an error if the context is not found. Use the optional flag for consistency.
- const pageBlock = usePageBlock();
+ const pageBlock = usePageBlock({ optional: true });
📝 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 pageBlock = usePageBlock(); | |
const blockId = pageBlock?.blockId; | |
const pageBlock = usePageBlock({ optional: true }); | |
const blockId = pageBlock?.blockId; |
🤖 Prompt for AI Agents
In packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
around lines 38 to 39, the code uses optional chaining to access blockId from
pageBlock, but usePageBlock() throws an error if the context is missing. To
handle this consistently, modify the call to usePageBlock() by passing the
optional flag so it returns undefined instead of throwing, then keep the
optional chaining for blockId access.
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
Outdated
Show resolved
Hide resolved
packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/dashboard/src/lib/components/data-table/data-table-bulk-actions.tsx
(4 hunks)packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
(3 hunks)packages/dashboard/src/lib/hooks/use-floating-bulk-actions.ts
(1 hunks)packages/dashboard/src/lib/index.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- packages/dashboard/src/lib/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/dashboard/src/lib/components/data-table/data-table-bulk-actions.tsx
- packages/dashboard/src/lib/components/shared/asset/asset-bulk-actions.tsx
🧰 Additional context used
📓 Path-based instructions (2)
packages/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{ts,tsx}
: Use TanStack Query (useQuery or useMutation) for data fetching; do not use Apollo Client.
When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
When performing mutations, follow the provided useMutation pattern: use graphql() for the mutation document, api.mutate for mutationFn, and do not pass variables at declaration.
Files:
packages/dashboard/src/lib/hooks/use-floating-bulk-actions.ts
packages/dashboard/src/**/*.{tsx,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
React component props objects should be typed as Readonly<...>.
Files:
packages/dashboard/src/lib/hooks/use-floating-bulk-actions.ts
🧠 Learnings (1)
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : use react for all ui components in the dashboard ...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : Use React for all UI components in the dashboard package.
Applied to files:
packages/dashboard/src/lib/hooks/use-floating-bulk-actions.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: e2e tests (24.x, mariadb)
- GitHub Check: e2e tests (24.x, mysql)
- GitHub Check: e2e tests (22.x, postgres)
- GitHub Check: e2e tests (22.x, sqljs)
- GitHub Check: e2e tests (20.x, sqljs)
- GitHub Check: e2e tests (22.x, mysql)
- GitHub Check: e2e tests (20.x, postgres)
- GitHub Check: e2e tests (20.x, mysql)
- GitHub Check: unit tests (20.x)
- GitHub Check: build (20.x)
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: unit tests (24.x)
- GitHub Check: build (24.x)
- GitHub Check: unit tests (22.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
packages/dashboard/src/lib/hooks/use-floating-bulk-actions.ts (4)
1-13
: LGTM!The imports and TypeScript interface definitions are well-structured and appropriate for the hook's functionality.
15-24
: LGTM!The function signature properly follows React hook conventions, includes clear documentation, and correctly uses
Readonly<...>
typing for the props object as required by the coding guidelines.
25-26
: LGTM!The state initialization uses appropriate default values and follows React hook patterns correctly.
77-82
: LGTM!The return statement provides a clean and intuitive API with all necessary positioning data and a convenient
shouldShow
computed property.
useEffect(() => { | ||
if (selectionCount === 0) { | ||
setIsPositioned(false); | ||
return; | ||
} | ||
|
||
const updatePosition = () => { | ||
const container = document.querySelector(containerSelector)?.closest('div') as HTMLElement; | ||
if (!container) return; | ||
|
||
const containerRect = container.getBoundingClientRect(); | ||
const viewportHeight = window.innerHeight; | ||
|
||
// Check if container bottom is visible in viewport | ||
const containerBottom = containerRect.bottom; | ||
const isContainerFullyVisible = containerBottom <= viewportHeight - bufferDistance; | ||
|
||
// Calculate horizontal center | ||
const containerLeft = containerRect.left; | ||
const containerWidth = containerRect.width; | ||
const centerX = containerLeft + containerWidth / 2; | ||
|
||
if (isContainerFullyVisible) { | ||
// Position relative to container bottom | ||
setPosition({ | ||
bottom: `${viewportHeight - containerBottom + bottomOffset}px`, | ||
left: `${centerX}px`, | ||
}); | ||
} else { | ||
// Position relative to viewport bottom, centered in container | ||
setPosition({ | ||
bottom: '2.5rem', | ||
left: `${centerX}px`, | ||
}); | ||
} | ||
|
||
setIsPositioned(true); | ||
}; | ||
|
||
updatePosition(); | ||
window.addEventListener('scroll', updatePosition); | ||
window.addEventListener('resize', updatePosition); | ||
|
||
return () => { | ||
window.removeEventListener('scroll', updatePosition); | ||
window.removeEventListener('resize', updatePosition); | ||
}; | ||
}, [selectionCount, containerSelector, bufferDistance]); |
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 missing dependency and consider performance optimizations.
The positioning logic is well-implemented, but there are a few issues to address:
- Missing dependency:
bottomOffset
should be included in the dependency array - Brittle DOM querying: The
.closest('div')
assumption may be fragile - Performance concern: Scroll/resize listeners fire frequently without throttling
Apply this diff to fix the missing dependency:
- }, [selectionCount, containerSelector, bufferDistance]);
+ }, [selectionCount, containerSelector, bufferDistance, bottomOffset]);
Consider these improvements:
1. More robust container selection:
- const container = document.querySelector(containerSelector)?.closest('div') as HTMLElement;
+ const element = document.querySelector(containerSelector);
+ const container = element?.closest('[data-container], .container, div') as HTMLElement;
2. Add throttling for better performance:
// Add throttle utility at the top
const throttle = (func: Function, delay: number) => {
let timeoutId: number;
let lastExecTime = 0;
return (...args: any[]) => {
const currentTime = Date.now();
if (currentTime - lastExecTime > delay) {
func(...args);
lastExecTime = currentTime;
} else {
clearTimeout(timeoutId);
timeoutId = setTimeout(() => func(...args), delay - (currentTime - lastExecTime));
}
};
};
// Then use throttled version
const throttledUpdatePosition = throttle(updatePosition, 16); // ~60fps
window.addEventListener('scroll', throttledUpdatePosition);
window.addEventListener('resize', throttledUpdatePosition);
🤖 Prompt for AI Agents
In packages/dashboard/src/lib/hooks/use-floating-bulk-actions.ts around lines 28
to 75, add bottomOffset to the useEffect dependency array to ensure updates when
it changes. Replace the brittle container selection using closest('div') with a
more robust query or direct containerSelector usage to avoid incorrect element
targeting. Implement a throttle utility function to wrap updatePosition and use
the throttled version for scroll and resize event listeners to reduce
performance overhead from frequent firing.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx (2)
12-21
: Consider memoizing the helper function for performance.The helper function works correctly, but consider wrapping it with
useCallback
to prevent unnecessary re-creation on each render.+import { useCallback } from 'react'; - const formatFullDateTime = (dateString: string) => { + const formatFullDateTime = useCallback((dateString: string) => { return formatDate(dateString, { year: 'numeric', month: 'long', day: 'numeric', hour: 'numeric', minute: 'numeric', second: 'numeric', }); - }; + }, [formatDate]);
23-36
: Consider using semantic HTML for better accessibility.The tooltip implementation is correct, but consider using a
<time>
element instead of a<div>
for better semantic HTML.<TooltipTrigger asChild> - <div className={className}> + <time dateTime={date} className={className}> {formatRelativeDate(date)} - </div> + </time> </TooltipTrigger>This provides better semantic meaning and accessibility for screen readers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
packages/dashboard/src/**/*.{tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{tsx,jsx}
: Use React for all UI components in the dashboard package.
Prefer re-using components from /src/lib/components, especially Shadcn components from /src/lib/components/ui.
Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
All labels or user-facing messages should use localization via the Trans component or useLingui().
Files:
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
packages/dashboard/src/lib/components/**/*.{tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
Use Shadcn UI and Tailwind CSS for UI components.
Files:
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
packages/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{ts,tsx}
: Use TanStack Query (useQuery or useMutation) for data fetching; do not use Apollo Client.
When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
When performing mutations, follow the provided useMutation pattern: use graphql() for the mutation document, api.mutate for mutationFn, and do not pass variables at declaration.
Files:
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
packages/dashboard/src/**/*.{tsx,ts}
📄 CodeRabbit Inference Engine (.cursor/rules/dashboard.mdc)
React component props objects should be typed as Readonly<...>.
Files:
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
🧠 Learnings (6)
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : use react for all ui components in the dashboard ...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : Use React for all UI components in the dashboard package.
Applied to files:
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
📚 Learning: applies to packages/dashboard/src/lib/components/**/*.{tsx,jsx} : use shadcn ui and tailwind css for...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/lib/components/**/*.{tsx,jsx} : Use Shadcn UI and Tailwind CSS for UI components.
Applied to files:
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : prefer re-using components from /src/lib/componen...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : Prefer re-using components from /src/lib/components, especially Shadcn components from /src/lib/components/ui.
Applied to files:
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : all labels or user-facing messages should use loc...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : All labels or user-facing messages should use localization via the Trans component or useLingui().
Applied to files:
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,ts} : react component props objects should be typed as r...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,ts} : React component props objects should be typed as Readonly<...>.
Applied to files:
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
📚 Learning: applies to packages/dashboard/src/**/*.{tsx,jsx} : prefer using the formfieldwrapper component for f...
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{tsx,jsx} : Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
Applied to files:
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx
🧬 Code Graph Analysis (1)
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx (1)
packages/dashboard/src/lib/hooks/use-local-format.ts (1)
useLocalFormat
(26-161)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/dashboard/src/lib/components/shared/history-timeline/history-entry-date.tsx (2)
1-7
: LGTM! Clean imports and interface definition.The imports follow the coding guidelines by using Shadcn UI components, and the props interface correctly uses
Readonly<>
as required for React component props.
9-10
: LGTM! Proper component definition and hook usage.The component correctly uses
Readonly<>
for props typing and efficiently destructures only the needed functions from theuseLocalFormat
hook.
Various ui fixes
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor