Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds a locked prop to the infinite canvas to disable interactions, introduces per-node sizing and a Safari foreignObject workaround, renames generator model from regions→sentinels, adjusts several node/status UI layouts, and changes multiple empty-state displays to render null (with FormSettingCard fallback). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/simulate/tree-generate.tsx (1)
224-258:⚠️ Potential issue | 🟡 MinorSet
min="1"for instance sliders to prevent empty sentinel groups.The slider currently allows
0instances per sentinel, which would create sentinels with no children in the visualization. While this doesn't cause a divide-by-zero error (the generator usesArray.from({ length: instanceCount }, ...)which handles 0 gracefully), it produces semantically invalid network topologies. All preset configurations enforcemin >= 1, so the slider should do the same.Fix
- min="0" + min="1" ... - min="0" + min="1"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/simulate/tree-generate.tsx around lines 224 - 258, The instance-per-sentinel range inputs allow 0 currently; change both input elements that read/write customConfig.instancesPerSentinel (the two <input type="range"> controls whose onChange call setCustomConfig) to use min="1" instead of min="0" so sliders cannot select zero, and ensure the onChange handlers clamp/adjust the paired value if necessary to keep min <= max (update setCustomConfig logic in those handlers to enforce the constraint while preserving generateMutation.isLoading disabling).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/nodes/node-wrapper/health-banner.tsx:
- Line 20: Replace the Tailwind template literal width class which the JIT can't
detect by moving the dynamic width into an inline style: keep the static
Tailwind classes (e.g., "mx-auto" and "-m-[20px]") on the div in the
HealthBanner component and add a style prop that sets width using
DEFAULT_NODE_WIDTH (e.g., style={{ width: `${DEFAULT_NODE_WIDTH}px` }} or
equivalent), removing the `w-[${DEFAULT_NODE_WIDTH}px]` from the className.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/tree/tree-element-node.tsx:
- Around line 1-35: The TreeElementNode component signature now requires a size
prop but canvas-boundary.tsx's error boundary fallback (the <TreeElementNode> at
line 11) is missing it; update that TreeElementNode usage to include a size
object (e.g., width: 400, height: 250) along with the existing id and position
so it matches how tree-layout.tsx passes size (NODE_SIZES[...] ) and preserves
the doubling logic in TreeElementNode.
In `@web/apps/dashboard/lib/trpc/routers/deploy/network/generate.ts`:
- Around line 14-19: The schema allows instancesPerSentinel.min >
instancesPerSentinel.max which can produce a negative range for getRandomInt;
update generatorConfigSchema to add a cross-field validation (using z.refine or
superRefine) on the instancesPerSentinel object to ensure min <= max and return
a clear validation error message; reference the instancesPerSentinel object and
its min/max fields in the refinement so the constraint is enforced before any
code (e.g., where getRandomInt is used) runs.
---
Outside diff comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/deployments/[deploymentId]/network/unkey-flow/components/simulate/tree-generate.tsx:
- Around line 224-258: The instance-per-sentinel range inputs allow 0 currently;
change both input elements that read/write customConfig.instancesPerSentinel
(the two <input type="range"> controls whose onChange call setCustomConfig) to
use min="1" instead of min="0" so sliders cannot select zero, and ensure the
onChange handlers clamp/adjust the paired value if necessary to keep min <= max
(update setCustomConfig logic in those handlers to enforce the constraint while
preserving generateMutation.isLoading disabling).
What does this PR do?
This PR fixes:
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated