Skip to content

feat(dashboard): BYOC cluster page redesign, create cluster next steps, themed Book a call - #5751

Merged
jog1t merged 1 commit into
mainfrom
frontend/byoc-cluster-polish-sync
Sep 18, 2026
Merged

jog1t merged 1 commit into
mainfrom
frontend/byoc-cluster-polish-sync

Conversation

@jog1t

@jog1t jog1t commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Syncs the BYOC dashboard work from the EE frontend so the two trees match again.

  • Cluster page is one screen: a header with status and actions, a Setup card holding both options side by side, and a right rail with Resources and Enterprise Support.
  • Regions, Commands and the metrics ingest token are tabs on one card. The token is a per-cluster singleton, so the tab is create / show / revoke for exactly one token, and the redacted view shows only the last four characters the API returns.
  • Region filter collapses to a +N more badge instead of stacking one badge per row.
  • Create cluster: the BYOC details step gains a description, a name placeholder and hint, and a Next steps aside. Each step sizes the dialog to its own content.
  • Book a call opens Cal's own modal instead of a dashboard dialog, themed from the dashboard's own tokens. Cal's embed only applies a theme to the iframe that is live when the config is sent, so the config is re-sent on linkReady; otherwise every reopen fell back to Cal's default palette.
  • Dashboard dialogs ignore outside clicks and Escape while the Cal booker is open, since its overlay mounts outside the React tree and its backdrop passes pointer events through.
  • MultiSelectFormField gains an opt-in maxCount prop; existing call sites are unchanged.
  • Region restart time decodes the operator boot id rather than rendering it as a timestamp.

@railway-app

railway-app Bot commented Sep 18, 2026

Copy link
Copy Markdown

🚅 Deployed to the actors-pr-5751 environment in rivet-frontend

Service Status Web Updated
frontend-inspector 😴 Sleeping (View Logs) Web Sep 18, 2026 at 12:46 am UTC
frontend-cloud ✅ Success (View Logs) Web Sep 18, 2026 at 12:39 am UTC
kitchen-sink ✅ Success (View Logs) Web Sep 18, 2026 at 12:39 am UTC
ladle ✅ Success (View Logs) Web Sep 18, 2026 at 12:39 am UTC
mcp-hub ✅ Success (View Logs) Web Sep 18, 2026 at 12:37 am UTC
website ❌ Build Failed (View Logs) Web Sep 18, 2026 at 12:36 am UTC

@railway-app
railway-app Bot temporarily deployed to rivet-frontend / actors-pr-5751 September 18, 2026 00:36 Destroyed

@the-company-company the-company-company Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 2 medium-severity findings

Reviewed commit 53ad6e4.

Comment thread frontend/src/components/hooks/use-dialog.tsx
Comment thread frontend/src/app/byoc/cluster-page.tsx
@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Review

Overall a clean sync of the BYOC dashboard redesign. The SectionCard refactor, tabbed activity panel, and themed Cal booker are well structured and match the stated goals. A few things worth a look before merge.

Potential bugs

Region pagination lost its per-page error/retry path (frontend/src/app/byoc/cluster-page.tsx)
useAllRegions now eagerly background-fetches all region pages via usePrefetchInfiniteQuery({ ..., pages: Infinity }), replacing the old manual "Show more" button (fetchNextPage + isFetchingNextPage). RegionsPanel still renders a Skeleton whenever hasNextPage is true:

{hasNextPage ? (
  <div className="mt-3 flex justify-center">
    <Skeleton className="h-4 w-32" />
  </div>
) : null}

If a later page's fetch fails (transient network error, timeout) while an earlier page already succeeded, data still has partial pages so hasNextPage stays true based on the last successful page — this skeleton spins indefinitely with no retry affordance, since refetch() (wired to the "Couldn't load regions" banner) only re-fetches already-fetched pages, not the next one. The only way to recover is switching tabs (which remounts RegionsPanel since TabsContent unmounts inactive tabs) or reloading. Worth adding a distinct error/retry state for the trailing pagination skeleton, or exposing fetchNextPage/error from useAllRegions.

Code quality

Third-party DOM coupling lives in the generic dialog hook (frontend/src/components/hooks/use-dialog.tsx)
isThirdPartyOverlayOpen() does a global document.querySelector("cal-modal-box") and is wired into onInteractOutside/onEscapeKeyDown for every dialog created via createDialogHook, not just the BYOC contact ones. This is a reasonable fix for the stated Cal-embed overlay problem, but it's a fairly brittle, app-wide coupling to a specific third-party custom element name — if Cal's embed markup changes, Escape/click-outside silently breaks for unrelated dialogs elsewhere in the app with no test coverage to catch it. Consider scoping this behavior (e.g. via a prop passed only from ByocContactTrigger-adjacent dialogs) rather than baking it into the shared hook.

Module-level mutable state for the Cal singleton (frontend/src/app/byoc/byoc-contact-trigger.tsx)
latestUiConfig/listening are plain module-scoped lets outside React's lifecycle. This is presumably intentional since the Cal SDK itself is a global singleton, but it's undocumented — a one-line comment explaining why this isn't useRef/useState (i.e., it must survive across all ByocContactTrigger instances and only bind the linkReady listener once for the whole app) would help the next reader.

Minor / nit

hslTokenToHex (frontend/src/app/byoc/cal-theme.ts) rounds each RGB channel with Math.round(value * 255).toString(16) without validating the result is a clean 2-digit hex byte before use. In practice the HSL math keeps values in range for the theme tokens this repo defines, so this is unlikely to bite today, but there's no guard if a future CSS var value produces something unexpected — a quick Number.isInteger/range check or clamping before toString(16) would make it defensive.

Test coverage

No unit test was added for readCalVars/hslTokenToHex in cal-theme.ts. Unlike the rest of the new UI (which per this repo's frontend conventions can skip Ladle stories since it depends on the full data-provider stack), this conversion function is pure and framework-free, so it'd be cheap to cover with a few fixed HSL-triplet → hex assertions and would catch regressions in the color math directly.

Nice touches

  • The operatorBootId → timestamp decode fix (bootIdToTimestamp) correctly resolves what was previously a garbage Time render.
  • Re-sending the Cal ui config on linkReady to fix the default-palette-on-reopen bug is a well-targeted fix with a clear explanation in the PR description.
  • MultiSelectFormField's new maxCount prop is opt-in and doesn't touch existing call sites.

🤖 Generated with Claude Code

@jog1t
jog1t merged commit c93a88b into main Sep 18, 2026
13 of 19 checks passed
@jog1t
jog1t deleted the frontend/byoc-cluster-polish-sync branch September 18, 2026 00:47
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