Skip to content

offers-page-list-view-switch#906

Merged
BilalG1 merged 2 commits intodevfrom
offers-page-list-view-switch
Sep 20, 2025
Merged

offers-page-list-view-switch#906
BilalG1 merged 2 commits intodevfrom
offers-page-list-view-switch

Conversation

@BilalG1
Copy link
Copy Markdown
Collaborator

@BilalG1 BilalG1 commented Sep 20, 2025

High-level PR Summary

This PR adds a toggle switch feature for the payments offers page, allowing users to switch between two different view modes: a catalog view (card-based layout) and a list view. The original code from page-client.tsx has been split into two separate files - page-client-catalogs-view.tsx for the existing card-based catalog view and page-client-list-view.tsx for the new list-based view. The main page-client.tsx file now acts as a simple router between these two views, maintaining a view state and rendering the appropriate component based on the selected view. Both view components include a toggle switch in their header to allow users to switch between views.

⏱️ Estimated Review Time: 15-30 minutes

💡 Review Order Suggestion
Order File Path
1 apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
2 apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx
3 apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx

Review by RecurseML

🔍 Review performed on 872174d..a5ad8d9

✨ No bugs found, your code is sparkling clean

✅ Files analyzed, no issues (3)

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx

Need help? Join our Discord

Summary by CodeRabbit

  • New Features

    • Added revamped Payments Offers management with two views: List and Catalogs.
    • Grouping and sorting of offers by customer type, addons, and price.
    • Inline create/edit/duplicate/delete for offers, prices, and items with dialogs and validations.
    • Search, filters, and hover-based highlighting with visual connections between offers and items.
    • Responsive layout (desktop and mobile), tooltips, and helpful empty-state onboarding.
  • Refactor

    • Simplified page into dedicated List and Catalogs views with seamless switching for a clearer, faster experience.

@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 20, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
stack-backend Ready Ready Preview Comment Sep 20, 2025 6:25pm
stack-dashboard Ready Ready Preview Comment Sep 20, 2025 6:25pm
stack-demo Ready Ready Preview Comment Sep 20, 2025 6:25pm
stack-docs Ready Ready Preview Comment Sep 20, 2025 6:25pm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 20, 2025

Walkthrough

Adds two new client pages for payments offers: a catalogs view and a list view. Refactors the main PageClient to switch between these views based on local state. Implements rich inline editing, grouping, sorting, dialogs, and onboarding states within the new views.

Changes

Cohort / File(s) Summary
Catalogs View (new)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx
Introduces a client-side catalogs view with grouped catalogs, tab-like switching, inline editing of offers/prices/items, cloning, validations, dialogs, and empty state handling. Exports default PageClient({ onViewChange }).
List View (new)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx
Adds a two-pane offers/items list with grouping, search, connectors, dialogs for create/update, onboarding screen, and responsive layout. Exports default PageClient({ onViewChange }).
Page Router/Switch
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
Simplifies to delegate rendering to the new Catalogs or List view based on internal state; removes inlined UI logic. Public export unchanged.

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant PageClient
  participant CatalogsView as PageClientCatalogsView
  participant ListView as PageClientListView

  Note over PageClient: Initialize with local view state ("catalogs" | "list")
  User->>PageClient: Load Payments Offers page
  alt view == "catalogs"
    PageClient->>CatalogsView: Render with onViewChange
    User->>CatalogsView: Interact (edit/clone/validate)
    CatalogsView-->>PageClient: onViewChange("list") [when toggled]
  else view == "list"
    PageClient->>ListView: Render with onViewChange
    User->>ListView: Interact (search/edit/create)
    ListView-->>PageClient: onViewChange("catalogs") [when toggled]
  end
Loading
sequenceDiagram
  actor User
  participant CatalogsView
  participant ConfigStore as Config Store (payments.\*)
  participant Dialogs as Offer/Item Dialogs
  participant Toasts as Toast UI

  User->>CatalogsView: Start editing offer/item
  CatalogsView->>Dialogs: Open dialog (create/edit/duplicate)
  Dialogs-->>CatalogsView: Draft changes
  CatalogsView->>ConfigStore: Save changes
  alt valid
    ConfigStore-->>CatalogsView: Persisted
    CatalogsView->>Toasts: Show success
  else invalid
    CatalogsView-->>User: Inline validation errors
    CatalogsView->>Toasts: Show error
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Offer catalog UI #904 — Splits the payments offers PageClient into catalog and list views with similar routing and UI decomposition, likely part of the same refactor series.

Poem

A rabbit taps the dashboard glass,
Two views bloom where one would pass.
Lists to browse, catalogs to tend,
Offers dance, and items blend.
Click, save, toast—then hop with glee,
Refactors set the UI free. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.35% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "offers-page-list-view-switch" accurately reflects the primary change in the diff — adding a switch between list and catalog views on the offers page — and is therefore on-topic and related to the main changeset, though it is formatted as a slug rather than a natural-language sentence.
Description Check ✅ Passed The PR description includes a clear high-level summary, estimated review time, suggested review order, and an automated analysis block that explains the refactor and the added toggle behavior, which satisfies the repository's minimal template and gives reviewers sufficient context about intent and scope.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch offers-page-list-view-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR implements a significant architectural refactoring for the payments offers page, introducing a dual-view system that allows users to switch between 'catalogs' and 'list' views for managing payment offers and items.

The main change extracts all complex offer management logic (~1572 lines) from the original page-client.tsx file and splits it into two specialized components:

  • PageClientCatalogsView: A comprehensive catalog view featuring card-based interfaces with inline editing, drag-and-drop interactions, price management with intervals, and visual grouping. This component maintains the original visual approach with interactive offer cards (~400 lines each) that support creation, editing, duplication, and deletion.

  • PageClientListView: A new compact list-based interface optimized for scalability and power users. It features search functionality, sorting by customer type priority, grouped displays, and innovative SVG connection lines that visually link related offers and items during hover interactions. The component adapts between desktop two-column layout and mobile tabbed layout.

The refactored main component now serves as a simple view router with a state-managed switcher (useState<"list" | "catalogs">) that conditionally renders either view based on user preference. Both views receive an onViewChange callback prop to enable seamless switching.

This architectural change addresses code maintainability by separating concerns while providing users with multiple interaction paradigms for the same underlying payment data. The catalog view caters to visual learners who prefer card-based interactions, while the list view serves users who need to efficiently navigate large datasets with search, filtering, and compact representation.

Confidence score: 3/5

  • This refactoring involves significant code reorganization with potential for state management inconsistencies between extracted components
  • Large components (~400-250 lines) with complex state handling increase risk of subtle bugs during the extraction process
  • Complex nested state updates and prototype property checking in multiple locations could introduce edge cases
  • Pay close attention to the state management logic in both view components, particularly the object manipulation patterns and the SVG connection line calculations in the list view

3 files reviewed, no comments

Edit Code Review Bot Settings | Greptile

actionItems
}: ListItemProps) {
const itemRefBackup = useRef<HTMLDivElement>(null);
itemRef ??= itemRefBackup;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Avoid mutating the incoming prop 'itemRef' with the nullish assignment (line 94). Instead, derive a local ref variable (e.g. 'const finalRef = itemRef || itemRefBackup') so that props remain immutable.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (10)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx (5)

9-11: Import ActionDialog for destructive confirmations

You use native confirm() below; per design system, prefer ActionDialog. Bring it into this module.

-import { Button, Card, CardContent, DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuSeparator, DropdownMenuTrigger, Switch, Label, toast } from "@stackframe/stack-ui";
+import { ActionDialog, Button, Card, CardContent, DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuSeparator, DropdownMenuTrigger, Switch, Label, toast } from "@stackframe/stack-ui";

612-626: Use Map for refs instead of Record for key–value collections

Guideline prefers ES6 Map; it also avoids accidental prototype keys and makes presence checks explicit. You can still pass refs by id via map.get(id).

Example:

-  const offerRefs = useMemo(() => {
-    const refs = Object.fromEntries(
-      Object.keys(paymentsConfig.offers)
-        .map(id => [id, React.createRef<HTMLDivElement>()])
-    );
-    return refs;
-  }, [paymentsConfig.offers]);
+  const offerRefs = useMemo(() => {
+    const m = new Map<string, React.RefObject<HTMLDivElement>>();
+    for (const id of Object.keys(paymentsConfig.offers)) m.set(id, React.createRef<HTMLDivElement>());
+    return m;
+  }, [paymentsConfig.offers]);

Then use offerRefs.get(id) in usages, e.g., for ConnectionLine and ListItem refs.

Also applies to: 620-626, 856-867, 869-879


381-393: Remove unused zebra prop (isEven)

isEven is calculated but unused in ListItem styling. Drop the prop and the counter to reduce noise.


816-821: Prefer overflow-auto over overflow-scroll

Avoid always-visible scrollbars; use auto to show only when needed.

-        < div className="flex gap-6 flex-1" style={{
-          flexBasis: "0px",
-          overflow: "scroll",
-        }
-        }>
+        <div className="flex gap-6 flex-1" style={{ flexBasis: "0px", overflow: "auto" }}>

186-223: ConnectionLine: update on font-size/layout changes

You listen to resize/scroll; layout shifts (e.g., fonts/theme) won’t trigger updates. Consider ResizeObserver on container and from/to nodes.

High-level:

  • Attach a ResizeObserver to containerRef.current and both endpoints to call updatePath.
  • Disconnect in cleanup.
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (1)

8-13: Persist selected view across reloads

Minor UX: remember the last selected view (localStorage or query param).

Example:

-  const [view, setView] = useState<"list" | "catalogs">("catalogs");
+  const [view, setView] = useState<"list" | "catalogs">(
+    () => (localStorage.getItem("offers:view") as any) || "catalogs"
+  );
+  const setViewPersist = (v: "list" | "catalogs") => {
+    localStorage.setItem("offers:view", v);
+    setView(v);
+  };
@@
-    return <PageClientCatalogsView onViewChange={setView} />;
+    return <PageClientCatalogsView onViewChange={setViewPersist} />;
@@
-  return <PageClientListView onViewChange={setView} />;
+  return <PageClientListView onViewChange={setViewPersist} />;
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx (4)

454-471: Avoid unreachable onClick + toast on disabled option; show a tooltip instead

The button is disabled when an item is already included; onClick won’t fire. Replace the toast branch with a tooltip to explain why it’s disabled.

-                  const isSelected = opt.id === itemId;
-                  const isUsed = existingIncludedItemIds.includes(opt.id) && !isSelected;
-                  return (
-                    <Button
+                  const isSelected = opt.id === itemId;
+                  const isUsed = existingIncludedItemIds.includes(opt.id) && !isSelected;
+                  return (
+                    <SimpleTooltip tooltip={isUsed ? "Item already included" : undefined}>
+                    <Button
                       key={opt.id}
                       variant={isSelected ? 'secondary' : 'ghost'}
                       size="sm"
                       className="justify-start"
                       disabled={isUsed}
                       onClick={() => {
                         if (isSelected) {
                           setItemSelectOpen(false);
                           return;
                         }
-                        if (isUsed) {
-                          toast({ title: 'Item already included' });
-                          return;
-                        }
                         onChangeItemId(opt.id);
                         setItemSelectOpen(false);
                       }}
                     >
                       <div className="flex flex-col items-start">
                         <span>{opt.displayName || opt.id}</span>
                         <span className="text-xs text-muted-foreground">{opt.customerType.toUpperCase()} • {opt.id}</span>
                       </div>
                     </Button>
+                    </SimpleTooltip>
                   );

1514-1520: Typo/microcopy: title should be plural

The page renders multiple offers; prefer “Offers”.

-      title='Offer'
+      title='Offers'

1472-1508: Add basic error handling around updateConfig

If these writes fail, the UI silently proceeds. Wrap in try/catch and surface a blocking alert.

Pattern:

try {
  await project.updateConfig(...);
  toast({ title: "Offer updated" }); // success toast is fine
} catch (e) {
  // Use ActionDialog to inform failure
}

Also applies to: 1500-1504, 1505-1508


63-70: IntervalPopover UX: allow typing decimals and guard zero count

Edge cases:

  • Prevent zero interval count when interval is required.
  • Consider allowing decimal currency input separately; interval count should remain integer.

Would you like me to patch count min=1 and enforce integer-only with input attributes?

Also applies to: 86-107, 200-241

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 872174d and 49ecde2.

📒 Files selected for processing (3)
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx (1 hunks)
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx (1 hunks)
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

For blocking alerts and errors in UI, do not use toast notifications; use alerts instead

Files:

  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Prefer ES6 Map over Record when representing key–value collections

Files:

  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
{apps/dashboard,apps/dev-launchpad,packages/stack-ui,packages/react}/**/*.{tsx,jsx,css}

📄 CodeRabbit inference engine (AGENTS.md)

Keep hover/click animations snappy; avoid pre-transition delays on hover and apply transitions after the action (e.g., fade-out on hover end)

Files:

  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx
  • apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx
🧬 Code graph analysis (3)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx (8)
packages/stack-shared/src/utils/dates.tsx (1)
  • DayInterval (146-146)
packages/stack-shared/src/utils/numbers.tsx (1)
  • prettyPrintWithMagnitudes (9-26)
apps/dashboard/src/components/code-block.tsx (1)
  • CodeBlock (18-75)
apps/dashboard/src/components/editable-input.tsx (1)
  • EditableInput (20-145)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
  • useAdminApp (27-34)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/dummy-data.tsx (1)
  • DUMMY_PAYMENTS_CONFIG (2-278)
packages/stack-shared/src/utils/objects.tsx (1)
  • typedEntries (263-265)
packages/stack-shared/src/utils/strings.tsx (1)
  • stringCompare (61-65)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx (9)
packages/stack-shared/src/config/schema.ts (1)
  • CompleteConfig (993-993)
packages/stack-shared/src/hooks/use-hover.tsx (1)
  • useHover (4-88)
packages/stack-shared/src/utils/numbers.tsx (1)
  • prettyPrintWithMagnitudes (9-26)
packages/stack-shared/src/utils/dates.tsx (1)
  • DayInterval (146-146)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/use-admin-app.tsx (1)
  • useAdminApp (27-34)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/list-section.tsx (1)
  • ListSection (19-90)
packages/stack-shared/src/utils/strings.tsx (1)
  • stringCompare (61-65)
apps/dashboard/src/components/illustrated-info.tsx (1)
  • IllustratedInfo (4-31)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/dummy-data.tsx (1)
  • DUMMY_PAYMENTS_CONFIG (2-278)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client.tsx (2)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx (1)
  • PageClient (1363-1583)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx (1)
  • PageClient (592-950)
⏰ 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). (9)
  • GitHub Check: all-good
  • GitHub Check: docker
  • GitHub Check: lint_and_build (latest)
  • GitHub Check: restart-dev-and-test
  • GitHub Check: build (22.x)
  • GitHub Check: setup-tests
  • GitHub Check: build (22.x)
  • GitHub Check: docker
  • GitHub Check: Security Check

Comment on lines +878 to +885
<DropdownMenuContent align="end" className="min-w-[160px]">
<DropdownMenuItem onClick={() => {
setIsEditing(true);
setDraft(offer);
}}>
Edit
</DropdownMenuItem>
<DropdownMenuItem onClick={() => { onDuplicate(offer); }}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use alert-style modal for blocking errors (no toasts for errors)

“Price ID already exists” is an error that blocks progress; replace toast with ActionDialog (or inline error).

-                    if (Object.prototype.hasOwnProperty.call(nextPrices, newId)) {
-                      toast({ title: "Price ID already exists" });
-                      return prev; // Do not change state
-                    }
+                    if (Object.prototype.hasOwnProperty.call(nextPrices, newId)) {
+                      setDuplicatePriceId(newId);
+                      setShowDuplicatePriceDialog(true);
+                      return prev; // Do not change state
+                    }

Add state near other OfferCard state:

+  const [showDuplicatePriceDialog, setShowDuplicatePriceDialog] = useState(false);
+  const [duplicatePriceId, setDuplicatePriceId] = useState<string | null>(null);

Render dialog at end of OfferCard:

<ActionDialog
  open={showDuplicatePriceDialog}
  onOpenChange={setShowDuplicatePriceDialog}
  title="Duplicate price ID"
  okButton={{ label: "OK", onClick: () => setShowDuplicatePriceDialog(false) }}
>
  A price with ID “{duplicatePriceId}” already exists in this offer.
</ActionDialog>

Also applies to: 719-723

🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx
around lines 878-885 (and also apply same change for 719-723), the current
duplicate-price error uses a toast but should be a blocking alert-style modal;
add local OfferCard state like showDuplicatePriceDialog:boolean and
duplicatePriceId:string near the other OfferCard state, change the onDuplicate
handler to set duplicatePriceId and set showDuplicatePriceDialog(true) instead
of invoking a toast, and render an ActionDialog at the end of the OfferCard
component wired to open={showDuplicatePriceDialog}
onOpenChange={setShowDuplicatePriceDialog} with the title "Duplicate price ID",
an OK button that closes the dialog, and body text "A price with ID
“{duplicatePriceId}” already exists in this offer."

Comment on lines +1196 to +1207
const id = newGroupId.trim();
if (!id) {
alert("Catalog ID is required");
return;
}
if (!/^[a-z0-9-]+$/.test(id)) {
alert("Catalog ID must be lowercase letters, numbers, and hyphens");
return;
}
if (Object.prototype.hasOwnProperty.call(groups, id)) {
alert("Catalog ID already exists");
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Replace native alert() with ActionDialog for validation errors

Use design-system alerts for blocking validation (ID required/format/exists) instead of alert().

Suggestion (pattern):

  • Track a validationError: string | null state.
  • Set it instead of calling alert().
  • Render <ActionDialog open={!!validationError} ...> with the message and single OK button.

This keeps interactions consistent with delete dialogs.

Also applies to: 1209-1214

🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx
around lines 1196-1207 (and similarly 1209-1214), replace the native alert()
calls used for validation with a component-driven dialog: add a validationError:
string | null state, set validationError to the appropriate message instead of
calling alert(), and render an <ActionDialog open={!!validationError}
title="Validation error" description={validationError} onClose={() =>
setValidationError(null)} primaryAction={{ label: "OK", action: () =>
setValidationError(null) }} to show the message and dismiss it; ensure existing
control flow returns after setting the state so behavior matches the original
early-return logic.

Comment on lines +1376 to +1455
// Group offers by groupId and sort by customer type priority
const groupedOffers = useMemo(() => {
const groups = new Map<string | undefined, Array<{ id: string, offer: Offer }>>();

// Group offers
for (const [id, offer] of typedEntries(paymentsConfig.offers)) {
const groupId = offer.groupId;
if (!groups.has(groupId)) {
groups.set(groupId, []);
}
groups.get(groupId)!.push({ id, offer });
}

// Sort offers within each group by customer type, then by ID
const customerTypePriority = { user: 1, team: 2, custom: 3 };
groups.forEach((offers) => {
offers.sort((a, b) => {
const priorityA = customerTypePriority[a.offer.customerType as keyof typeof customerTypePriority] || 4;
const priorityB = customerTypePriority[b.offer.customerType as keyof typeof customerTypePriority] || 4;
if (priorityA !== priorityB) {
return priorityA - priorityB;
}
// If same customer type, sort addons last
if (a.offer.isAddOnTo !== b.offer.isAddOnTo) {
return a.offer.isAddOnTo ? 1 : -1;
}
// If same customer type and addons, sort by lowest price
const getPricePriority = (offer: Offer) => {
if (offer.prices === 'include-by-default') return 0;
if (typeof offer.prices !== 'object') return 0;
return Math.min(...Object.values(offer.prices).map(price => +(price.USD ?? Infinity)));
};
const priceA = getPricePriority(a.offer);
const priceB = getPricePriority(b.offer);
if (priceA !== priceB) {
return priceA - priceB;
}
// Otherwise, sort by ID
return stringCompare(a.id, b.id);
});
});

// Sort groups by their predominant customer type
const sortedGroups = new Map<string | undefined, Array<{ id: string, offer: Offer }>>();

// Helper to get group priority
const getGroupPriority = (groupId: string | undefined) => {
if (!groupId) return 999; // Ungrouped always last

const offers = groups.get(groupId) || [];
if (offers.length === 0) return 999;

// Get the most common customer type in the group
const typeCounts = offers.reduce((acc, { offer }) => {
const type = offer.customerType;
acc[type] = (acc[type] || 0) + 1;
return acc;
}, {} as Record<string, number>);

// Find predominant type
const predominantType = Object.entries(typeCounts)
.sort(([, a], [, b]) => b - a)[0]?.[0];

return customerTypePriority[predominantType as keyof typeof customerTypePriority] || 4;
};

// Sort group entries
const sortedEntries = Array.from(groups.entries()).sort(([aId], [bId]) => {
const priorityA = getGroupPriority(aId);
const priorityB = getGroupPriority(bId);
return priorityA - priorityB;
});

// Rebuild map in sorted order
sortedEntries.forEach(([groupId, offers]) => {
sortedGroups.set(groupId, offers);
});

return sortedGroups;
}, [paymentsConfig]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Deduplicate grouping/sorting logic across views

This grouping/sorting code is duplicated in both list and catalogs views. Extract a shared utility (e.g., getGroupedSortedOffers(paymentsConfig)) to avoid drift.

🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-catalogs-view.tsx
around lines 1376-1455 the grouping/sorting logic is duplicated across list and
catalog views; extract it into a shared utility (e.g.,
src/lib/payments/getGroupedSortedOffers.ts) with a clear typed signature
getGroupedSortedOffers(paymentsConfig: PaymentsConfig): Map<string|undefined,
Array<{ id: string; offer: Offer }>> that encapsulates grouping, per-group offer
sorting (customer type priority, addons last, price then id) and group ordering
by predominant customer type; replace the inline useMemo with a call to
useMemo(() => getGroupedSortedOffers(paymentsConfig), [paymentsConfig]) in both
views, export the function, keep existing types, preserve the price computation
and customerTypePriority constants inside the utility, and update imports and
any tests accordingly.

Comment on lines +98 to +112
return (
<div
ref={itemRef}
className={cn(
"px-3 py-3 cursor-pointer relative duration-200 hover:duration-0 hover:bg-primary/10 transition-colors flex items-center justify-between group",
isHovered && "duration-0",
isHighlighted && "bg-primary/10",
!isMenuHovered && isHovered && "bg-primary/10",
isMenuHovered && isHovered && "bg-primary/5",
isHighlighted && !isMenuHovered && isHovered && "hover:bg-primary/20"
)}
onClick={onClick}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Make ListItem rows keyboard-accessible (div -> interactive semantics)

Clickable divs are not reachable via keyboard. Add role, tabIndex, and Enter/Space handling (or switch to a button).

-  return (
-    <div
+  return (
+    <div
       ref={itemRef}
       className={cn(
         "px-3 py-3 cursor-pointer relative duration-200 hover:duration-0 hover:bg-primary/10 transition-colors flex items-center justify-between group",
         isHovered && "duration-0",
         isHighlighted && "bg-primary/10",
         !isMenuHovered && isHovered && "bg-primary/10",
         isMenuHovered && isHovered && "bg-primary/5",
         isHighlighted && !isMenuHovered && isHovered && "hover:bg-primary/20"
       )}
       onClick={onClick}
+      role="button"
+      tabIndex={0}
+      onKeyDown={(e) => {
+        if (e.key === 'Enter' || e.key === ' ') {
+          e.preventDefault();
+          onClick?.();
+        }
+      }}
       onMouseEnter={onMouseEnter}
       onMouseLeave={onMouseLeave}
     >
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return (
<div
ref={itemRef}
className={cn(
"px-3 py-3 cursor-pointer relative duration-200 hover:duration-0 hover:bg-primary/10 transition-colors flex items-center justify-between group",
isHovered && "duration-0",
isHighlighted && "bg-primary/10",
!isMenuHovered && isHovered && "bg-primary/10",
isMenuHovered && isHovered && "bg-primary/5",
isHighlighted && !isMenuHovered && isHovered && "hover:bg-primary/20"
)}
onClick={onClick}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
>
return (
<div
ref={itemRef}
className={cn(
"px-3 py-3 cursor-pointer relative duration-200 hover:duration-0 hover:bg-primary/10 transition-colors flex items-center justify-between group",
isHovered && "duration-0",
isHighlighted && "bg-primary/10",
!isMenuHovered && isHovered && "bg-primary/10",
isMenuHovered && isHovered && "bg-primary/5",
isHighlighted && !isMenuHovered && isHovered && "hover:bg-primary/20"
)}
onClick={onClick}
role="button"
tabIndex={0}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
onClick?.();
}
}}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
>
🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx
around lines 98 to 112, the ListItem is a clickable div which is not
keyboard-accessible; change it to provide interactive semantics by either
converting the div to a button or adding role="button", tabIndex={0}, and a
keyDown handler that calls the same onClick for Enter and Space keys (prevent
default for Space). Ensure onClick remains for mouse and that any focus/hover
styles still apply.

Comment on lines +318 to +333
function OffersList({
groupedOffers,
paymentsGroups,
hoveredItemId,
getConnectedOffers,
offerRefs,
onOfferMouseEnter,
onOfferMouseLeave,
onOfferAdd,
setEditingOffer,
setShowOfferDialog,
}: OffersListProps) {
const stackAdminApp = useAdminApp();
const project = stackAdminApp.useProject();
const [searchQuery, setSearchQuery] = useState("");
let globalIndex = 0;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Replace native confirm() with ActionDialog for offer deletion

Use the design-system modal instead of window.confirm. This avoids blocking browser dialogs and aligns with the guideline on blocking UI.

 function OffersList({
@@
 }: OffersListProps) {
   const stackAdminApp = useAdminApp();
   const project = stackAdminApp.useProject();
   const [searchQuery, setSearchQuery] = useState("");
+  const [pendingDelete, setPendingDelete] = useState<{ id: string, name: string } | null>(null);
   let globalIndex = 0;
-                      {
-                        item: "Delete",
-                        onClick: async () => {
-                          if (confirm(`Are you sure you want to delete the offer "${offer.displayName}"?`)) {
-                            await project.updateConfig({ [`payments.offers.${id}`]: null });
-                            toast({ title: "Offer deleted" });
-                          }
-                        },
-                        danger: true,
-                      },
+                      {
+                        item: "Delete",
+                        onClick: () => setPendingDelete({ id, name: offer.displayName || id }),
+                        danger: true,
+                      },

Add the dialog near the end of OffersList’s return:

<ActionDialog
  open={!!pendingDelete}
  onOpenChange={(o) => { if (!o) setPendingDelete(null); }}
  title="Delete offer"
  danger
  okButton={{
    label: "Delete",
    onClick: async () => {
      if (!pendingDelete) return;
      try {
        await project.updateConfig({ [`payments.offers.${pendingDelete.id}`]: null });
        toast({ title: "Offer deleted" });
      } finally {
        setPendingDelete(null);
      }
    },
  }}
  cancelButton
>
  Are you sure you want to delete “{pendingDelete?.name}”?
</ActionDialog>

Also applies to: 396-415

🤖 Prompt for AI Agents
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx
lines 318-333 (and similarly update the block at 396-415): replace the native
window.confirm deletion flow with the design-system ActionDialog component; add
a pendingDelete state if not present, render an ActionDialog near the end of
OffersList’s return with open bound to !!pendingDelete, onOpenChange clearing
pendingDelete when closed, a danger-styled title "Delete offer", an okButton
whose onClick asynchronously calls project.updateConfig to remove the offer key
(e.g. set payments.offers.<id> to null), shows a toast on success, and always
clears setPendingDelete(null) in finally, and include cancelButton; remove usage
of window.confirm and instead setPendingDelete(item) to trigger the modal.

Comment on lines +440 to +453
function ItemsList({
items,
hoveredOfferId,
getConnectedItems,
itemRefs,
onItemMouseEnter,
onItemMouseLeave,
onItemAdd,
setEditingItem,
setShowItemDialog,
}: ItemsListProps) {
const stackAdminApp = useAdminApp();
const project = stackAdminApp.useProject();
const [searchQuery, setSearchQuery] = useState("");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Replace native confirm() with ActionDialog for item deletion

Mirror the offer flow for ItemsList.

 function ItemsList({
@@
 }: ItemsListProps) {
   const stackAdminApp = useAdminApp();
   const project = stackAdminApp.useProject();
   const [searchQuery, setSearchQuery] = useState("");
+  const [pendingDelete, setPendingDelete] = useState<{ id: string, name: string } | null>(null);
-                {
-                  item: "Delete",
-                  onClick: async () => {
-                    if (confirm(`Are you sure you want to delete the item "${item.displayName}"?`)) {
-                      await project.updateConfig({ [`payments.items.${id}`]: null });
-                      toast({ title: "Item deleted" });
-                    }
-                  },
-                  danger: true,
-                },
+                {
+                  item: "Delete",
+                  onClick: () => setPendingDelete({ id, name: item.displayName || id }),
+                  danger: true,
+                },

Append dialog:

<ActionDialog
  open={!!pendingDelete}
  onOpenChange={(o) => { if (!o) setPendingDelete(null); }}
  title="Delete item"
  danger
  okButton={{
    label: "Delete",
    onClick: async () => {
      if (!pendingDelete) return;
      try {
        await project.updateConfig({ [`payments.items.${pendingDelete.id}`]: null });
        toast({ title: "Item deleted" });
      } finally {
        setPendingDelete(null);
      }
    },
  }}
  cancelButton
>
  Are you sure you want to delete “{pendingDelete?.name}”?
</ActionDialog>

Also applies to: 520-531

🤖 Prompt for AI Agents
In
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/offers/page-client-list-view.tsx
around lines 440-453 and also 520-531, replace any usage of the native confirm()
deletion flow with the React ActionDialog pattern used by the offers flow: add a
pendingDelete state to hold the item to delete, remove confirm() calls and
instead set pendingDelete when the delete button is clicked, render an
ActionDialog with open={!!pendingDelete}, onOpenChange that clears pendingDelete
when closed, a danger-styled title, an okButton whose onClick calls
project.updateConfig({ [`payments.items.${pendingDelete.id}`]: null }) inside a
try/finally to toast "Item deleted" and clear pendingDelete, and include
cancelButton; ensure dialog body shows the item name via pendingDelete?.name and
update both locations (lines 440-453 and 520-531) to use this pattern.

@BilalG1 BilalG1 merged commit 6ef0cac into dev Sep 20, 2025
28 of 31 checks passed
@BilalG1 BilalG1 deleted the offers-page-list-view-switch branch September 20, 2025 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant