chore: implement new design for log details #4052
Conversation
ogzhanolguncu
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRefactors multiple log-details UIs across API overview, audit, and shared dashboard: replaces Card/CardContent with bordered divs, standardizes header height/padding, changes CopyButton to outline variant and repositions it, simplifies copy/formatting logic and some prop types, and updates close button/icon classes and spacing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as CopyButton (UI)
participant F as Formatter (getFormattedContent)
participant CB as Clipboard (navigator.clipboard)
User->>UI: clicks Copy
UI->>F: request formatted text (details)
alt details is string
F-->>UI: string
else details is array
F-->>UI: joined string (newline-separated)
else details is ReactNode
F-->>UI: (may return empty or string depending on implementation)
end
UI->>CB: writeText(formatted)
CB-->>UI: success / failure
note right of UI: Outline variant, positioned in container
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/components/logs/details/log-details/components/log-meta.tsx (1)
22-44: Don't drop falsy values when extracting React element text.
if (reactElement.props.value)/if (reactElement.props.children)treat0, empty strings, orfalseas absent, so those values disappear in the copied payload. Copying a status of0or an intentionally blank string currently returns nothing. Please test with falsy values.- if (reactElement.props.value) { - if (reactElement.props.value instanceof Date) { - return reactElement.props.value.toISOString(); - } - return String(reactElement.props.value); - } + if ("value" in reactElement.props) { + const { value } = reactElement.props; + if (value instanceof Date) { + return value.toISOString(); + } + if (value !== undefined && value !== null) { + return String(value); + } + } - if (reactElement.props.children) { - if (typeof reactElement.props.children === "string") { - return reactElement.props.children; - } - if (Array.isArray(reactElement.props.children)) { - return reactElement.props.children - .map((child: React.ReactNode) => extractTextFromReactElement(child)) - .join(""); - } - return extractTextFromReactElement(reactElement.props.children); - } + if ("children" in reactElement.props) { + const children = React.Children.toArray(reactElement.props.children); + if (children.length === 0) { + return ""; + } + return children.map((child) => extractTextFromReactElement(child)).join(""); + }apps/dashboard/components/logs/details/log-details/components/log-section.tsx (1)
44-60: Helper function may not handle ReactNode correctly for copying.The
getFormattedContentfunction returns an empty string whendetailsis aReact.ReactNode. This means users cannot copy ReactNode content to the clipboard. Consider extracting text from ReactNode elements, similar to theextractTextFromReactElementpattern used in the API overview variant.Compare with the related snippet from
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-section.tsx(lines 48-68):const getTextToCopy = () => { if (typeof details === "string") { return details; } return Object.entries(details) .map(([key, value]) => { if (value === null || value === undefined) { return key; } if (typeof value === "object" && value !== null && "props" in value) { return `${key}: ${extractTextFromReactElement(value)}`; } return `${key}: ${value}`; }) .join("\n"); };Apply this approach or add a similar text extraction utility:
const getFormattedContent = (details: LogSectionDetails): string => { if (Array.isArray(details)) { return details .map((header) => { const [key, ...valueParts] = header.split(":"); const value = valueParts.join(":").trim(); return `${key}: ${value}`; }) .join("\n"); } if (typeof details === "string") { return details; } - return ""; + // Extract text from React elements for copying + if (typeof details === "object" && details !== null && "props" in details) { + return extractTextFromReactElement(details); + } + + return String(details); };
🧹 Nitpick comments (1)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-outcome-distribution-section.tsx (1)
49-56: Consider accessibility of hover-only CopyButton.The CopyButton uses
opacity-0 group-hover:opacity-100, making it invisible until hover. While this is a common pattern for secondary actions, it poses accessibility challenges:
- Keyboard users may not discover the button exists until they tab to it (and low opacity makes focus state unclear)
- Touch device users cannot hover to reveal the button
- Screen reader users receive no indication that copy functionality is available
Consider one of these alternatives:
- Always show the button at reduced opacity (e.g.,
opacity-50 group-hover:opacity-100)- Add a visible focus state:
focus-visible:opacity-100- Provide an alternative copy mechanism in a more discoverable location
Apply this diff to improve keyboard accessibility:
<CopyButton value={getTextToCopy()} shape="square" variant="outline" - className="absolute bottom-2 right-2 opacity-0 group-hover:opacity-100 transition-opacity rounded-md p-4 bg-gray-2 hover:bg-gray-2 size-2" + className="absolute bottom-2 right-2 opacity-0 group-hover:opacity-100 focus-visible:opacity-100 transition-opacity rounded-md p-4 bg-gray-2 hover:bg-gray-2 size-2" aria-label="Copy content" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-header.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-outcome-distribution-section.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-section.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/roles-permissions.tsx(3 hunks)apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/components/log-footer.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/components/log-header.tsx(1 hunks)apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/components/log-section.tsx(2 hunks)apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/index.tsx(1 hunks)apps/dashboard/components/logs/details/log-details/components/log-header.tsx(1 hunks)apps/dashboard/components/logs/details/log-details/components/log-meta.tsx(1 hunks)apps/dashboard/components/logs/details/log-details/components/log-section.tsx(2 hunks)apps/dashboard/components/logs/details/log-details/index.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-03T14:21:19.543Z
Learnt from: chronark
PR: unkeyed/unkey#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
🧬 Code graph analysis (11)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-header.tsx (1)
internal/icons/src/icons/xmark.tsx (1)
XMark(15-51)
apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/components/log-footer.tsx (1)
internal/clickhouse/src/logs.ts (1)
log(25-39)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-outcome-distribution-section.tsx (1)
internal/ui/src/components/buttons/copy-button.tsx (1)
CopyButton(27-81)
apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/components/log-header.tsx (1)
internal/icons/src/icons/xmark.tsx (1)
XMark(15-51)
apps/dashboard/components/logs/details/log-details/components/log-meta.tsx (1)
internal/ui/src/components/buttons/copy-button.tsx (1)
CopyButton(27-81)
apps/dashboard/components/logs/details/log-details/components/log-header.tsx (2)
internal/clickhouse/src/logs.ts (1)
log(25-39)internal/icons/src/icons/xmark.tsx (1)
XMark(15-51)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/roles-permissions.tsx (1)
internal/ui/src/components/buttons/copy-button.tsx (1)
CopyButton(27-81)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-section.tsx (3)
apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/components/log-section.tsx (1)
LogSection(3-53)apps/dashboard/components/logs/details/log-details/components/log-section.tsx (1)
LogSection(5-42)internal/ui/src/components/buttons/copy-button.tsx (1)
CopyButton(27-81)
apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/components/log-section.tsx (1)
internal/ui/src/components/buttons/copy-button.tsx (1)
CopyButton(27-81)
apps/dashboard/components/logs/details/log-details/index.tsx (2)
apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/gateway-logs/utils.ts (2)
safeParseJson(61-72)extractResponseField(20-36)apps/dashboard/components/logs/details/log-details/components/log-footer.tsx (1)
LogFooter(17-109)
apps/dashboard/components/logs/details/log-details/components/log-section.tsx (3)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-section.tsx (1)
LogSection(49-103)apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/components/log-section.tsx (1)
LogSection(3-53)internal/ui/src/components/buttons/copy-button.tsx (1)
CopyButton(27-81)
⏰ 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). (3)
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Go API Local / Test
🔇 Additional comments (18)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-outcome-distribution-section.tsx (2)
24-48: Well-structured design implementation.The new bordered container design is clean and consistent:
- Proper use of design tokens (gray-2, gray-4, gray-11, accent-12)
- Dark mode support via
dark:bg-black- Consistent spacing and rounded corners
- Logical visual hierarchy with header and content sections
- Responsive hover states with the
groupclassThe refactor successfully replaces the Card component with a custom bordered container while maintaining readability and visual consistency.
51-51: Dropshapeprop removal suggestion TheButtoncomponent declares ashapevariant, so passingshape="square"toCopyButtonis valid.Likely an incorrect or invalid review comment.
apps/dashboard/components/logs/details/log-details/components/log-section.tsx (3)
3-3: Type expansion looks good.The union type
LogSectionDetailsappropriately extends the prop to supportReact.ReactNode, allowing more flexible content rendering beyond strings and arrays.
13-40: Container structure aligns with design system updates.The migration from
Card/CardContentto a custom bordered container with consistent styling (border,bg-gray-2,border-gray-4,rounded-[10px]) standardizes the visual presentation across log details components.
32-38: CopyButton integration follows best practices.The
CopyButtoncorrectly uses:
variant="outline"per the new designshape="square"for consistent button shape- Group hover pattern for smooth opacity transitions
- Proper positioning with
absolute bottom-2 right-2Based on learnings about
@unkey/uicomponents.apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-header.tsx (3)
14-14: Header standardization improves consistency.The fixed height (
h-[50px]) and consistent padding (px-4 py-2) align with the broader header layout updates across log details components, ensuring uniform vertical spacing.
28-32: Close button structure and styling updated correctly.The
shrink-0wrapper prevents the close button from being compressed during layout changes, and the XMark icon styling (text-grayA-9,size="sm-regular") aligns with the updated design system.
24-26: Flex layout maintains spacing withoutmr-4. The parent’sjustify-betweenand the left wrapper’sflex-1ensure adequate separation between the title and close button.apps/dashboard/app/(app)/[workspaceSlug]/audit/components/table/log-details/components/log-header.tsx (3)
17-17: Header standardization matches other components.The fixed height and padding align with the API overview header variant, ensuring consistent vertical spacing across all log detail views.
19-19: Badge styling improved.The removal of the trailing space in the Badge className (
font-monoinstead offont-mono) ensures consistent typography without extraneous whitespace.
23-27: Close button updates align with design system.The
shrink-0wrapper and XMark styling (text-grayA-9,size="sm-regular") match the changes in the API overview variant, maintaining visual consistency.apps/dashboard/components/logs/details/log-details/components/log-header.tsx (3)
13-13: Header standardization matches other variants.The fixed height and padding align with the API overview and audit header components.
14-30: Inline status badge simplifies layout.Moving the status badge inline after the path text removes duplication (the previous separate status badge and vertical separator) and streamlines the header layout. The conditional styling for 2xx, 4xx, and 5xx response status ranges correctly applies color-coded badges.
33-37: Close button styling consistent.The XMark icon styling (
text-grayA-9,size="sm-regular") matches the updates in other header components.apps/dashboard/components/logs/details/log-details/index.tsx (4)
49-53: Styled empty spans improve visual consistency.Rendering empty Request Body and Response Body values as styled spans (
text-xs text-accent-12 truncate) instead of plain text provides a consistent visual treatment for empty states.Also applies to: 62-66
77-91: Meta content empty state handling improved.The
createMetaContentfunction now returns styled empty spans for null/invalid meta values instead of plainEMPTY_TEXTstrings, aligning with the Request/Response Body rendering pattern.
325-330: Padding consistency verified Footer wrapper usespx-4, matching other sections like LogSection and LogMeta—no change needed.
157-157: Confirm and restore padding on non-animated log details Removingp-4on line 157 strips all container padding and child components don’t define their own; ensure visual consistency by re-adding padding or adjusting inner elements.
...s/[apiId]/_overview/components/table/components/log-details/components/roles-permissions.tsx
Show resolved
Hide resolved
...g]/apis/[apiId]/_overview/components/table/components/log-details/components/log-section.tsx
Outdated
Show resolved
Hide resolved
ogzhanolguncu
force-pushed
the
redesign-log-details
branch
from
7a13cb0 to
5a0fc71
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-section.tsx (1)
41-41: Consider removingtruncateto avoid hiding log data.The
truncateclass cuts off values with ellipsis, which could hide important debugging information in logs. The parent element already hasbreak-words, suggesting wrapping is intended. Consider allowing values to wrap fully for better visibility.Apply this diff to allow full value display:
- <span className="ml-2 text-accent-12 truncate">{value}</span> + <span className="ml-2 text-accent-12 break-words">{value}</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-section.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-section.tsx (1)
internal/ui/src/components/buttons/copy-button.tsx (1)
CopyButton(27-81)
⏰ 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). (2)
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
| if (typeof value === "object" && value !== null && "props" in value) { | ||
| return `${key}: ${value}`; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove dead code from previous React element extraction logic.
Lines 20-22 check for objects with a props property but perform the same operation as the fallback case (line 23). Based on the past review comments, this appears to be leftover code from the removed React element text-extraction logic.
Apply this diff to remove the redundant check:
return Object.entries(details)
.map(([key, value]) => {
if (value === null || value === undefined) {
return key;
}
- if (typeof value === "object" && value !== null && "props" in value) {
- return `${key}: ${value}`;
- }
return `${key}: ${value}`;
})
.join("\n");📝 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.
| if (typeof value === "object" && value !== null && "props" in value) { | |
| return `${key}: ${value}`; | |
| } | |
| return Object.entries(details) | |
| .map(([key, value]) => { | |
| if (value === null || value === undefined) { | |
| return key; | |
| } | |
| return `${key}: ${value}`; | |
| }) | |
| .join("\n"); |
🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-section.tsx
around lines 20 to 22, remove the dead redundant conditional that checks for
objects with a "props" property (the if (typeof value === "object" && value !==
null && "props" in value) block) since it simply returns the same string as the
fallback; delete that entire conditional so the fallback case handles these
values and keep surrounding logic intact.
| value={getTextToCopy()} | ||
| shape="square" | ||
| variant="outline" | ||
| className="absolute bottom-2 right-2 opacity-0 group-hover:opacity-100 transition-opacity rounded-md p-4 bg-gray-2 hover:bg-gray-2 size-2" |
There was a problem hiding this comment.
Fix incorrect button size.
The size-2 class sets the button dimensions to 2px (0.125rem), which is far too small for an interactive element. This appears to be a typo or incorrect value.
Remove size-2 to let the button size be determined by padding and icon size:
- className="absolute bottom-2 right-2 opacity-0 group-hover:opacity-100 transition-opacity rounded-md p-4 bg-gray-2 hover:bg-gray-2 size-2"
+ className="absolute bottom-2 right-2 opacity-0 group-hover:opacity-100 transition-opacity rounded-md p-4 bg-gray-2 hover:bg-gray-2"Alternatively, if a specific size is needed, consider size-8 or size-10 for an appropriately sized button.
📝 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.
| className="absolute bottom-2 right-2 opacity-0 group-hover:opacity-100 transition-opacity rounded-md p-4 bg-gray-2 hover:bg-gray-2 size-2" | |
| className="absolute bottom-2 right-2 opacity-0 group-hover:opacity-100 transition-opacity rounded-md p-4 bg-gray-2 hover:bg-gray-2" |
🤖 Prompt for AI Agents
In
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_overview/components/table/components/log-details/components/log-section.tsx
around line 51, the button element wrongly includes the class "size-2" which
reduces its dimensions to 2px; remove "size-2" so the button size is governed by
padding and icon size, or replace it with an appropriate utility like "size-8"
or "size-10" if a fixed size is required.
What does this PR do?
This PR implements new log details drawer design. Mainly targets sections.
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
/logs
/projects/[pId]/gateway-logs
/audit
/ratelimits/[nspace]/logs
/apis/[apiId]
/apis/[apiId]/keys/kspace/keyId
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated