Skip to content

Conversation

@softmarshmallow
Copy link
Member

@softmarshmallow softmarshmallow commented Jan 19, 2026

  • New Features

    • Added site-wide default language setting to portal configuration.
    • Added default country selection for phone fields in forms.
    • Portal login page now displays dynamic tenant-specific title.
  • Bug Fixes

    • Improved email language resolution logic for challenge and access flows.
    • Enhanced customer data handling for email-based sessions.
    • Refined phone field data preservation during updates.
Screenshot 2026-01-20 at 1 19 49 AM

@vercel
Copy link

vercel bot commented Jan 19, 2026

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

Project Deployment Review Updated (UTC)
docs Ready Ready Preview, Comment Jan 19, 2026 4:09pm
grida Ready Ready Preview, Comment Jan 19, 2026 4:09pm
5 Skipped Deployments
Project Deployment Review Updated (UTC)
code Ignored Ignored Jan 19, 2026 4:09pm
legacy Ignored Ignored Jan 19, 2026 4:09pm
backgrounds Skipped Skipped Jan 19, 2026 4:09pm
blog Skipped Skipped Jan 19, 2026 4:09pm
viewer Skipped Skipped Jan 19, 2026 4:09pm

Request Review

@coderabbitai
Copy link

coderabbitai bot commented Jan 19, 2026

Walkthrough

The PR introduces async language resolution via getLocale in email challenge routes, implements phone field country picker support with new PhoneFieldData typing, adds customer session binding logic to prevent duplicate customer creation, enables dynamic portal title fetching, supports configurable site languages, and adds a temporary single-campaign redirect feature.

Changes

Cohort / File(s) Summary
Language Resolution in Email Challenges
editor/app/(api)/(public)/v1/session/.../challenge/email/start/route.ts, editor/app/(tenant)/~/[tenant]/api/ciam/auth/challenge/with-email/route.ts, editor/app/(tenant)/~/[tenant]/api/p/access/with-email/route.ts
Replaced synchronous select_lang with async getLocale for email language determination; introduced fallback language computation and async resolution flow across three challenge/access routes.
Customer Session Binding & Upsert Logic
editor/app/(api)/(public)/v1/submit/[id]/route.ts
Introduced effective_session_customer_id combining session-bound and verified customers; refactored upsert flow to skip customer creation when a trusted session customer exists; added email/phone/name enrichment with non-blocking error handling.
Tel Field Data Typing & Schema
editor/grida-forms-hosted/types.ts, editor/app/(api)/private/editor/[form_id]/fields/route.ts
Added PhoneFieldData interface with default_country property; extended FormFieldDataSchema union; implemented special tel field null-handling in safe_data_field to preserve object shape.
Phone Country Picker Component
editor/components/extension/phone-country-picker.tsx, editor/components/formfield/form-field.tsx, editor/components/formfield/phone-field/phone-field.tsx
New reusable PhoneCountryPicker component with search and selection UI; wired defaultCountry from field data to PhoneField; reordered precedence in usePhoneFieldDefaultCountry to prioritize field-level over provider default.
Phone Field Panel Integration
editor/scaffolds/panels/field-edit-panel.tsx
Added phone-specific settings panel with default country override switch and country selector; integrated country options via useMemo and getCountries from libphonenumber-js.
Portal & Site Settings Language Support
editor/app/(tenant)/~/[tenant]/(p)/p/login/page.tsx, editor/app/(workbench)/[org]/[proj]/(console)/(resources)/www/page.tsx, editor/app/(workbench)/[org]/[proj]/(console)/(resources)/www/section-general.tsx
Added dynamic portal title fetching; introduced lang field to site settings with UI language selector; updated site form to handle and persist language preferences.
Portal Session Redirect
editor/app/(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx
Added temporary feature flag and single-campaign redirect logic; short-circuits UI rendering when exactly one referrer campaign exists and redirect path is valid.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Daily #500: Modifies language-resolution logic in the same email challenge route file, updating langCandidate and fallback computation.
  • grida_ciam #491: Implements coordinated email-OTP/challenge flow changes across routes, customer handling, and i18n/select_lang → getLocale migration.
  • [CRM] Project Customers Page #277: Overlaps on customer management surfaces including customer types and APIs that relate to the session binding logic.

Suggested labels

daily

Poem

🐰 Hops through languages and phones we go,
With countries picked and sessions flow,
Default tongues and portals gleam,
Customer journeys, a seamless dream!
A picker here, a binding there—
The form fields now have extra flair! 🌍✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.74% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Daily RC' is vague and generic; it does not clearly convey what the pull request does or address the specific changes made across 12+ files including language resolution refactoring, phone field country picker support, and portal title improvements. Use a more specific title that describes the primary change, such as 'Add phone country picker and refactor language resolution' or similar that captures the main functionality being introduced.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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 enterprise

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.

…utes and enhance site settings form with language options
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: 4

Caution

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

⚠️ Outside diff range comments (1)
editor/app/(workbench)/[org]/[proj]/(console)/(resources)/www/page.tsx (1)

210-216: Add fallback for legacy rows where lang is null.

While the type system defines lang as a required string, legacy records may have null values. Passing null to the Select component would create an empty/uncontrolled input and could cause a NOT NULL constraint violation when saving. Default to "en" (which is already marked as the default option in the component) to handle any data inconsistency gracefully.

Suggested fix
        defaultValues={{
          title: data.title,
          description: data.description,
-          lang: data.lang,
+          lang: data.lang ?? "en",
        }}
🤖 Fix all issues with AI agents
In `@editor/app/`(tenant)/~/[tenant]/(p)/p/login/page.tsx:
- Around line 45-56: The brand element currently uses the Link component with
href="#" (Link) which is decorative and non-functional; replace the Link wrapper
with a plain div (preserving className="flex items-center gap-2 min-w-0" and
data-testid="portal-login-navbar-brand-link") so the element is semantic and
accessible, keep the inner span (data-testid="portal-login-navbar-title")
unchanged; also remove or adjust any now-unused Link import if applicable.

In `@editor/app/`(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx:
- Around line 133-145: The redirect uses campaign.www_route_path without
validating it (see TMP_SHOULD_REDIRECT_WHEN_SINGLE, iam_referrers, redirect and
the JSX interpolation `${r.campaign.www_route_path}/t/${r.code}`), creating an
open-redirect risk; fix by validating and normalizing www_route_path before use:
ensure it is a safe relative path (e.g., starts with a single '/' and does not
start with '//' or contain a scheme like 'http:'), reject or fallback for any
absolute or protocol-relative values, and apply the same
validation/normalization both in the redirect branch (where return redirect(...)
is called) and in the JSX rendering that builds
`${r.campaign.www_route_path}/t/${r.code}` so only validated relative paths are
used.

In
`@editor/app/`(workbench)/[org]/[proj]/(console)/(resources)/www/section-general.tsx:
- Around line 77-91: The FieldLabel isn't programmatically associated with the
Select control, so assistive tech and click-to-focus won't target the
SelectTrigger; add a stable id (e.g., "default-lang-select") to the
SelectTrigger and set that id on FieldLabel via htmlFor (or set aria-labelledby
on Select/SelectTrigger if FieldLabel doesn't support htmlFor) so clicking the
label focuses/opens the Select and the label is announced correctly; update the
components around FieldLabel, Select, and SelectTrigger (and keep
SelectValue/value.lang and onValueChange logic unchanged).

In `@editor/components/extension/phone-country-picker.tsx`:
- Around line 87-145: The code calls
RPNInput.getCountryCallingCode(selectedCountry) (in PhoneCountryPicker) and
similarly inside PhoneCountryPickerItem without guarding against
unsupported/invalid country codes; update these call sites (PhoneCountryPicker
and PhoneCountryPickerItem) to first validate the country using
RPNInput.isSupportedCountry(selectedCountry) or wrap getCountryCallingCode in a
try-catch, and provide a safe fallback (e.g., empty string or "—") when
invalid/unsupported so the UI won't throw at runtime.
🧹 Nitpick comments (6)
editor/components/formfield/form-field.tsx (1)

305-314: Default country wiring is functional, but type casting could be tightened.

The logic correctly extracts and passes default_country to PhoneField. However, the double cast pattern (as any on line 312) is a bit loose.

Consider using a more explicit cast to CountryCode | undefined:

♻️ Suggested refinement
         const defaultCountry = (data as PhoneFieldData | undefined)
           ?.default_country;

         return (
-          // `@ts-expect-error` - PhoneField component prop type mismatch
           <PhoneField
             {...(sharedInputProps as React.ComponentProps<"input">)}
-            defaultCountry={defaultCountry as any}
+            defaultCountry={defaultCountry as CountryCode | undefined}
           />
         );

This would require importing CountryCode from libphonenumber-js/core.

editor/scaffolds/panels/field-edit-panel.tsx (1)

537-590: Phone field configuration section is well-implemented.

The UI flow is intuitive: toggle enables/disables country override, and the picker appears conditionally. The state mutation logic correctly:

  • Preserves existing data properties when updating
  • Removes default_country when toggling off
  • Defaults to "US" when first enabling

The multiple (data as PhoneFieldData | null | undefined) casts are repetitive but acceptable for this scope.

♻️ Optional: Extract repeated casting to a helper
// At the top of the component or as a utility
const phoneData = data as PhoneFieldData | null | undefined;

Then use phoneData?.default_country throughout this section.

editor/app/(tenant)/~/[tenant]/(p)/p/login/page.tsx (2)

37-40: Parallelize params + locale resolution for a small latency win.
You can resolve params and getLocale together since they’re independent.

⚡ Suggested tweak
-  const { tenant } = await params;
-  const locale = await getLocale(["en", "ko"]);
-  const title = await fetchPortalTitle(tenant);
+  const [{ tenant }, locale] = await Promise.all([
+    params,
+    getLocale(["en", "ko"]),
+  ]);
+  const title = await fetchPortalTitle(tenant);

14-29: Use .maybeSingle() instead of .single() to properly handle missing rows.

.single() returns an error when no row exists, but the current code only destructures data, so errors are silently ignored. This masks real failures (network errors, auth issues, etc.). Since the function already has a fallback to "Customer Portal", use .maybeSingle() which returns null when no rows exist and only errors on actual failures. Capture and log the error object explicitly.

♻️ Suggested adjustment
-  const { data: wwwPublic } = await client
+  const { data: wwwPublic, error } = await client
     .from("www_public")
     .select("title")
     .eq("name", tenant)
-    .single()
+    .maybeSingle()
     .returns<Pick<WwwPublicRow, "title">>();
+
+  if (error) {
+    console.warn("fetchPortalTitle failed", { tenant, error });
+  }
editor/app/(tenant)/~/[tenant]/api/ciam/auth/challenge/with-email/route.ts (1)

11-11: Trim tenant lang before select_lang for consistency.
If www.lang can include whitespace (e.g., " en "), select_lang will fall back unexpectedly.

♻️ Suggested tweak
- const fallback_lang = select_lang(www.lang, supported_languages, "en");
+ const fallback_lang = select_lang(www.lang?.trim(), supported_languages, "en");

Also applies to: 122-127

editor/app/(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx (1)

10-15: Consider wiring the temporary redirect flag to config.
A feature flag or env toggle would make rollback safer without a code change.

Comment on lines +45 to +56
<Link
href="#"
className="flex items-center gap-2 min-w-0"
data-testid="portal-login-navbar-brand-link"
>
<span
className="font-semibold truncate"
data-testid="portal-login-navbar-title"
>
{title}
</span>
</Link>
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:

cat -n editor/app/\(tenant\)/~/\[tenant\]/\(p\)/p/login/page.tsx | head -80

Repository: gridaco/grida

Length of output: 2353


🌐 Web query:

Next.js Link component href="#" accessibility screen reader issues

💡 Result:

Short answer: using href="#" (or an anchor with no real destination) inside a Next.js causes accessibility and focus/announcement problems for screen-reader and keyboard users. Prefer a real fragment (href="#id") or an actual URL for navigation; use a (not a link) for JS actions; and ensure semantic/proper attributes (aria-label, aria-disabled, id, passHref when needed). Sources and quick fixes below.

Why it's a problem

  • href="#" is an empty/same-page target that can confuse screen readers and keyboard focus (focus may remain on the trigger, and AT may not announce a page change). [1][3][4]
  • Next.js renders/extends an actual , and the docs expect real hrefs (including fragments). Using non-semantic or missing hrefs harms semantics/SEO and AT behaviour. [2][5]

Recommended fixes (concise)

  • If you need to jump to an element on the page, use a real fragment linking to an element id:
  • If the UI element triggers a JS action (open modal, toggle) and is not navigation, use a
(with aria attributes) instead of a link:
Open. Buttons expose correct semantics to AT. [1][3]

  • If you must present a disabled link, use aria-disabled and prevent pointer/keyboard activation, but prefer hiding or replacing it with non-interactive text:
    • <a href={url} aria-disabled="true" onClick={(e)=>e.preventDefault()}>Label and style pointer-events:none; also announce with aria-disabled. Note: many screen readers treat aria-disabled on anchors inconsistently—test with real AT. [1][3]
  • If using a custom child component that renders an , include passHref/legacyBehavior per Next.js docs so the underlying gets an href (important for accessibility/SEO). [2][4]
  • References

    • MDN: ARIA link role / prefer native semantics. [1] (developer.mozilla.org)
    • Next.js Link docs (Link renders to , supports fragments; passHref notes). [2] (nextjs.org)
    • Discussion on empty href/# accessibility problems. [3] (stackoverflow.com)
    • Reports of Next.js Link + same-page/hash focus behavior. [4] (stackoverflow.com)
    • Next.js accessibility/route announcer guidance (client navigation announcements). [5] (nextjs.org)

    If you want, tell me the exact code you're using and I’ll show the minimal fix.

    Citations:


    🌐 Web query:

    Next.js Link best practices when there's no destination href empty string

    💡 Result:

    Short answer — don't render a Next.js with an empty/undefined href. Instead conditionally render a real link only when you have a destination; otherwise render a semantic non-link control (button, span with role="button", or an anchor with aria-disabled and no href) and handle click behavior/accessibility explicitly.

    Concrete best practices

    • Condition the Link: only output when href is a non-empty, valid URL/path. This avoids invalid anchors and client-side navigation problems. [1][2]
    • If you need a non-navigating control that looks like a link, use a (preferred for actions) or a / (if purely presentational), and implement keyboard and screen-reader behavior. Ensure aria-disabled, tabindex, and onKeyDown/onClick semantics are correct for accessibility. [3]
    • If href may be external, use a plain with target/rel instead of next/link (next/link is for internal client-side navigation). [4]
    • Avoid using next/link with empty string or undefined because href is required for correct client-side routing and accessibility; conditionally compute/validate href first (e.g., href && <Link ... />). [1][2]
    • If you must render an anchor that shouldn’t navigate, preventDefault onClick and set aria-disabled="true", but prefer rendering a non-link element for actions. [3]

    References

    • Next.js Link docs / API (href is the navigation prop; use Link for internal client-side navigation). [1]
    • Next.js guidance / component docs and ESLint rule about proper Link usage (wrap anchors / passHref and only use Link for navigation). [2]
    • Accessibility guidance and best-practice recommendations for link vs. button semantics and disabled links. [3]
    • Advice: use plain for external links (next/link is for internal routes). [4]

    Would you like a short code example showing the conditional rendering + accessible alternative?


    Replace decorative Link with <div> to fix accessibility.
    Using href="#" creates a non-functional anchor that confuses screen readers and keyboard navigation. Since this is a brand element with no navigation target or action, use a plain <div> instead.

    Suggested fix
    -          <Link
    -            href="#"
    -            className="flex items-center gap-2 min-w-0"
    -            data-testid="portal-login-navbar-brand-link"
    -          >
    +          <div
    +            className="flex items-center gap-2 min-w-0"
    +            data-testid="portal-login-navbar-brand-link"
    +          >
                 <span
                   className="font-semibold truncate"
                   data-testid="portal-login-navbar-title"
                 >
                   {title}
                 </span>
    -          </Link>
    +          </div>
    📝 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
    <Link
    href="#"
    className="flex items-center gap-2 min-w-0"
    data-testid="portal-login-navbar-brand-link"
    >
    <span
    className="font-semibold truncate"
    data-testid="portal-login-navbar-title"
    >
    {title}
    </span>
    </Link>
    <div
    className="flex items-center gap-2 min-w-0"
    data-testid="portal-login-navbar-brand-link"
    >
    <span
    className="font-semibold truncate"
    data-testid="portal-login-navbar-title"
    >
    {title}
    </span>
    </div>
    🤖 Prompt for AI Agents
    In `@editor/app/`(tenant)/~/[tenant]/(p)/p/login/page.tsx around lines 45 - 56,
    The brand element currently uses the Link component with href="#" (Link) which
    is decorative and non-functional; replace the Link wrapper with a plain div
    (preserving className="flex items-center gap-2 min-w-0" and
    data-testid="portal-login-navbar-brand-link") so the element is semantic and
    accessible, keep the inner span (data-testid="portal-login-navbar-title")
    unchanged; also remove or adjust any now-unused Link import if applicable.
    

    Comment on lines +133 to +145
    if (
    TMP_SHOULD_REDIRECT_WHEN_SINGLE &&
    iam_referrers &&
    iam_referrers.length === 1
    ) {
    const r = iam_referrers[0];
    const path = r?.campaign?.www_route_path;
    const code = r?.code;
    if (typeof path === "string" && path && typeof code === "string" && code) {
    // FIXME: tenant url
    return redirect(`${path}/t/${code}`);
    }
    }
    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:

    # First, let's look at the actual file and the context around lines 133-145
    cat -n "editor/app/(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx" | sed -n '120,160p'

    Repository: gridaco/grida

    Length of output: 1506


    🏁 Script executed:

    # Get the full file size to understand what we're working with
    wc -l "editor/app/(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx"

    Repository: gridaco/grida

    Length of output: 122


    🏁 Script executed:

    # Search for type definitions related to 'campaign' or 'www_route_path'
    rg -i "www_route_path|campaign" --type ts --type tsx -A 2 -B 2 | head -100

    Repository: gridaco/grida

    Length of output: 84


    🏁 Script executed:

    # Search for the iam_referrers variable source and its type
    rg -i "iam_referrers" --type ts --type tsx -B 3 -A 3 | head -80

    Repository: gridaco/grida

    Length of output: 84


    🏁 Script executed:

    # Look for existing URL/path validation patterns in the codebase
    rg "(startsWith|redirect|protocol|absolute)" --type ts --type tsx -B 1 -A 1 | grep -i "path\|url\|redirect" | head -50

    Repository: gridaco/grida

    Length of output: 84


    🏁 Script executed:

    # Search without type filter for www_route_path and campaign type info
    rg "www_route_path" -A 2 -B 2

    Repository: gridaco/grida

    Length of output: 5231


    🏁 Script executed:

    # Look for type definitions or interfaces related to campaign
    rg "campaign" --type ts -A 2 -B 1 | head -150

    Repository: gridaco/grida

    Length of output: 10853


    🏁 Script executed:

    # Search for supabase schema or type definitions
    find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.sql" \) | xargs grep -l "campaign_public" | head -10

    Repository: gridaco/grida

    Length of output: 576


    🏁 Script executed:

    # Look for redirect usage and URL validation patterns throughout codebase
    rg "redirect\(" -A 1 -B 1 | grep -A 1 -B 1 "startsWith\|url\|path" | head -60

    Repository: gridaco/grida

    Length of output: 2196


    🏁 Script executed:

    # Search for URL validation helpers or utility functions
    find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "isSafeUrl\|validateUrl\|sanitizeUrl" | head -10

    Repository: gridaco/grida

    Length of output: 94


    🏁 Script executed:

    # Check if there are other usages of www_route_path in the session page that need attention
    sed -n '160,175p' "editor/app/(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx"

    Repository: gridaco/grida

    Length of output: 534


    🏁 Script executed:

    # Look at the schema definition for route_path to see if there are any constraints
    rg "route_path" supabase/schemas/ supabase/migrations/ -A 2 -B 2

    Repository: gridaco/grida

    Length of output: 2425


    Validate www_route_path before redirect to prevent open redirects.

    The campaign.www_route_path field can contain absolute URLs or protocol-relative paths without database constraints. If base_path or name (which compose this field) contain values like https://attacker.com or //attacker.com, the redirect becomes exploitable. Add validation to ensure the path is relative and safe:

    Suggested guard
    -    if (typeof path === "string" && path && typeof code === "string" && code) {
    +    const isSafePath =
    +      typeof path === "string" &&
    +      path &&
    +      path.startsWith("/") &&
    +      !path.startsWith("//") &&
    +      !path.includes("://");
    +    if (isSafePath && typeof code === "string" && code) {
           // FIXME: tenant url
           return redirect(`${path}/t/${code}`);
         }

    Note: The same vulnerable pattern appears in the JSX at line 169 (${r.campaign.www_route_path}/t/${r.code}); apply the same validation there.

    🤖 Prompt for AI Agents
    In `@editor/app/`(tenant)/~/[tenant]/(p)/p/session/[token]/page.tsx around lines
    133 - 145, The redirect uses campaign.www_route_path without validating it (see
    TMP_SHOULD_REDIRECT_WHEN_SINGLE, iam_referrers, redirect and the JSX
    interpolation `${r.campaign.www_route_path}/t/${r.code}`), creating an
    open-redirect risk; fix by validating and normalizing www_route_path before use:
    ensure it is a safe relative path (e.g., starts with a single '/' and does not
    start with '//' or contain a scheme like 'http:'), reject or fallback for any
    absolute or protocol-relative values, and apply the same
    validation/normalization both in the redirect branch (where return redirect(...)
    is called) and in the JSX rendering that builds
    `${r.campaign.www_route_path}/t/${r.code}` so only validated relative paths are
    used.
    

    Comment on lines +77 to +91
    <Field>
    <FieldLabel>Default (fallback) language</FieldLabel>
    <Select
    value={value.lang}
    onValueChange={(v) =>
    onValueChange?.({
    ...value,
    lang: v,
    })
    }
    disabled={disabled}
    >
    <SelectTrigger className="w-full">
    <SelectValue />
    </SelectTrigger>
    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

    Associate the language label with the Select trigger.
    Right now the label isn’t programmatically tied to the control, which weakens accessibility and click-to-focus behavior.

    💡 Suggested fix
    -          <FieldLabel>Default (fallback) language</FieldLabel>
    +          <FieldLabel htmlFor="site-lang" id="site-lang-label">
    +            Default (fallback) language
    +          </FieldLabel>
    ...
    -            <SelectTrigger className="w-full">
    +            <SelectTrigger
    +              id="site-lang"
    +              className="w-full"
    +              aria-labelledby="site-lang-label"
    +            >
    📝 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
    <Field>
    <FieldLabel>Default (fallback) language</FieldLabel>
    <Select
    value={value.lang}
    onValueChange={(v) =>
    onValueChange?.({
    ...value,
    lang: v,
    })
    }
    disabled={disabled}
    >
    <SelectTrigger className="w-full">
    <SelectValue />
    </SelectTrigger>
    <Field>
    <FieldLabel htmlFor="site-lang" id="site-lang-label">
    Default (fallback) language
    </FieldLabel>
    <Select
    value={value.lang}
    onValueChange={(v) =>
    onValueChange?.({
    ...value,
    lang: v,
    })
    }
    disabled={disabled}
    >
    <SelectTrigger
    id="site-lang"
    className="w-full"
    aria-labelledby="site-lang-label"
    >
    <SelectValue />
    </SelectTrigger>
    🤖 Prompt for AI Agents
    In
    `@editor/app/`(workbench)/[org]/[proj]/(console)/(resources)/www/section-general.tsx
    around lines 77 - 91, The FieldLabel isn't programmatically associated with the
    Select control, so assistive tech and click-to-focus won't target the
    SelectTrigger; add a stable id (e.g., "default-lang-select") to the
    SelectTrigger and set that id on FieldLabel via htmlFor (or set aria-labelledby
    on Select/SelectTrigger if FieldLabel doesn't support htmlFor) so clicking the
    label focuses/opens the Select and the label is announced correctly; update the
    components around FieldLabel, Select, and SelectTrigger (and keep
    SelectValue/value.lang and onValueChange logic unchanged).
    

    Comment on lines +87 to +145
    export function PhoneCountryPicker({
    disabled,
    value: selectedCountry,
    options: countryList,
    onChange,
    className,
    }: PhoneCountryPickerProps) {
    const selectedLabel =
    countryList.find((c) => c.value === selectedCountry)?.label ??
    selectedCountry;

    return (
    <Popover>
    <PopoverTrigger asChild>
    <Button
    type="button"
    variant="outline"
    className={cn("w-full justify-between gap-2 px-3 focus:z-10", className)}
    disabled={disabled}
    >
    <span className="flex min-w-0 items-center gap-2">
    <FlagComponent country={selectedCountry} countryName={selectedLabel} />
    <span className="line-clamp-1 text-left">{selectedLabel}</span>
    <span className="ml-auto shrink-0 text-sm text-foreground/50">{`+${RPNInput.getCountryCallingCode(
    selectedCountry
    )}`}</span>
    </span>
    <ChevronsUpDown
    className={cn(
    "size-4 shrink-0 opacity-50",
    disabled ? "hidden" : "opacity-100"
    )}
    />
    </Button>
    </PopoverTrigger>
    <PopoverContent className="w-[320px] p-0">
    <Command>
    <CommandInput placeholder="Search country..." />
    <CommandList>
    <ScrollArea className="h-72">
    <CommandEmpty>No country found.</CommandEmpty>
    <CommandGroup>
    {countryList.map(({ value, label }) => (
    <PhoneCountryPickerItem
    key={value}
    country={value}
    countryName={label}
    selectedCountry={selectedCountry}
    onChange={onChange}
    />
    ))}
    </CommandGroup>
    </ScrollArea>
    </CommandList>
    </Command>
    </PopoverContent>
    </Popover>
    );
    }
    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:

    # Find where RPNInput is imported and defined
    rg -n "RPNInput" editor/components/extension/phone-country-picker.tsx -B 3 -A 1

    Repository: gridaco/grida

    Length of output: 1408


    🏁 Script executed:

    # Search for RPNInput definition in the codebase
    rg -n "class RPNInput|export.*RPNInput|from.*RPNInput" --type ts

    Repository: gridaco/grida

    Length of output: 161


    🏁 Script executed:

    # Check the PhoneCountryPickerProps type definition
    rg -n "PhoneCountryPickerProps" editor/components/extension/phone-country-picker.tsx -B 10 -A 5

    Repository: gridaco/grida

    Length of output: 991


    🏁 Script executed:

    # Look for getCountryCallingCode implementation or documentation
    rg -n "getCountryCallingCode" --type ts -A 5

    Repository: gridaco/grida

    Length of output: 1772


    🏁 Script executed:

    # Check for error handling patterns in similar files
    fd -e tsx -e ts editor/components/ | head -5

    Repository: gridaco/grida

    Length of output: 434


    🏁 Script executed:

    # Check if there's any error handling or validation for country codes elsewhere
    rg -n "try|catch|getCountryCallingCode" editor/components/extension/ --type ts -B 2 -A 2

    Repository: gridaco/grida

    Length of output: 14366


    🏁 Script executed:

    # Look at the full phone-input.tsx to see if there's error handling there
    cat -n editor/components/extension/phone-input.tsx | grep -A 10 -B 10 "getCountryCallingCode"

    Repository: gridaco/grida

    Length of output: 899


    🏁 Script executed:

    # Search for any package.json or tsconfig to understand library versions
    fd -type f package.json | head -1 | xargs cat | grep -A 2 "react-phone-number-input"

    Repository: gridaco/grida

    Length of output: 227


    🌐 Web query:

    react-phone-number-input getCountryCallingCode invalid country code error handling

    💡 Result:

    Use the library's country-check helper (or guard the call) — don't call getCountryCallingCode with an unknown/invalid ISO alpha-2 code.

    Recommended approaches

    • Check support first:
      import { isSupportedCountry, getCountryCallingCode } from 'react-phone-number-input/input'
      if (isSupportedCountry(country)) {
      const calling = getCountryCallingCode(country) // safe
      } else {
      // handle invalid country code (show error / fallback)
      }

    • Or guard / try-catch (if using libphonenumber-js directly):
      try {
      const cc = getCountryCallingCode(country) // may throw / return undefined for invalid country
      } catch (e) {
      // handle invalid country code
      }

    Notes

    • react-phone-number-input exposes isSupportedCountry() and getCountryCallingCode() for building custom selects. [1]
    • libphonenumber-js documents that parsing/validation can return INVALID_COUNTRY and that getCountryCallingCode exists for valid two-letter country codes. Validate before calling or handle the parse error. [2]

    Sources
    [1] react-phone-number-input README (country helpers).
    [2] libphonenumber-js docs (validation / getCountryCallingCode / INVALID_COUNTRY).


    Add defensive handling for getCountryCallingCode() calls.

    The react-phone-number-input library documentation recommends validating country codes before calling getCountryCallingCode(), as it can throw or return undefined for unsupported codes. Although the component's type system constrains selectedCountry to RPNInput.Country, runtime values may not align with library-supported countries.

    Guard the calls on lines 110–111, line 58 (PhoneCountryPickerItem), and similar calls in related components using either:

    • isSupportedCountry() check before calling the function
    • Try-catch around the call

    This prevents potential runtime errors when invalid country codes are passed.

    🤖 Prompt for AI Agents
    In `@editor/components/extension/phone-country-picker.tsx` around lines 87 - 145,
    The code calls RPNInput.getCountryCallingCode(selectedCountry) (in
    PhoneCountryPicker) and similarly inside PhoneCountryPickerItem without guarding
    against unsupported/invalid country codes; update these call sites
    (PhoneCountryPicker and PhoneCountryPickerItem) to first validate the country
    using RPNInput.isSupportedCountry(selectedCountry) or wrap getCountryCallingCode
    in a try-catch, and provide a safe fallback (e.g., empty string or "—") when
    invalid/unsupported so the UI won't throw at runtime.
    

    @softmarshmallow softmarshmallow merged commit 9d1c93a into main Jan 19, 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