Skip to content

Better Customers Management#535

Merged
softmarshmallow merged 4 commits intomainfrom
canary
Feb 8, 2026
Merged

Better Customers Management#535
softmarshmallow merged 4 commits intomainfrom
canary

Conversation

@softmarshmallow
Copy link
Member

@softmarshmallow softmarshmallow commented Feb 8, 2026

grida-campaign-import-participants-with-tags-filtering.mov
  • Removed the ImportFromCustomersDialog component and replaced it with CustomerPickerDialog for improved customer selection during campaign participation.
  • Updated the referrers-table component to utilize the new CustomerPickerDialog.
  • Refactored the Customers page to integrate the new CustomerTable context and streamline customer data handling.
  • Added a new fetchCustomerIds function to support lightweight customer ID queries.

Summary by CodeRabbit

  • New Features

    • Added a Share drawer with SMS, email, native share and copy actions; integrated consent-based two-step sharing (English/Korean).
    • New Customer picker dialog for importing customers into campaigns with progress and retry UI.
  • Refactor

    • Redesigned customer listing into a modular table/grid with improved selection, filters, toolbar and realtime updates.
    • Streamlined customer ID retrieval and import flows for more reliable operations.

- Removed the ImportFromCustomersDialog component and replaced it with CustomerPickerDialog for improved customer selection during campaign participation.
- Updated the referrers-table component to utilize the new CustomerPickerDialog.
- Refactored the Customers page to integrate the new CustomerTable context and streamline customer data handling.
- Added a new fetchCustomerIds function to support lightweight customer ID queries.
@vercel
Copy link

vercel bot commented Feb 8, 2026

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Feb 8, 2026 3:36pm
grida Ready Ready Preview, Comment Feb 8, 2026 3:36pm
5 Skipped Deployments
Project Deployment Actions Updated (UTC)
code Ignored Ignored Feb 8, 2026 3:36pm
legacy Ignored Ignored Feb 8, 2026 3:36pm
backgrounds Skipped Skipped Feb 8, 2026 3:36pm
blog Skipped Skipped Feb 8, 2026 3:36pm
viewer Skipped Skipped Feb 8, 2026 3:36pm

Request Review

@coderabbitai
Copy link

coderabbitai bot commented Feb 8, 2026

Walkthrough

Replaces the legacy import dialog with a new CustomerPickerDialog integrated with a new CustomerTable provider/grid system, adds a ShareDrawerDialog and changes the share flow to a two-step consent + share flow, and refactors the customers page to use the new CustomerTable APIs.

Changes

Cohort / File(s) Summary
Customer Table / Grid
editor/scaffolds/grid/wellknown/customer-grid.tsx, editor/scaffolds/platform/customer/use-customer-feed.ts
Adds a provider-backed CustomerTable namespace (Provider, Toolbar, SelectionBar, Grid, Footer, etc.), context/hooks for selection/filtering/realtime, header checkbox/selection improvements, and a new fetchCustomerIds utility for ID-only queries.
Customer Import Dialogs
editor/scaffolds/platform/customer/customer-picker-dialog.tsx, editor/scaffolds/platform/customer/import-from-customers-dialog.tsx
Removes the old ImportFromCustomersDialog and introduces CustomerPickerDialog that uses CustomerTable for selection, import progress, success/error states, and exposes onImport(ids) => Promise.
Referrer UI Integration
editor/app/(workbench)/[org]/[proj]/(campaign)/.../_components/referrers-table.tsx
Swaps ImportFromCustomersDialogCustomerPickerDialog, wires onOpenChange to sync open state and trigger refresh on close, and updates imports/usages accordingly.
Customers Page
editor/app/(workbench)/[org]/[proj]/(resources)/customers/page.tsx
Refactors page to use CustomerTable.Provider and useCustomerTable hook plus CustomerTable subcomponents (SelectionBar, Toolbar, Grid, Footer), replacing prior inline grid/state handling and simplifying delete/refresh flows.
Share Dialog & Flow
editor/components/dialogs/share-dialog.tsx, editor/theme/templates/enterprise/west-referral/referrer/share.tsx, editor/theme/templates/enterprise/west-referral/referrer/page.tsx
Adds ShareDrawerDialog and related public types (WebSharePayload, ShareDialogLocale, ShareDrawerAction); refactors referral share flow into consent drawer + prepared ShareDrawerDialog, changes callbacks from onConfirm to async onPrepare returning WebSharePayload, and removes inline navigator.share/clipboard flows.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ReferrerPage as Referrer Page
    participant ConsentDrawer as Consent Drawer
    participant Controller as ReferrerPage (prepare)
    participant ShareDialog as ShareDrawerDialog
    participant ShareAPI as Web Share API / Clipboard

    User->>ReferrerPage: Click Share / Reshare
    ReferrerPage->>ConsentDrawer: open consent drawer
    ConsentDrawer->>User: Display consent + Continue
    User->>ConsentDrawer: Confirm & Continue
    ConsentDrawer->>Controller: onPrepare() (async)
    Controller->>Controller: build/await WebSharePayload
    Controller->>ShareDialog: open with payload
    ShareDialog->>User: Show actions (sms/email/native/copy)
    User->>ShareDialog: Choose action
    ShareDialog->>ShareAPI: navigator.share or copy to clipboard
    ShareAPI-->>User: Share completed / copied
Loading
sequenceDiagram
    participant User
    participant ReferrersTable as Referrers Table
    participant PickerDialog as CustomerPickerDialog
    participant CustomerTable as CustomerTable Grid
    participant Backend as Backend

    User->>ReferrersTable: Click Import Customers
    ReferrersTable->>PickerDialog: open dialog
    PickerDialog->>CustomerTable: render Provider + Grid
    CustomerTable->>Backend: fetch customers (filters/predicates)
    Backend-->>CustomerTable: return rows
    User->>CustomerTable: select rows
    User->>PickerDialog: click Import Selected
    PickerDialog->>Backend: onImport(selectedIds)
    Backend-->>PickerDialog: success / error
    PickerDialog->>User: show progress / success / error
    PickerDialog->>ReferrersTable: close -> trigger refresh
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

chore

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Better Customers Management' is concise and directly reflects the main objective of replacing ImportFromCustomersDialog with CustomerPickerDialog and refactoring customer management workflows.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch canary

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.

- Introduced a new ShareDrawerDialog component to facilitate sharing content via SMS, email, and native sharing options.
- Integrated the ShareDrawerDialog into the referrer page template, replacing the previous sharing logic.
- Updated the ShareDialog component to support a two-step consent and sharing process.
- Refactored related components to improve code organization and maintainability.
@vercel vercel bot temporarily deployed to Preview – backgrounds February 8, 2026 15:18 Inactive
@vercel vercel bot temporarily deployed to Preview – viewer February 8, 2026 15:18 Inactive
@vercel vercel bot temporarily deployed to Preview – blog February 8, 2026 15:18 Inactive
@softmarshmallow softmarshmallow marked this pull request as ready for review February 8, 2026 15:19
@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copy link

@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: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/referrers-table.tsx (1)

299-327: ⚠️ Potential issue | 🟡 Minor

refresh() fires twice after a successful import.

onImport's finally block (line 314) calls refresh(), and onOpenChange (line 325) also calls refresh() when the dialog closes. After a successful import the list will be fetched twice. Consider removing the refresh() from the finally block since the onOpenChange handler already covers it on close.

Proposed fix
   const onImport = async (ids: string[]) => {
     return await fetch(
       `/private/west/campaigns/${campaign.id}/participants/import`,
       {
         method: "POST",
         body: JSON.stringify({
           role: "referrer",
           customer_ids: ids,
         } satisfies Platform.WEST.Referral.ImportParticipantsRequestBody),
       }
     )
-      .then((res) => {
-        return res.ok;
-      })
-      .finally(() => {
-        refresh();
-      });
+      .then((res) => res.ok);
   };
editor/theme/templates/enterprise/west-referral/referrer/page.tsx (1)

31-35: 🛠️ Refactor suggestion | 🟠 Major

Duplicate WebSharePayload type — import from the shared module instead.

This type is identical to the one exported from @/components/dialogs/share-dialog. Duplicating it creates a drift risk if the canonical type is updated later.

Proposed fix
+import type { WebSharePayload } from "@/components/dialogs/share-dialog";
+
-type WebSharePayload = {
-  title?: string;
-  text?: string;
-  url?: string;
-};
🤖 Fix all issues with AI agents
In `@editor/components/dialogs/share-dialog.tsx`:
- Around line 189-207: The anchor rendered inside the Button with asChild (the
SMS button using buildSmsHref and shareText) still receives a disabled prop
which anchors ignore; change the rendering so that when shareText is falsy you
either omit the href and add aria-disabled="true" plus tabIndex={-1} and
pointer-events: none, or render a non-interactive element (e.g., a span) instead
of an <a>; apply the same pattern to the email button(s) to ensure no keyboard
or pointer activation occurs when payloads are missing.
- Around line 176-181: The code renders descriptionHtml via
dangerouslySetInnerHTML in the ShareDialog component without sanitization,
creating an XSS risk; update the component to sanitize descriptionHtml before
injection (or remove the deprecated prop), e.g., add a DOMPurify import and
compute a safeHtml = DOMPurify.sanitize(descriptionHtml) (or guard that only
trusted content is used) and then use dangerouslySetInnerHTML={{ __html:
safeHtml }}; also add DOMPurify to dependencies and ensure any tests or callers
are updated if you opt to remove the prop in favor of the description ReactNode.
- Around line 63-74: The buildMailtoHref function uses
URLSearchParams.toString() which encodes spaces as '+'; change it to manually
construct the mailto query using encodeURIComponent for subject and body so
spaces become '%20' per RFC 6068. Specifically, update buildMailtoHref to avoid
URLSearchParams and return a string like `mailto:?` plus `subject=` and `body=`
pairs where values are encoded with encodeURIComponent (only include the subject
pair when subject is present) and join pairs with '&', ensuring no '+'
characters are produced.
- Around line 168-170: DrawerTitle is using a hardcoded English string; replace
the literal "Share" with the localized token from your i18n hook (e.g., use
t.share) in the ShareDialog component so screen readers receive the translated
label (locate the DrawerTitle element in share-dialog.tsx and swap the string to
{t.share}).

In `@editor/scaffolds/grid/wellknown/customer-grid.tsx`:
- Around line 528-572: The renderHeaderCell callback in selectColumn violates
the Rules of Hooks by calling useRowSelection and useCellRootProps inside the
callback; extract that header UI into a top-level React component (e.g.,
CustomerGridSelectHeader) which accepts props needed from the grid (column,
tabIndex, estimatedCount, selectedRows, onSelectedRowsChange, and a handler to
call onRowSelectionChange) and move the hooks (useRowSelection,
useCellRootProps) to the top of that component; then change
selectColumn.renderHeaderCell to return <CustomerGridSelectHeader ... /> (or a
function that renders it) and wire Checkbox events to call onRowSelectionChange
and onSelectedRowsChange exactly as before.
- Around line 293-307: fetchCustomerIds in use-customer-feed.ts is relying on
Supabase's default 1000-row cap so "Select All" silently truncates results;
update fetchCustomerIds to explicitly override the limit (or implement paging)
so it returns all matching rows—for example add a .limit(...) with a
sufficiently large number or loop with offset/limit (or use cursor pagination)
until all rows are retrieved—then ensure the caller (customer-grid.tsx
handleClick which calls fetchCustomerIds and sets setSelection) receives the
full id list so selections are not truncated.

In `@editor/scaffolds/platform/customer/customer-picker-dialog.tsx`:
- Around line 57-70: The handleImportSelected function currently calls
onImport(ids) without error handling so any thrown rejection prevents
setImporting(false) from running; wrap the await onImport(ids) in a try/catch
and move setImporting(false) into a finally block (or ensure it always runs) so
the importing spinner is cleared, and in the catch setError(...) and call
toast.error(...) with the caught error message; update references to
setImportComplete and setImportCount only on successful import inside the try
block.

In `@editor/scaffolds/platform/customer/use-customer-feed.ts`:
- Around line 87-121: fetchCustomerIds currently issues a select on
client.from("customer_with_tags") without any .limit()/.range(), so PostgREST's
default row cap will silently truncate results; update fetchCustomerIds to
either set an explicit high limit (e.g. call .limit(...) or .range(...) on the
query builder before executing) or implement pagination (loop using offset/range
until no more rows) to ensure all matching UIDs are returned when applying
q_predicates and q_text_search (references: fetchCustomerIds, q_predicates,
q_text_search, Platform.Customer.TABLE_SEARCH_TEXT).

In `@editor/theme/templates/enterprise/west-referral/referrer/share.tsx`:
- Around line 84-89: The template HTML is injected via dangerouslySetInnerHTML
using data?.article?.html which risks XSS if campaign authors can edit
templates; sanitize the HTML client-side before assigning it: import a sanitizer
such as DOMPurify and call DOMPurify.sanitize(data?.article?.html ?? "") and
pass that sanitized string to the element's __html (replace direct use of
data?.article?.html in dangerouslySetInnerHTML). Also, if possible, add a short
guard or fallback for empty/undefined content and document/confirm whether
ingestion/storage already sanitizes templates for defense-in-depth.
🧹 Nitpick comments (6)
editor/scaffolds/platform/customer/customer-picker-dialog.tsx (1)

107-121: Success state has no way to dismiss the dialog.

After a successful import the user sees the confirmation but there's no "Done" / "Close" button. They must click outside or press Escape. Consider adding a close action for discoverability, or auto-closing after a short delay.

editor/scaffolds/grid/wellknown/customer-grid.tsx (1)

171-179: setSelection omitted from useMemo deps — stable for now, but fragile.

setSelection is a React useState setter so it's referentially stable, but omitting it from the dependency array is a minor smell. If this is ever refactored to a non-stable callback, the memo won't re-create.

Proposed fix
   const value = useMemo<CustomerTableContextValue>(
     () => ({
       tablespace,
       selection,
       setSelection,
       clearSelection,
       hasSelection,
     }),
-    [tablespace, selection, clearSelection, hasSelection]
+    [tablespace, selection, setSelection, clearSelection, hasSelection]
   );
editor/scaffolds/platform/customer/use-customer-feed.ts (1)

44-85: Predicate-encoding logic is duplicated between fetchCustomers and fetchCustomerIds.

Both functions encode predicates and apply an optional text-search filter using the same pattern. This is a small amount of code so it's tolerable for now, but if predicate logic grows, extracting a shared helper (e.g., applyFilters(query, predicates, textSearch)) would reduce drift risk.

Also applies to: 92-121

editor/components/dialogs/share-dialog.tsx (2)

87-129: Component doesn't expose a className prop on its outer wrapper.

Per the project's coding guidelines for /editor/components, components should expose className on outer wrappers to allow consumers to restyle.

Proposed fix
 export function ShareDrawerDialog({
+  className,
   open,
   onOpenChange,
   ...
 }: {
+  className?: string;
   open?: boolean;
   ...
 }) {
   ...
   return (
-    <Drawer open={open} onOpenChange={onOpenChange} data-testid={testId}>
+    <Drawer open={open} onOpenChange={onOpenChange} data-testid={testId} className={className}>

As per coding guidelines, "Expose className on outer wrappers and important slots in components to allow consumers to restyle."


158-166: Unhandled navigator.share() rejection leaks to the caller in some paths.

onNativeShareClick doesn't catch internally — it relies on the call-site .catch(() => {}) on line 241. If another call-site forgets that .catch, the promise rejection (e.g. user cancellation AbortError) would be unhandled. Consider adding a try/catch inside the function for defensiveness.

Proposed fix
   const onNativeShareClick = async () => {
     if (!payload) return;
     if (typeof navigator.share !== "function") return;
-    await navigator.share({
-      title: payload.title?.trim(),
-      text: payload.text?.trimEnd(),
-      url: payload.url?.trim(),
-    });
+    try {
+      await navigator.share({
+        title: payload.title?.trim(),
+        text: payload.text?.trimEnd(),
+        url: payload.url?.trim(),
+      });
+    } catch {
+      // User cancelled or share failed — silently ignore.
+    }
   };
editor/theme/templates/enterprise/west-referral/referrer/share.tsx (1)

51-51: Unnecessary type assertion to access open — it's already on the rest props type.

props inherits open from React.ComponentProps<typeof Drawer>, so the cast is redundant.

Proposed fix
-  const consentOpen = (props as { open?: boolean }).open;
+  const consentOpen = props.open;

Comment on lines +63 to +74
function buildMailtoHref({
subject,
body,
}: {
subject?: string;
body: string;
}): string {
const params = new URLSearchParams();
if (subject) params.set("subject", subject);
params.set("body", body);
return `mailto:?${params.toString()}`;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

URLSearchParams encodes spaces as +, which some email clients render literally in mailto: URIs.

RFC 6068 expects percent-encoding (%20) for spaces in mailto bodies. URLSearchParams.toString() produces + for spaces, which can cause literal + signs in the email body on certain clients.

Proposed fix using manual encoding
 function buildMailtoHref({
   subject,
   body,
 }: {
   subject?: string;
   body: string;
 }): string {
-  const params = new URLSearchParams();
-  if (subject) params.set("subject", subject);
-  params.set("body", body);
-  return `mailto:?${params.toString()}`;
+  const parts: string[] = [];
+  if (subject) parts.push(`subject=${encodeURIComponent(subject)}`);
+  parts.push(`body=${encodeURIComponent(body)}`);
+  return `mailto:?${parts.join("&")}`;
 }
🤖 Prompt for AI Agents
In `@editor/components/dialogs/share-dialog.tsx` around lines 63 - 74, The
buildMailtoHref function uses URLSearchParams.toString() which encodes spaces as
'+'; change it to manually construct the mailto query using encodeURIComponent
for subject and body so spaces become '%20' per RFC 6068. Specifically, update
buildMailtoHref to avoid URLSearchParams and return a string like `mailto:?`
plus `subject=` and `body=` pairs where values are encoded with
encodeURIComponent (only include the subject pair when subject is present) and
join pairs with '&', ensuring no '+' characters are produced.

Comment on lines +168 to +170
return (
<Drawer open={open} onOpenChange={onOpenChange} data-testid={testId}>
<DrawerTitle className="sr-only">Share</DrawerTitle>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

DrawerTitle is hardcoded in English instead of using the localized string.

Line 170 uses "Share" directly instead of {t.share}, which bypasses the localization system and will always display English for screen readers.

Proposed fix
-      <DrawerTitle className="sr-only">Share</DrawerTitle>
+      <DrawerTitle className="sr-only">{t.share}</DrawerTitle>
📝 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 (
<Drawer open={open} onOpenChange={onOpenChange} data-testid={testId}>
<DrawerTitle className="sr-only">Share</DrawerTitle>
return (
<Drawer open={open} onOpenChange={onOpenChange} data-testid={testId}>
<DrawerTitle className="sr-only">{t.share}</DrawerTitle>
🤖 Prompt for AI Agents
In `@editor/components/dialogs/share-dialog.tsx` around lines 168 - 170,
DrawerTitle is using a hardcoded English string; replace the literal "Share"
with the localized token from your i18n hook (e.g., use t.share) in the
ShareDialog component so screen readers receive the translated label (locate the
DrawerTitle element in share-dialog.tsx and swap the string to {t.share}).

Comment on lines +189 to +207
{has("sms") && (
<div className="flex flex-col items-center gap-1">
<Button
asChild
type="button"
variant="secondary"
size="icon-lg"
disabled={!shareText}
className="rounded-full"
>
<a href={buildSmsHref(shareText)} aria-label={t.message}>
<MessageSquareText className="size-5" />
</a>
</Button>
<span className="text-xs text-muted-foreground">
{t.message}
</span>
</div>
)}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Disabled <a> elements remain navigable — disabled is not a valid HTML attribute for anchors.

With asChild, the disabled prop is forwarded to the <a> tag, but <a> doesn't natively support disabled. The link may appear visually dimmed via CSS, but it is still keyboard-focusable and activatable, opening the SMS/email app with an empty body.

Consider conditionally rendering the href (or using aria-disabled with pointer-events: none and tabIndex={-1}) to actually prevent navigation when the payload is missing:

Proposed approach (SMS button example, apply same pattern to email)
                     <Button
                       asChild
                       type="button"
                       variant="secondary"
                       size="icon-lg"
                       disabled={!shareText}
                       className="rounded-full"
                     >
                       <a
-                        href={buildSmsHref(shareText)}
+                        href={shareText ? buildSmsHref(shareText) : undefined}
                         aria-label={t.message}
+                        aria-disabled={!shareText || undefined}
+                        tabIndex={shareText ? undefined : -1}
                       >
                         <MessageSquareText className="size-5" />
                       </a>
                     </Button>

Also applies to: 209-233

🤖 Prompt for AI Agents
In `@editor/components/dialogs/share-dialog.tsx` around lines 189 - 207, The
anchor rendered inside the Button with asChild (the SMS button using
buildSmsHref and shareText) still receives a disabled prop which anchors ignore;
change the rendering so that when shareText is falsy you either omit the href
and add aria-disabled="true" plus tabIndex={-1} and pointer-events: none, or
render a non-interactive element (e.g., a span) instead of an <a>; apply the
same pattern to the email button(s) to ensure no keyboard or pointer activation
occurs when payloads are missing.

Comment on lines +293 to +307
const handleClick = async () => {
setLoading(true);
const { data, error } = await fetchCustomerIds(ciamClient, project.id, {
q_predicates: tablespace.predicates,
q_text_search: tablespace.q_text_search,
});
setLoading(false);

if (error || !data) {
console.error("Failed to fetch customer IDs", error);
return;
}

setSelection(new Set(data.map((c) => c.uid)));
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

fd 'use-customer-feed' --type f

Repository: gridaco/grida

Length of output: 112


🏁 Script executed:

rg -n 'fetchCustomerIds' --type ts --type tsx -B 2 -A 15

Repository: gridaco/grida

Length of output: 84


🌐 Web query:

Supabase PostgREST default row limit 2025

💡 Result:

  • PostgREST upstream default: no hard row limit (db-max-rows defaults to ∞). [1]
  • Supabase hosted default (Data API / PostgREST): max 1,000 rows returned per request by default, configurable in Project API Settings (and effectively maps to PostgREST’s pgrst.db_max_rows). [2]

Sources:
[1] PostgREST config docs (db-max-rows default ∞) (docs.postgrest.org)
[2] Supabase docs (“By default… maximum of 1,000 rows… can be changed in Project API Settings”) (supabase.com)

Citations:


🏁 Script executed:

cat -n editor/scaffolds/platform/customer/use-customer-feed.ts

Repository: gridaco/grida

Length of output: 6102


fetchCustomerIds silently truncates results to Supabase's default 1000-row limit.

The function (lines 92-121 in use-customer-feed.ts) intentionally omits pagination to "return all matching rows" for the "Select All" feature, but lacks any explicit row limit override. When Supabase's default 1000-row limit applies, datasets with >1000 matching customers will silently truncate, causing incorrect selection in the import flow. Add an explicit .limit() or configure a higher row limit in Project API Settings.

🤖 Prompt for AI Agents
In `@editor/scaffolds/grid/wellknown/customer-grid.tsx` around lines 293 - 307,
fetchCustomerIds in use-customer-feed.ts is relying on Supabase's default
1000-row cap so "Select All" silently truncates results; update fetchCustomerIds
to explicitly override the limit (or implement paging) so it returns all
matching rows—for example add a .limit(...) with a sufficiently large number or
loop with offset/limit (or use cursor pagination) until all rows are
retrieved—then ensure the caller (customer-grid.tsx handleClick which calls
fetchCustomerIds and sets setSelection) receives the full id list so selections
are not truncated.

Comment on lines +528 to +572
const selectColumn =
estimatedCount != null
? {
...SelectColumn,
renderHeaderCell: (props: RenderHeaderCellProps<unknown>) => {
const { column } = props;
// eslint-disable-next-line react-hooks/rules-of-hooks
const [, onRowSelectionChange] = useRowSelection();
// eslint-disable-next-line react-hooks/rules-of-hooks
const rootprops = useCellRootProps(-1, column.key);

let checked: CheckedState = false;
if (selectedRows && selectedRows.size > 0) {
if (selectedRows.size >= estimatedCount) {
checked = true;
} else {
checked = "indeterminate";
}
}

return (
<CellRoot {...rootprops} className="border-t-0">
<Checkbox
aria-label="Select All"
tabIndex={props.tabIndex}
className="rdg-row__select-column__select-action"
checked={checked}
onCheckedChange={() => {
if (checked === false) {
onRowSelectionChange({
type: "HEADER",
checked: true,
});
} else {
onSelectedRowsChange?.(new Set());
}
}}
onClick={(e) => e.stopPropagation()}
onDoubleClick={(e) => e.stopPropagation()}
/>
</CellRoot>
);
},
}
: SelectColumn;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Hooks called inside a render callback violate the Rules of Hooks.

useRowSelection (line 535) and useCellRootProps (line 537) are called inside the renderHeaderCell function, which is a callback — not a React component. The static analysis tool correctly flags this. Extract the header into a standalone component so hooks are called at the top level.

Proposed fix — extract a component
+function IndeterminateSelectHeaderCell({
+  selectedRows,
+  estimatedCount,
+  onSelectedRowsChange,
+  ...props
+}: RenderHeaderCellProps<unknown> & {
+  selectedRows?: ReadonlySet<string>;
+  estimatedCount: number;
+  onSelectedRowsChange?: (rows: Set<string>) => void;
+}) {
+  const { column } = props;
+  const [, onRowSelectionChange] = useRowSelection();
+  const rootprops = useCellRootProps(-1, column.key);
+
+  let checked: CheckedState = false;
+  if (selectedRows && selectedRows.size > 0) {
+    if (selectedRows.size >= estimatedCount) {
+      checked = true;
+    } else {
+      checked = "indeterminate";
+    }
+  }
+
+  return (
+    <CellRoot {...rootprops} className="border-t-0">
+      <Checkbox
+        aria-label="Select All"
+        tabIndex={props.tabIndex}
+        className="rdg-row__select-column__select-action"
+        checked={checked}
+        onCheckedChange={() => {
+          if (checked === false) {
+            onRowSelectionChange({ type: "HEADER", checked: true });
+          } else {
+            onSelectedRowsChange?.(new Set());
+          }
+        }}
+        onClick={(e) => e.stopPropagation()}
+        onDoubleClick={(e) => e.stopPropagation()}
+      />
+    </CellRoot>
+  );
+}

Then in CustomerGrid, use the extracted component:

  const selectColumn =
    estimatedCount != null
      ? {
          ...SelectColumn,
-          renderHeaderCell: (props: RenderHeaderCellProps<unknown>) => {
-            // eslint-disable-next-line react-hooks/rules-of-hooks
-            ...
-          },
+          renderHeaderCell: (props: RenderHeaderCellProps<unknown>) => (
+            <IndeterminateSelectHeaderCell
+              {...props}
+              selectedRows={selectedRows}
+              estimatedCount={estimatedCount}
+              onSelectedRowsChange={onSelectedRowsChange}
+            />
+          ),
        }
      : SelectColumn;
📝 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
const selectColumn =
estimatedCount != null
? {
...SelectColumn,
renderHeaderCell: (props: RenderHeaderCellProps<unknown>) => {
const { column } = props;
// eslint-disable-next-line react-hooks/rules-of-hooks
const [, onRowSelectionChange] = useRowSelection();
// eslint-disable-next-line react-hooks/rules-of-hooks
const rootprops = useCellRootProps(-1, column.key);
let checked: CheckedState = false;
if (selectedRows && selectedRows.size > 0) {
if (selectedRows.size >= estimatedCount) {
checked = true;
} else {
checked = "indeterminate";
}
}
return (
<CellRoot {...rootprops} className="border-t-0">
<Checkbox
aria-label="Select All"
tabIndex={props.tabIndex}
className="rdg-row__select-column__select-action"
checked={checked}
onCheckedChange={() => {
if (checked === false) {
onRowSelectionChange({
type: "HEADER",
checked: true,
});
} else {
onSelectedRowsChange?.(new Set());
}
}}
onClick={(e) => e.stopPropagation()}
onDoubleClick={(e) => e.stopPropagation()}
/>
</CellRoot>
);
},
}
: SelectColumn;
function IndeterminateSelectHeaderCell({
selectedRows,
estimatedCount,
onSelectedRowsChange,
...props
}: RenderHeaderCellProps<unknown> & {
selectedRows?: ReadonlySet<string>;
estimatedCount: number;
onSelectedRowsChange?: (rows: Set<string>) => void;
}) {
const { column } = props;
const [, onRowSelectionChange] = useRowSelection();
const rootprops = useCellRootProps(-1, column.key);
let checked: CheckedState = false;
if (selectedRows && selectedRows.size > 0) {
if (selectedRows.size >= estimatedCount) {
checked = true;
} else {
checked = "indeterminate";
}
}
return (
<CellRoot {...rootprops} className="border-t-0">
<Checkbox
aria-label="Select All"
tabIndex={props.tabIndex}
className="rdg-row__select-column__select-action"
checked={checked}
onCheckedChange={() => {
if (checked === false) {
onRowSelectionChange({
type: "HEADER",
checked: true,
});
} else {
onSelectedRowsChange?.(new Set());
}
}}
onClick={(e) => e.stopPropagation()}
onDoubleClick={(e) => e.stopPropagation()}
/>
</CellRoot>
);
}
const selectColumn =
estimatedCount != null
? {
...SelectColumn,
renderHeaderCell: (props: RenderHeaderCellProps<unknown>) => (
<IndeterminateSelectHeaderCell
{...props}
selectedRows={selectedRows}
estimatedCount={estimatedCount}
onSelectedRowsChange={onSelectedRowsChange}
/>
),
}
: SelectColumn;
🧰 Tools
🪛 Biome (2.3.13)

[error] 535-535: This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component.

For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level

(lint/correctness/useHookAtTopLevel)


[error] 537-537: This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component.

For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level

(lint/correctness/useHookAtTopLevel)

🤖 Prompt for AI Agents
In `@editor/scaffolds/grid/wellknown/customer-grid.tsx` around lines 528 - 572,
The renderHeaderCell callback in selectColumn violates the Rules of Hooks by
calling useRowSelection and useCellRootProps inside the callback; extract that
header UI into a top-level React component (e.g., CustomerGridSelectHeader)
which accepts props needed from the grid (column, tabIndex, estimatedCount,
selectedRows, onSelectedRowsChange, and a handler to call onRowSelectionChange)
and move the hooks (useRowSelection, useCellRootProps) to the top of that
component; then change selectColumn.renderHeaderCell to return
<CustomerGridSelectHeader ... /> (or a function that renders it) and wire
Checkbox events to call onRowSelectionChange and onSelectedRowsChange exactly as
before.

Comment on lines +57 to +70
const handleImportSelected = async () => {
setImporting(true);
setError(null);
const ids = Array.from(selection);
const ok = await onImport(ids);
setImporting(false);
if (ok) {
setImportComplete(true);
setImportCount(ids.length);
} else {
setError("Failed to import customers");
toast.error("Failed to import customers");
}
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Unhandled rejection leaves the dialog stuck in "Importing" state.

If onImport throws (e.g. network error), setImporting(false) on line 62 is never reached and the dialog permanently shows the progress spinner. Wrap in try/catch or use a finally block.

🐛 Proposed fix
   const handleImportSelected = async () => {
     setImporting(true);
     setError(null);
     const ids = Array.from(selection);
-    const ok = await onImport(ids);
-    setImporting(false);
-    if (ok) {
-      setImportComplete(true);
-      setImportCount(ids.length);
-    } else {
-      setError("Failed to import customers");
-      toast.error("Failed to import customers");
+    try {
+      const ok = await onImport(ids);
+      if (ok) {
+        setImportComplete(true);
+        setImportCount(ids.length);
+      } else {
+        setError("Failed to import customers");
+        toast.error("Failed to import customers");
+      }
+    } catch {
+      setError("Failed to import customers");
+      toast.error("Failed to import customers");
+    } finally {
+      setImporting(false);
     }
   };
🤖 Prompt for AI Agents
In `@editor/scaffolds/platform/customer/customer-picker-dialog.tsx` around lines
57 - 70, The handleImportSelected function currently calls onImport(ids) without
error handling so any thrown rejection prevents setImporting(false) from
running; wrap the await onImport(ids) in a try/catch and move
setImporting(false) into a finally block (or ensure it always runs) so the
importing spinner is cleared, and in the catch setError(...) and call
toast.error(...) with the caught error message; update references to
setImportComplete and setImportCount only on successful import inside the try
block.

Comment on lines +87 to +121
/**
* Lightweight query that fetches only customer UIDs matching the given
* predicates and text search. No pagination -- returns all matching rows.
* Intended for "Select All" across pages.
*/
export async function fetchCustomerIds(
client: SupabaseClient<Database, "grida_ciam_public">,
project_id: number,
query: Pick<Data.Relation.QueryState, "q_predicates" | "q_text_search">
) {
const { q_predicates, q_text_search } = query;
const q = client
.from("customer_with_tags")
.select("uid")
.eq("project_id", project_id);

// predicates
const valid_predicates = q_predicates
?.map(Data.Query.Predicate.Extension.encode)
?.filter(Data.Query.Predicate.is_predicate_fulfilled);
valid_predicates?.forEach((predicate) => {
q.filter(predicate.column, predicate.op, predicate.value);
});

// text search (filter)
if (q_text_search && q_text_search.query) {
q.filter(
Platform.Customer.TABLE_SEARCH_TEXT,
"ilike",
`%${q_text_search.query}%`
);
}

return await q;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

No upper bound on returned rows — Supabase will apply its default limit.

This query has no .limit() or .range(), so PostgREST's server-side default (commonly 1000) silently caps the result set. For the "Select All" use case this means customers beyond the cap are never selected, and the user sees the wrong count.

Either raise the limit explicitly (e.g. .limit(100_000)) or paginate in a loop until all IDs are collected.

Quick fix — set an explicit high limit
   const q = client
     .from("customer_with_tags")
     .select("uid")
-    .eq("project_id", project_id);
+    .eq("project_id", project_id)
+    .limit(100_000); // explicit upper bound; consider paginated fetch for very large datasets
📝 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
/**
* Lightweight query that fetches only customer UIDs matching the given
* predicates and text search. No pagination -- returns all matching rows.
* Intended for "Select All" across pages.
*/
export async function fetchCustomerIds(
client: SupabaseClient<Database, "grida_ciam_public">,
project_id: number,
query: Pick<Data.Relation.QueryState, "q_predicates" | "q_text_search">
) {
const { q_predicates, q_text_search } = query;
const q = client
.from("customer_with_tags")
.select("uid")
.eq("project_id", project_id);
// predicates
const valid_predicates = q_predicates
?.map(Data.Query.Predicate.Extension.encode)
?.filter(Data.Query.Predicate.is_predicate_fulfilled);
valid_predicates?.forEach((predicate) => {
q.filter(predicate.column, predicate.op, predicate.value);
});
// text search (filter)
if (q_text_search && q_text_search.query) {
q.filter(
Platform.Customer.TABLE_SEARCH_TEXT,
"ilike",
`%${q_text_search.query}%`
);
}
return await q;
}
/**
* Lightweight query that fetches only customer UIDs matching the given
* predicates and text search. No pagination -- returns all matching rows.
* Intended for "Select All" across pages.
*/
export async function fetchCustomerIds(
client: SupabaseClient<Database, "grida_ciam_public">,
project_id: number,
query: Pick<Data.Relation.QueryState, "q_predicates" | "q_text_search">
) {
const { q_predicates, q_text_search } = query;
const q = client
.from("customer_with_tags")
.select("uid")
.eq("project_id", project_id)
.limit(100_000); // explicit upper bound; consider paginated fetch for very large datasets
// predicates
const valid_predicates = q_predicates
?.map(Data.Query.Predicate.Extension.encode)
?.filter(Data.Query.Predicate.is_predicate_fulfilled);
valid_predicates?.forEach((predicate) => {
q.filter(predicate.column, predicate.op, predicate.value);
});
// text search (filter)
if (q_text_search && q_text_search.query) {
q.filter(
Platform.Customer.TABLE_SEARCH_TEXT,
"ilike",
`%${q_text_search.query}%`
);
}
return await q;
}
🤖 Prompt for AI Agents
In `@editor/scaffolds/platform/customer/use-customer-feed.ts` around lines 87 -
121, fetchCustomerIds currently issues a select on
client.from("customer_with_tags") without any .limit()/.range(), so PostgREST's
default row cap will silently truncate results; update fetchCustomerIds to
either set an explicit high limit (e.g. call .limit(...) or .range(...) on the
query builder before executing) or implement pagination (loop using offset/range
until no more rows) to ensure all matching UIDs are returned when applying
q_predicates and q_text_search (references: fetchCustomerIds, q_predicates,
q_text_search, Platform.Customer.TABLE_SEARCH_TEXT).

Comment on lines +84 to +89
<article
className="p-4 prose prose-sm dark:prose-invert"
dangerouslySetInnerHTML={{
__html: data?.article?.html ?? "",
}}
/>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if DOMPurify or any HTML sanitization library is already used in the project
rg -l "dompurify\|DOMPurify\|sanitize-html\|sanitizeHtml\|xss\|purify|Purify" --type=ts --type=tsx -g '!node_modules' 2>/dev/null | head -20

Repository: gridaco/grida

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Search for article HTML handling and where it comes from
rg -n "article.*html\|data\?\.article" --type=ts --type=tsx -g '!node_modules' -g '!*.test.*' 2>/dev/null | head -40

Repository: gridaco/grida

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Check the full context of the share.tsx file
wc -l "editor/theme/templates/enterprise/west-referral/referrer/share.tsx"

Repository: gridaco/grida

Length of output: 127


🏁 Script executed:

#!/bin/bash
# Read the share.tsx file
cat -n "editor/theme/templates/enterprise/west-referral/referrer/share.tsx"

Repository: gridaco/grida

Length of output: 4799


🏁 Script executed:

#!/bin/bash
# Search for where 'data' is sourced (props, fetch, API calls, etc.)
rg -n "article|data\.|props\.|fetch\|useQuery\|apollo" "editor/theme/templates/enterprise/west-referral/referrer/share.tsx" -A 3 -B 3

Repository: gridaco/grida

Length of output: 994


🏁 Script executed:

#!/bin/bash
# Look for CMS integration or HTML rendering utilities in the project
rg -l "cms\|CMS\|markdown\|html\|sanitize" --type=ts --type=tsx "editor/theme/" 2>/dev/null | head -20

Repository: gridaco/grida

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Find TemplateData definition
rg -n "TemplateData|West_Referrral__Duo_001" --type=ts --type=tsx -g '!node_modules' | head -30

Repository: gridaco/grida

Length of output: 84


🏁 Script executed:

#!/bin/bash
# Find the templates file that's imported
find . -path ./node_modules -prune -o -type f -name "templates.*" -print | grep -E "\.(ts|tsx)$" | head -20

Repository: gridaco/grida

Length of output: 167


🏁 Script executed:

#!/bin/bash
# Check if there's a sanitization utilities file anywhere in the project
rg -l "sanitize|Sanitize|purify|Purify" --type=ts --type=tsx -g '!node_modules' 2>/dev/null

Repository: gridaco/grida

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Read the templates.ts file
cat -n "./editor/theme/templates/enterprise/west-referral/templates.ts" | head -150

Repository: gridaco/grida

Length of output: 2808


🏁 Script executed:

#!/bin/bash
# Check parent templates.ts for TemplateData definition
cat -n "./editor/k/templates.ts" | head -100

Repository: gridaco/grida

Length of output: 291


🏁 Script executed:

#!/bin/bash
# Search for where template data is loaded or fetched
rg -n "grida_www|template.*data|TemplateData" --type ts -g '!node_modules' -g '!*.test.*' | grep -E "(fetch|load|query|mutation)" | head -20

Repository: gridaco/grida

Length of output: 197


🏁 Script executed:

#!/bin/bash
# Look for API routes or server-side handlers for template data
fd -t f "route\|api" --base-directory "editor/" -o --base-directory "apps/" 2>/dev/null | head -20

Repository: gridaco/grida

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Search for template data validation or sanitization at ingestion points
rg -n "template|schema|validate|sanitize" --type ts -g '!node_modules' -g '!*.test.*' | grep -i "data\|input\|validate" | head -30

Repository: gridaco/grida

Length of output: 3823


🏁 Script executed:

#!/bin/bash
# Search for API routes handling template data or campaigns
find . -path ./node_modules -prune -o -type d -name "api" -print | head -10

Repository: gridaco/grida

Length of output: 191


🏁 Script executed:

#!/bin/bash
# Look for richtext processing, html validation, or sanitization in backend services
rg -n "richtext|dangerously|html.*sanitize|sanitize.*html" --type ts -g '!node_modules' 2>/dev/null | head -20

Repository: gridaco/grida

Length of output: 1730


🏁 Script executed:

#!/bin/bash
# Check if there's any mention of DOMPurify or sanitization in package.json files
find . -name "package.json" -path '*/node_modules' -prune -o -type f -name "package.json" -exec grep -l "dompurify\|sanitize-html\|xss" {} \; 2>/dev/null

Repository: gridaco/grida

Length of output: 39


Add client-side HTML sanitization for user-editable template content.

Template article content (data?.article?.html) comes from campaign-editable template data and is rendered via dangerouslySetInnerHTML. While not direct user input, if campaign authors can modify this HTML, it creates an XSS vector. No sanitization libraries (DOMPurify, sanitize-html) were found in the project.

Confirm that template data is sanitized upstream during ingestion/storage, or add client-side sanitization (e.g., DOMPurify) as defense-in-depth.

🧰 Tools
🪛 ast-grep (0.40.5)

[warning] 85-85: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html

(react-unsafe-html-injection)

🪛 Biome (2.3.13)

[error] 87-87: Avoid passing content using the dangerouslySetInnerHTML prop.

Setting content using code can expose users to cross-site scripting (XSS) attacks

(lint/security/noDangerouslySetInnerHtml)

🤖 Prompt for AI Agents
In `@editor/theme/templates/enterprise/west-referral/referrer/share.tsx` around
lines 84 - 89, The template HTML is injected via dangerouslySetInnerHTML using
data?.article?.html which risks XSS if campaign authors can edit templates;
sanitize the HTML client-side before assigning it: import a sanitizer such as
DOMPurify and call DOMPurify.sanitize(data?.article?.html ?? "") and pass that
sanitized string to the element's __html (replace direct use of
data?.article?.html in dangerouslySetInnerHTML). Also, if possible, add a short
guard or fallback for empty/undefined content and document/confirm whether
ingestion/storage already sanitizes templates for defense-in-depth.

- Eliminated the descriptionHtml prop in favor of using the description prop directly as a ReactNode.
- Updated the rendering logic to simplify the component and enhance maintainability.
Copy link

@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: 2

🤖 Fix all issues with AI agents
In `@editor/components/dialogs/share-dialog.tsx`:
- Around line 152-160: The onNativeShareClick handler currently calls
navigator.share without an SSR-safe guard and doesn't handle Promise rejections;
update onNativeShareClick to first check typeof navigator !== "undefined" &&
typeof navigator.share === "function" before calling, and wrap the await
navigator.share(...) call in a try/catch so any rejection (e.g., user
cancellation) is handled/logged and not silently swallowed; reference the
onNativeShareClick function to locate where to add the SSR guard and the
try/catch around navigator.share.
- Around line 226-229: The disabled prop accesses navigator.share directly
causing SSR ReferenceError; update the condition to first check typeof navigator
!== "undefined" before checking navigator.share (mirror buildSmsHref's guard) so
the expression becomes something like !shareText || !(typeof navigator !==
"undefined" && typeof navigator.share === "function"); modify the disabled
expression near the onClick that calls onNativeShareClick and ensure any other
navigator.share checks use the same typeof navigator !== "undefined" guard.
🧹 Nitpick comments (1)
editor/components/dialogs/share-dialog.tsx (1)

87-123: Consider exposing a className prop on the root wrapper for consumer restylability.

The component currently doesn't accept a className prop, which limits the ability of consumers to restyle the outer container. As per coding guidelines for /editor/components, components should expose className on outer wrappers to allow consumers to restyle.

Comment on lines +152 to +160
const onNativeShareClick = async () => {
if (!payload) return;
if (typeof navigator.share !== "function") return;
await navigator.share({
title: payload.title?.trim(),
text: payload.text?.trimEnd(),
url: payload.url?.trim(),
});
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

navigator.share() rejection (e.g. user cancellation) is swallowed, but the guard at Line 154 also needs an SSR check.

Line 154 accesses navigator.share directly. While the button should be disabled during SSR (once the above fix is applied), it's still good practice to guard:

-    if (typeof navigator.share !== "function") return;
+    if (typeof navigator === "undefined" || typeof navigator.share !== "function") return;

This is a minor defensive addition — the main SSR fix is on the disabled prop (Line 228).

📝 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
const onNativeShareClick = async () => {
if (!payload) return;
if (typeof navigator.share !== "function") return;
await navigator.share({
title: payload.title?.trim(),
text: payload.text?.trimEnd(),
url: payload.url?.trim(),
});
};
const onNativeShareClick = async () => {
if (!payload) return;
if (typeof navigator === "undefined" || typeof navigator.share !== "function") return;
await navigator.share({
title: payload.title?.trim(),
text: payload.text?.trimEnd(),
url: payload.url?.trim(),
});
};
🤖 Prompt for AI Agents
In `@editor/components/dialogs/share-dialog.tsx` around lines 152 - 160, The
onNativeShareClick handler currently calls navigator.share without an SSR-safe
guard and doesn't handle Promise rejections; update onNativeShareClick to first
check typeof navigator !== "undefined" && typeof navigator.share === "function"
before calling, and wrap the await navigator.share(...) call in a try/catch so
any rejection (e.g., user cancellation) is handled/logged and not silently
swallowed; reference the onNativeShareClick function to locate where to add the
SSR guard and the try/catch around navigator.share.

Comment on lines +226 to +229
onClick={() => onNativeShareClick().catch(() => {})}
disabled={
!shareText || typeof navigator.share !== "function"
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

SSR crash: navigator.share is accessed without a typeof navigator guard.

"use client" components are still server-rendered during Next.js SSR. On the server, navigator is not defined, so evaluating typeof navigator.share will throw a ReferenceError — unlike typeof navigator, the typeof operator does not prevent the error when accessing a property of an undeclared variable.

Note that buildSmsHref (Line 56) already handles this correctly with typeof navigator !== "undefined".

Proposed fix
                       disabled={
-                        !shareText || typeof navigator.share !== "function"
+                        !shareText ||
+                        typeof navigator === "undefined" ||
+                        typeof navigator.share !== "function"
                       }
📝 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
onClick={() => onNativeShareClick().catch(() => {})}
disabled={
!shareText || typeof navigator.share !== "function"
}
onClick={() => onNativeShareClick().catch(() => {})}
disabled={
!shareText ||
typeof navigator === "undefined" ||
typeof navigator.share !== "function"
}
🤖 Prompt for AI Agents
In `@editor/components/dialogs/share-dialog.tsx` around lines 226 - 229, The
disabled prop accesses navigator.share directly causing SSR ReferenceError;
update the condition to first check typeof navigator !== "undefined" before
checking navigator.share (mirror buildSmsHref's guard) so the expression becomes
something like !shareText || !(typeof navigator !== "undefined" && typeof
navigator.share === "function"); modify the disabled expression near the onClick
that calls onNativeShareClick and ensure any other navigator.share checks use
the same typeof navigator !== "undefined" guard.

@softmarshmallow softmarshmallow merged commit 4e58e84 into main Feb 8, 2026
10 checks passed
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