Conversation
MichaelUnkey
requested review from
Flo4604,
chronark,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR adds GPU-accelerated rendering hints by applying the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
apps/dashboard/app/(app)/[workspaceSlug]/logs/components/controls/components/logs-display/components/display-popover.tsx (1)
159-159: Safari shadow fix via GPU compositing — looks good; verify stacking context side‑effectsAdding transform-gpu should prevent shadow clipping in Safari. Please sanity‑check:
- Any position: sticky/fixed descendants and focus rings still behave,
- Z‑index layering vs. headers/overlays remains correct.
If we keep adding this broadly, consider setting the default on the Popover base to avoid per‑instance duplication.
apps/dashboard/components/navbar-popover.tsx (1)
145-147: GPU layer on PopoverContent — approved; confirm perf/text on SafariThis should stop shadow cropping. Please verify:
- Virtualized list scroll smoothness,
- Text rendering doesn’t get noticeably thinner on Safari 17/18 and Chrome.
If needed at scale, default transform-gpu in the Popover component or a shared “panel” utility.
apps/dashboard/components/logs/checkbox/filter-item.tsx (1)
159-159: Drover.Content GPU layer — approved; verify nested interactionsTransform introduces a new containing block. Please test nested drovers/tooltips and any position: fixed descendants still behave as expected. If only some items need this, consider a prop to toggle transform-gpu where shadows are present.
apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/index.tsx (1)
38-38: Audit panel GPU layer — approved; ensure consistent layeringThis mirrors the logs panel. Please verify shadows render fully on Safari and the panel sits above page content. If needed, align z‑index with the logs panel for consistency across contexts.
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/create-identity-options.tsx (1)
60-60: TooltipContent GPU compositing — approved; confirm pointer/text fidelityLooks good. Please test on macOS/iOS Safari that:
- Pointer interactions remain correct (no dead zones),
- Text rendering looks crisp; if it appears thinner, consider enabling transform-gpu only while open (e.g., via data-state classes).
internal/ui/src/components/dialog/dialog-container.tsx (1)
39-41: Good addition; tidy up small class nits.
- transform-gpu here is fine for Safari shadows.
- Minor: duplicate max-h-[90vh] (Lines 39 and 41).
- Token consistency: border-gray-4 vs prevalent border-grayA-4 elsewhere.
Apply this cleanup:
- "drop-shadow-2xl transform-gpu border-gray-4 max-h-[90vh] overflow-hidden !rounded-2xl p-0 gap-0 flex flex-col flex-grow", + "drop-shadow-2xl transform-gpu border-grayA-4 overflow-hidden !rounded-2xl p-0 gap-0 flex flex-col flex-grow", "w-[90%] md:w-[70%] lg:w-[70%] xl:w-[50%] 2xl:w-[45%] max-w-[600px] max-h-[90vh] sm:max-h-[90vh] md:max-h-[70vh] lg:max-h-[90vh] xl:max-h-[80vh]",Please sanity-check z-index stacking against other portaled overlays (tooltips/popovers) in Safari/macOS and iOS.
apps/dashboard/app/(app)/[workspaceSlug]/authorization/roles/components/upsert-role/components/assign-permission/create-permission-options.tsx (1)
72-73: LGTM for Safari shadow fix; keep an eye on text AA.GPU compositing can slightly change font anti‑aliasing in Safari/Chrome. If you notice blur, consider limiting transform-gpu to interactive states only.
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/utils.ts (1)
7-7: Gate GPU transform to interactive states to reduce layers.Instead of always-on transform-gpu, apply it only when focused/active:
- "focus:border-grayA-12 focus:ring-gray-6 focus-visible:outline-none focus:ring-offset-0 drop-shadow-button transform-gpu", + "focus:border-grayA-12 focus:ring-gray-6 focus-visible:outline-none focus:ring-offset-0 drop-shadow-button focus:transform-gpu active:transform-gpu",Check sidebar scroll/perf on low‑end Macs after gating.
internal/ui/src/components/buttons/button.tsx (1)
58-63: Scope GPU transform to focus/active to avoid global compositing.Current change makes all buttons composited even when idle. Prefer gating to interactive states.
- "focus:ring focus:ring-gray-5 focus-visible:outline-none focus:ring-offset-0 drop-shadow-button transform-gpu", + "focus:ring focus:ring-gray-5 focus-visible:outline-none focus:ring-offset-0 drop-shadow-button focus:transform-gpu active:transform-gpu",- "focus:border-grayA-12 focus:ring focus:ring-gray-5 focus-visible:outline-none focus:ring-offset-0 drop-shadow-button transform-gpu", + "focus:border-grayA-12 focus:ring focus:ring-gray-5 focus-visible:outline-none focus:ring-offset-0 drop-shadow-button focus:transform-gpu active:transform-gpu",- "focus:border-grayA-12 focus:ring focus:ring-gray-5 focus-visible:outline-none focus:ring-offset-0 drop-shadow-button transform-gpu", + "focus:border-grayA-12 focus:ring focus:ring-gray-5 focus-visible:outline-none focus:ring-offset-0 drop-shadow-button focus:transform-gpu active:transform-gpu",
- Check for any visual diffs on focus/active rings.
- Validate no font AA regressions on Windows/Chromium.
Also applies to: 64-69, 70-75
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/hooks/use-fetch-identities/create-identity-options.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/components/assign-permission/create-permission-options.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/components/assign-role/create-key-options.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/index.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/authorization/roles/components/upsert-role/components/assign-key/create-key-options.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/authorization/roles/components/upsert-role/components/assign-permission/create-permission-options.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/logs/components/controls/components/logs-display/components/display-popover.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/settings/root-keys/components/root-key/root-key-success.tsx(1 hunks)apps/dashboard/components/logs/checkbox/filter-item.tsx(1 hunks)apps/dashboard/components/logs/details/log-details/index.tsx(1 hunks)apps/dashboard/components/logs/table-action.popover.tsx(1 hunks)apps/dashboard/components/navbar-popover.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/utils.ts(1 hunks)apps/dashboard/components/ui/drawer.tsx(1 hunks)apps/dashboard/components/virtual-table/components/loading-indicator.tsx(1 hunks)internal/ui/src/components/buttons/button.tsx(1 hunks)internal/ui/src/components/buttons/keyboard-button.tsx(1 hunks)internal/ui/src/components/dialog/dialog-container.tsx(1 hunks)internal/ui/src/components/dialog/navigable-dialog.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-12-03T14:23:07.189Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/app/(app)/logs/components/log-details/resizable-panel.tsx:37-49
Timestamp: 2024-12-03T14:23:07.189Z
Learning: In `apps/dashboard/app/(app)/logs/components/log-details/resizable-panel.tsx`, the resize handler is already debounced.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/index.tsxapps/dashboard/components/logs/details/log-details/index.tsx
📚 Learning: 2025-01-30T20:38:00.058Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2825
File: apps/dashboard/app/(app)/logs-v2/components/controls/components/logs-datetime/index.tsx:0-0
Timestamp: 2025-01-30T20:38:00.058Z
Learning: In the logs dashboard, keyboard shortcuts that toggle UI elements (like popovers) should be implemented in the component that owns the state being toggled, not in the presentational wrapper components. For example, the 'T' shortcut for toggling the datetime filter is implemented in DatetimePopover, not in LogsDateTime.
Applied to files:
apps/dashboard/components/navbar-popover.tsxapps/dashboard/components/logs/table-action.popover.tsxapps/dashboard/app/(app)/[workspaceSlug]/logs/components/controls/components/logs-display/components/display-popover.tsx
📚 Learning: 2024-12-03T14:07:45.173Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/components/ui/group-button.tsx:21-31
Timestamp: 2024-12-03T14:07:45.173Z
Learning: In the `ButtonGroup` component (`apps/dashboard/components/ui/group-button.tsx`), avoid suggesting the use of `role="group"` in ARIA attributes.
Applied to files:
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/utils.tsapps/dashboard/app/(app)/[workspaceSlug]/authorization/roles/components/upsert-role/components/assign-permission/create-permission-options.tsxapps/dashboard/app/(app)/[workspaceSlug]/authorization/roles/components/upsert-role/components/assign-key/create-key-options.tsxinternal/ui/src/components/buttons/button.tsxinternal/ui/src/components/buttons/keyboard-button.tsx
📚 Learning: 2024-12-03T14:21:19.543Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/app/(app)/logs/components/log-details/components/log-footer.tsx:58-61
Timestamp: 2024-12-03T14:21:19.543Z
Learning: For the "Outcome" field in the `LogFooter` component (`apps/dashboard/app/(app)/logs/components/log-details/components/log-footer.tsx`), default to "N/A" instead of "VALID" when handling null values to avoid confusing customers.
Applied to files:
apps/dashboard/components/logs/details/log-details/index.tsx
📚 Learning: 2024-10-08T15:33:04.290Z
Learnt from: AkshayBandi027
Repo: unkeyed/unkey PR: 2215
File: apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx:28-29
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In `authorization/roles/[roleId]/update-role.tsx`, the tag `role-${role.id}` is revalidated after updating a role to ensure that the caching mechanism is properly handled for roles.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/components/assign-permission/create-permission-options.tsxapps/dashboard/app/(app)/[workspaceSlug]/authorization/roles/components/upsert-role/components/assign-permission/create-permission-options.tsxapps/dashboard/app/(app)/[workspaceSlug]/authorization/roles/components/upsert-role/components/assign-key/create-key-options.tsxapps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/components/assign-role/create-key-options.tsx
📚 Learning: 2025-08-25T13:05:22.441Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3834
File: internal/ui/src/components/form/input.tsx:130-131
Timestamp: 2025-08-25T13:05:22.441Z
Learning: The Input component in internal/ui/src/components/form/input.tsx is designed to support interactive right icons (like X buttons for deletion) by enabling pointer events on the right icon container, allowing clicks to be handled by the icon rather than passed through to the input.
Applied to files:
apps/dashboard/components/virtual-table/components/loading-indicator.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build / Build
- GitHub Check: Test Dashboard / Test Dashboard
- GitHub Check: Test Packages / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
apps/dashboard/components/ui/drawer.tsx (1)
42-42: Drawer fixed + transform-gpu — approved; spot‑check iOS Safari behaviorsLooks good. Please verify:
- Overlay z‑order vs. content (new stacking context),
- iOS Safari momentum scrolling inside the drawer,
- Soft keyboard open/close doesn’t cause layout jank.
apps/dashboard/components/logs/table-action.popover.tsx (1)
91-91: PopoverContent GPU compositing — approved; confirm layering and focusEnsure:
- Focus outlines aren’t clipped,
- Popover remains above sticky table headers after this change.
If you see overlap issues, add an explicit z level (e.g., z-50) on PopoverContent.
apps/dashboard/components/logs/details/log-details/index.tsx (1)
142-142: LogDetails panel GPU layer — approved; quick sticky checkGood addition; and nice use of willChange when animated. Please confirm any sticky headers/controls inside the scrolling panel still stick correctly with the transform in place.
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/components/assign-role/create-key-options.tsx (1)
67-68: OK to force compositing; verify stacking context.transform-gpu creates a new stacking context. With z-[100] it should still overlay correctly, but please smoke‑test near other portaled layers and sticky headers.
internal/ui/src/components/dialog/navigable-dialog.tsx (1)
73-76: Looks good; watch for fixed/sticky descendants.A transformed DialogContent changes the containing block for position: fixed and can affect sticky. If any step content uses fixed/sticky toolbars, verify behavior didn’t regress in Safari.
apps/dashboard/app/(app)/[workspaceSlug]/settings/root-keys/components/root-key/root-key-success.tsx (1)
38-40: OK; note overflow combo.transform-gpu is fine here. The overflow-hidden + overflow-y-auto combo yields hidden on x and auto on y (intended). If the original shadow clipping was along x, confirm there’s no residual clipping when content grows.
apps/dashboard/app/(app)/[workspaceSlug]/authorization/roles/components/upsert-role/components/assign-key/create-key-options.tsx (1)
61-62: LGTM; verify layering with nearby popovers.New stacking context is expected; confirm it doesn’t get occluded by other z-indexed UI around role edit forms.
internal/ui/src/components/buttons/keyboard-button.tsx (1)
26-26: Safari shadow fix looks good; confirm no side effects from new stacking context.
transform-gpushould help prevent clipped shadows in Safari. Please sanity-check: text antialiasing on macOS/iOS, and that no sticky/fixed descendants rely on the ancestor being untransformed.apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/components/assign-permission/create-permission-options.tsx (1)
58-58: TooltipContent: GPU promotion approved; verify clipping and layering in Safari.With
overflow-hiddenanddrop-shadow-2xl, the addedtransform-gpushould avoid Safari clipping. Please verify at varied zoom levels on Safari macOS/iOS that shadows aren’t cut and z-index (z-[100]) still wins over nearby overlays.apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx (1)
122-122: DialogContent: good for Safari shadows; confirm scroll/overlay behavior.
transform-gpucan impact scrolling and create a stacking context. Please confirm on Safari (macOS/iOS): smooth scroll inside the dialog, and that portal overlays (confirm popover, tooltips) are not obscured/clipped.apps/dashboard/components/virtual-table/components/loading-indicator.tsx (1)
93-93: Footer card GPU promotion OK; quick perf check requested.Looks good. Please spot-check CPU/GPU usage and animations on mobile Safari when this footer appears/disappears; repeated GPU layers can add memory/paint cost.
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (11/04/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
3 participants
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps us understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated