fix(logs): split compact logs layout changes from PR 788#878
fix(logs): split compact logs layout changes from PR 788#878NieiR wants to merge 1 commit intoding113:devfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a focused set of UI improvements for the logs dashboard, aiming to enhance the user experience through a more compact and organized layout. By making the statistics panel inline and the filter section collapsible, the changes contribute to a cleaner and more efficient interface for viewing and managing logs. These targeted updates are intentionally isolated to ensure a streamlined review process. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 Walkthrough总体概述该拉取请求包含四个文件的更改：添加新的 变更内容
代码审查工作量🎯 3 (中等) | ⏱️ ~25 分钟 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively refactors the logs dashboard UI to be more compact, as described. The changes are well-executed, introducing a collapsible filter section and a more condensed statistics panel. The code is cleaner and more componentized as a result. I have a couple of suggestions for minor improvements to enhance maintainability and conciseness.
| const activeFilterCount = useMemo(() => { | ||
| let count = 0; | ||
| if (filters.userId !== undefined) count++; | ||
| if (filters.keyId !== undefined) count++; | ||
| if (filters.providerId !== undefined) count++; | ||
| if (filters.sessionId) count++; | ||
| if (filters.startTime && filters.endTime) count++; | ||
| if (filters.statusCode !== undefined || filters.excludeStatusCode200) count++; | ||
| if (filters.model) count++; | ||
| if (filters.endpoint) count++; | ||
| if (filters.minRetryCount !== undefined && filters.minRetryCount > 0) count++; | ||
| return count; | ||
| }, [filters]); |
There was a problem hiding this comment.
The current implementation for calculating activeFilterCount is correct, but it could be refactored to be more concise and easier to maintain, especially if more filters are added in the future. Using a declarative array-based approach can improve readability.
| const activeFilterCount = useMemo(() => { | |
| let count = 0; | |
| if (filters.userId !== undefined) count++; | |
| if (filters.keyId !== undefined) count++; | |
| if (filters.providerId !== undefined) count++; | |
| if (filters.sessionId) count++; | |
| if (filters.startTime && filters.endTime) count++; | |
| if (filters.statusCode !== undefined || filters.excludeStatusCode200) count++; | |
| if (filters.model) count++; | |
| if (filters.endpoint) count++; | |
| if (filters.minRetryCount !== undefined && filters.minRetryCount > 0) count++; | |
| return count; | |
| }, [filters]); | |
| const activeFilterCount = useMemo(() => { | |
| const conditions = [ | |
| filters.userId !== undefined, | |
| filters.keyId !== undefined, | |
| filters.providerId !== undefined, | |
| !!filters.sessionId, | |
| !!(filters.startTime && filters.endTime), | |
| filters.statusCode !== undefined || filters.excludeStatusCode200, | |
| !!filters.model, | |
| !!filters.endpoint, | |
| (filters.minRetryCount ?? 0) > 0, | |
| ]; | |
| return conditions.filter(Boolean).length; | |
| }, [filters]); |
| if (compactEmpty && !isLoading && totalCount === 0) { | ||
| return ( | ||
| <div | ||
| className={`flex items-center gap-3 px-4 py-2.5 rounded-lg border border-border/50 bg-card/30 text-sm ${className}`} |
There was a problem hiding this comment.
For improved maintainability and consistency with other components in the project, consider using the cn utility function for concatenating class names instead of template literals. This is especially useful for handling conditional classes and keeps the code cleaner.
To apply this, you would first need to import the utility:
import { cn } from "@/lib/utils";Then, you could update this line to:
className={cn("flex items-center gap-3 px-4 py-2.5 rounded-lg border border-border/50 bg-card/30 text-sm", className)}![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (compactEmpty && !isLoading && totalCount === 0) { | ||
| return ( | ||
| <div | ||
| className={`flex items-center gap-3 px-4 py-2.5 rounded-lg border border-border/50 bg-card/30 text-sm ${className}`} | ||
| > | ||
| <Activity className="h-4 w-4 text-muted-foreground shrink-0" /> | ||
| <span className="text-muted-foreground">{tc("activeSessions.title")}</span> | ||
| <span className="text-xs text-muted-foreground">{tc("activeSessions.empty")}</span> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Layout shift during initial load with compactEmpty
When compactEmpty is true the condition is only checked after loading completes (!isLoading). Until then the component falls through to the full card layout (with maxHeight="200px" and a loading spinner), so on initial mount there is a visible height jump from ~200 px down to ~40 px once the empty state is confirmed.
To avoid the shift you can show the compact row during loading as well:
| if (compactEmpty && !isLoading && totalCount === 0) { | |
| return ( | |
| <div | |
| className={`flex items-center gap-3 px-4 py-2.5 rounded-lg border border-border/50 bg-card/30 text-sm ${className}`} | |
| > | |
| <Activity className="h-4 w-4 text-muted-foreground shrink-0" /> | |
| <span className="text-muted-foreground">{tc("activeSessions.title")}</span> | |
| <span className="text-xs text-muted-foreground">{tc("activeSessions.empty")}</span> | |
| </div> | |
| ); | |
| } | |
| if (compactEmpty && (isLoading || totalCount === 0) && !(isLoading && totalCount > 0)) { |
Or more simply, treat compactEmpty as "always render compact when there are no sessions to display (including while loading)":
| if (compactEmpty && !isLoading && totalCount === 0) { | |
| return ( | |
| <div | |
| className={`flex items-center gap-3 px-4 py-2.5 rounded-lg border border-border/50 bg-card/30 text-sm ${className}`} | |
| > | |
| <Activity className="h-4 w-4 text-muted-foreground shrink-0" /> | |
| <span className="text-muted-foreground">{tc("activeSessions.title")}</span> | |
| <span className="text-xs text-muted-foreground">{tc("activeSessions.empty")}</span> | |
| </div> | |
| ); | |
| } | |
| if (compactEmpty && totalCount === 0) { |
This ensures the element keeps its compact height throughout the loading phase, preventing the layout jump. (The loading spinner inside the compact row is omitted in the current implementation, which is fine — the compact row is a low-profile element and a brief skeleton would be a heavier change.)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/customs/active-sessions-list.tsx
Line: 69-79
Comment:
**Layout shift during initial load with `compactEmpty`**
When `compactEmpty` is `true` the condition is only checked after loading completes (`!isLoading`). Until then the component falls through to the full card layout (with `maxHeight="200px"` and a loading spinner), so on initial mount there is a visible height jump from ~200 px down to ~40 px once the empty state is confirmed.
To avoid the shift you can show the compact row during loading as well:
```suggestion
if (compactEmpty && (isLoading || totalCount === 0) && !(isLoading && totalCount > 0)) {
```
Or more simply, treat `compactEmpty` as "always render compact when there are no sessions to display (including while loading)":
```suggestion
if (compactEmpty && totalCount === 0) {
```
This ensures the element keeps its compact height throughout the loading phase, preventing the layout jump. (The loading spinner inside the compact row is omitted in the current implementation, which is fine — the compact row is a low-profile element and a brief skeleton would be a heavier change.)
How can I resolve this? If you propose a fix, please make it concise.| const activeFilterCount = useMemo(() => { | ||
| let count = 0; | ||
| if (filters.userId !== undefined) count++; | ||
| if (filters.keyId !== undefined) count++; | ||
| if (filters.providerId !== undefined) count++; | ||
| if (filters.sessionId) count++; | ||
| if (filters.startTime && filters.endTime) count++; | ||
| if (filters.statusCode !== undefined || filters.excludeStatusCode200) count++; | ||
| if (filters.model) count++; | ||
| if (filters.endpoint) count++; | ||
| if (filters.minRetryCount !== undefined && filters.minRetryCount > 0) count++; | ||
| return count; | ||
| }, [filters]); |
There was a problem hiding this comment.
Inconsistent date-range counting vs hasStatsFilters
The activeFilterCount memo only increments by 1 when both startTime and endTime are set:
if (filters.startTime && filters.endTime) count++;However, hasStatsFilters (which controls whether the stats panel is rendered) uses Object.values(statsFilters).some(v => v !== undefined && v !== false), so it will be true if either boundary is set alone.
This means it is possible for the stats bar to appear while the filter badge still reads 0 — for example when only a startTime is provided. If the filter form prevents that state in practice this is harmless, but if partial date ranges are a valid intermediate state (e.g. during typing) the badge will mislead the user.
Consider aligning the two checks:
| const activeFilterCount = useMemo(() => { | |
| let count = 0; | |
| if (filters.userId !== undefined) count++; | |
| if (filters.keyId !== undefined) count++; | |
| if (filters.providerId !== undefined) count++; | |
| if (filters.sessionId) count++; | |
| if (filters.startTime && filters.endTime) count++; | |
| if (filters.statusCode !== undefined || filters.excludeStatusCode200) count++; | |
| if (filters.model) count++; | |
| if (filters.endpoint) count++; | |
| if (filters.minRetryCount !== undefined && filters.minRetryCount > 0) count++; | |
| return count; | |
| }, [filters]); | |
| if (filters.startTime !== undefined || filters.endTime !== undefined) count++; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsx
Line: 284-296
Comment:
**Inconsistent date-range counting vs `hasStatsFilters`**
The `activeFilterCount` memo only increments by 1 when _both_ `startTime` and `endTime` are set:
```ts
if (filters.startTime && filters.endTime) count++;
```
However, `hasStatsFilters` (which controls whether the stats panel is rendered) uses `Object.values(statsFilters).some(v => v !== undefined && v !== false)`, so it will be `true` if _either_ boundary is set alone.
This means it is possible for the stats bar to appear while the filter badge still reads 0 — for example when only a `startTime` is provided. If the filter form prevents that state in practice this is harmless, but if partial date ranges are a valid intermediate state (e.g. during typing) the badge will mislead the user.
Consider aligning the two checks:
```suggestion
if (filters.startTime !== undefined || filters.endTime !== undefined) count++;
```
How can I resolve this? If you propose a fix, please make it concise.| function Separator() { | ||
| return <span className="text-border hidden sm:inline">|</span>; | ||
| } |
There was a problem hiding this comment.
Local Separator shadows shadcn's Separator component
The file defines a private Separator function that renders a styled pipe character. The name collides with the commonly-imported shadcn/ui Separator component. While this is scoped to the file and causes no runtime issue today, future developers seeing <Separator /> here may assume it is the radix-based Separator and be confused by the apparent lack of an import.
Consider a more descriptive name such as StatsDivider or PipeDivider to make the local nature obvious.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/logs/_components/usage-logs-stats-panel.tsx
Line: 133-135
Comment:
**Local `Separator` shadows shadcn's `Separator` component**
The file defines a private `Separator` function that renders a styled pipe character. The name collides with the commonly-imported shadcn/ui `Separator` component. While this is scoped to the file and causes no runtime issue today, future developers seeing `<Separator />` here may assume it is the radix-based `Separator` and be confused by the apparent lack of an import.
Consider a more descriptive name such as `StatsDivider` or `PipeDivider` to make the local nature obvious.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR implements a clean, focused UI refactoring for the logs dashboard with no significant issues identified.
PR Size: S
- Lines changed: 322 (158 additions, 164 deletions)
- Files changed: 4
Review Assessment
Architecture & Design:
- Clean transition from card-based to inline compact layout
- Proper use of for to prevent unnecessary re-renders
- Well-implemented auto-expand logic for filter panel with proper ref-based guard
Code Quality:
- Follows CLAUDE.md guidelines
- No emoji in code
- Proper i18n usage
- Unused import () correctly removed
Logic & Correctness:
- Filter count logic correctly handles date range and status code filters
- Error states properly handled in all components
- TypeScript types properly maintained
UX Improvements:
- Compact empty state reduces visual clutter
- Collapsible filter panel with active count badge improves information density
- Auto-expand on URL filters improves discoverability
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate for UI refactoring
- Code clarity - Good
Automated review by Claude AI
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-stats-panel.tsx:
- Around line 133-135: The visual pipe in the Separator component is being
exposed to assistive tech; update the Separator function (Separator) to mark the
element as purely decorative by adding aria-hidden="true" (and optionally
role="presentation") to the <span> so screen readers ignore it while preserving
the visual separator for sighted users.
- Around line 106-118: Replace the hardcoded concatenation passed into the
StatItem detail props with single i18n keys so word order can vary per locale:
create new translation keys (e.g. "logs.stats.totalTokensDetail" and
"logs.stats.cacheTokensDetail") and pass the already formatted numbers as
interpolation parameters (formatted totalInputTokens/totalOutputTokens and
formatted totalCacheCreationTokens/totalCacheReadTokens) to those keys; update
all 5 locale files (zh-CN, zh-TW, en, ja, ru) with the corresponding
translations for those keys. Ensure the change touches the two StatItem usages
in usage-logs-stats-panel.tsx and uses the t(...) call with parameter objects
rather than building strings in JSX.
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsx:
- Around line 284-305: activeFilterCount currently only increments when both
filters.startTime and filters.endTime exist, so single-sided time filters are
ignored; update the useMemo that computes activeFilterCount (referencing
activeFilterCount, filters.startTime, filters.endTime) to increment when either
startTime or endTime is present (i.e., count += 1 if filters.startTime ||
filters.endTime), leaving the existing behavior for the both-present case
intact; this will ensure the UI badge and the filterAutoExpandRef /
setIsFiltersOpen logic trigger correctly on URL-only start or end time filters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 42d5df2b-af2b-4bbe-99ba-9e1a21c38423
📒 Files selected for processing (4)
src/app/[locale]/dashboard/logs/_components/usage-logs-sections.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-stats-panel.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsxsrc/components/customs/active-sessions-list.tsx
| <StatItem | ||
| label={t("logs.stats.totalTokens")} | ||
| value={formatTokenAmount(stats.totalTokens)} | ||
| detail={`${t("logs.stats.input")} ${formatTokenAmount(stats.totalInputTokens)} / ${t("logs.stats.output")} ${formatTokenAmount(stats.totalOutputTokens)}`} | ||
| /> | ||
|
|
||
| <Separator /> | ||
|
|
||
| <StatItem | ||
| label={t("logs.stats.cacheTokens")} | ||
| value={formatTokenAmount(stats.totalCacheCreationTokens + stats.totalCacheReadTokens)} | ||
| detail={`${t("logs.stats.write")} ${formatTokenAmount(stats.totalCacheCreationTokens)} / ${t("logs.stats.read")} ${formatTokenAmount(stats.totalCacheReadTokens)}`} | ||
| /> |
There was a problem hiding this comment.
把 token 明细收敛成单个翻译消息。
这里把 input/output、write/read 片段直接在组件里拼成整句，词序和空格就被写死了；一旦某个语言需要不同语序，这两处就没法只靠 locale 文件调整。建议把整句改成单个 i18n key，只把已经格式化好的数字作为参数传进去。
建议修改
<StatItem
label={t("logs.stats.totalTokens")}
value={formatTokenAmount(stats.totalTokens)}
- detail={`${t("logs.stats.input")} ${formatTokenAmount(stats.totalInputTokens)} / ${t("logs.stats.output")} ${formatTokenAmount(stats.totalOutputTokens)}`}
+ detail={t("logs.stats.totalTokensDetail", {
+ input: formatTokenAmount(stats.totalInputTokens),
+ output: formatTokenAmount(stats.totalOutputTokens),
+ })}
/>
@@
<StatItem
label={t("logs.stats.cacheTokens")}
value={formatTokenAmount(stats.totalCacheCreationTokens + stats.totalCacheReadTokens)}
- detail={`${t("logs.stats.write")} ${formatTokenAmount(stats.totalCacheCreationTokens)} / ${t("logs.stats.read")} ${formatTokenAmount(stats.totalCacheReadTokens)}`}
+ detail={t("logs.stats.cacheTokensDetail", {
+ write: formatTokenAmount(stats.totalCacheCreationTokens),
+ read: formatTokenAmount(stats.totalCacheReadTokens),
+ })}
/>还需要在 5 个 locale 文件里补上对应翻译键。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-stats-panel.tsx
around lines 106 - 118, Replace the hardcoded concatenation passed into the
StatItem detail props with single i18n keys so word order can vary per locale:
create new translation keys (e.g. "logs.stats.totalTokensDetail" and
"logs.stats.cacheTokensDetail") and pass the already formatted numbers as
interpolation parameters (formatted totalInputTokens/totalOutputTokens and
formatted totalCacheCreationTokens/totalCacheReadTokens) to those keys; update
all 5 locale files (zh-CN, zh-TW, en, ja, ru) with the corresponding
translations for those keys. Ensure the change touches the two StatItem usages
in usage-logs-stats-panel.tsx and uses the t(...) call with parameter objects
rather than building strings in JSX.
| function Separator() { | ||
| return <span className="text-border hidden sm:inline">|</span>; | ||
| } |
There was a problem hiding this comment.
把分隔符标记为纯装饰。
| 在这里是纯视觉分隔符，但当前桌面端会进入辅助技术树，屏幕阅读器会把它读出来，打断每组统计的朗读。给它加上 aria-hidden="true" 就够了。
建议修改
function Separator() {
- return <span className="text-border hidden sm:inline">|</span>;
+ return (
+ <span aria-hidden="true" className="text-border hidden sm:inline">
+ |
+ </span>
+ );
}📝 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.
| function Separator() { | |
| return <span className="text-border hidden sm:inline">|</span>; | |
| } | |
| function Separator() { | |
| return ( | |
| <span aria-hidden="true" className="text-border hidden sm:inline"> | |
| | | |
| </span> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-stats-panel.tsx
around lines 133 - 135, The visual pipe in the Separator component is being
exposed to assistive tech; update the Separator function (Separator) to mark the
element as purely decorative by adding aria-hidden="true" (and optionally
role="presentation") to the <span> so screen readers ignore it while preserving
the visual separator for sighted users.
| const activeFilterCount = useMemo(() => { | ||
| let count = 0; | ||
| if (filters.userId !== undefined) count++; | ||
| if (filters.keyId !== undefined) count++; | ||
| if (filters.providerId !== undefined) count++; | ||
| if (filters.sessionId) count++; | ||
| if (filters.startTime && filters.endTime) count++; | ||
| if (filters.statusCode !== undefined || filters.excludeStatusCode200) count++; | ||
| if (filters.model) count++; | ||
| if (filters.endpoint) count++; | ||
| if (filters.minRetryCount !== undefined && filters.minRetryCount > 0) count++; | ||
| return count; | ||
| }, [filters]); | ||
|
|
||
| // Auto-expand filter panel when URL-based filters are present on initial load | ||
| const filterAutoExpandRef = useRef(true); | ||
| useEffect(() => { | ||
| if (filterAutoExpandRef.current && activeFilterCount > 0) { | ||
| setIsFiltersOpen(true); | ||
| } | ||
| filterAutoExpandRef.current = false; | ||
| }, [activeFilterCount]); |
There was a problem hiding this comment.
单边时间过滤会被计成 0 个筛选条件。
Line 290 只在 startTime 和 endTime 同时存在时才加 1，但这里的过滤器类型把这两个字段定义成独立可选项。URL 里只带开始时间或结束时间时，统计面板会认为有过滤条件，折叠按钮却不显示数量，下面基于 activeFilterCount 的首次自动展开也不会触发。
建议修改
- if (filters.startTime && filters.endTime) count++;
+ if (filters.startTime !== undefined || filters.endTime !== undefined) count++;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsx
around lines 284 - 305, activeFilterCount currently only increments when both
filters.startTime and filters.endTime exist, so single-sided time filters are
ignored; update the useMemo that computes activeFilterCount (referencing
activeFilterCount, filters.startTime, filters.endTime) to increment when either
startTime or endTime is present (i.e., count += 1 if filters.startTime ||
filters.endTime), leaving the existing behavior for the both-present case
intact; this will ensure the UI badge and the filterAutoExpandRef /
setIsFiltersOpen logic trigger correctly on URL-only start or end time filters.
There was a problem hiding this comment.
Code Review Summary
This PR implements a clean, focused UI refactoring for the logs dashboard with no significant issues identified.
PR Size: S
- Lines changed: 322 (158 additions, 164 deletions)
- Files changed: 4
Review Assessment
Architecture & Design:
- Clean transition from card-based to inline compact layout
- Proper use of useMemo for statsFilters to prevent unnecessary re-renders
- Well-implemented auto-expand logic for filter panel with proper ref-based guard
Code Quality:
- Follows CLAUDE.md guidelines
- No emoji in code
- Proper i18n usage
- Unused import (cn) correctly removed
Logic & Correctness:
- Filter count logic correctly handles date range and status code filters
- Error states properly handled in all components
- TypeScript types properly maintained
UX Improvements:
- Compact empty state reduces visual clutter
- Collapsible filter panel with active count badge improves information density
- Auto-expand on URL filters improves discoverability
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate for UI refactoring
- Code clarity - Good
Automated review by Claude AI
|
Closing this PR because the current CI failures need to be recovered in one baseline-fix PR first. I have resubmitted the blocking build/test fixes as a new standalone PR so the CI baseline can be restored before smaller follow-up changes are reviewed separately. |
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
Summary
Split from PR #788 to isolate the logs page UI changes into a standalone, independently reviewable PR.
Problem
Issue #766 requested optimizing the logs page to reduce above-fold space, specifically:
Related Issues
Changes
Core Changes
This PR extracts only the logs dashboard layout changes from PR #788:
Excluded from this PR
Why Split This Way
How to Verify
Description enhanced by Claude AI
Greptile Summary
This PR refactors the logs dashboard UI into a more compact layout by collapsing the filter panel into a togglable trigger row (with an active-filter count badge), replacing the glassmorphism 4-column stats card with an inline stats bar, and showing a compact single-row empty state for the active sessions list.
Key changes:
usage-logs-view-virtualized.tsx: Filter criteria section is now aCollapsiblewith a chevron trigger; active filter count badge appears when collapsed;statsFilterswrapped inuseMemo; auto-expand logic via afilterAutoExpandReffires only on initial mount if URL filters are present.usage-logs-stats-panel.tsx: Entire glassmorphism card replaced with a slim inlineflexrow usingStatItemsub-components and a local pipeSeparator.cnimport correctly removed.active-sessions-list.tsx: NewcompactEmptyboolean prop renders a compact single-line empty state instead of the full card; all hooks are still called unconditionally (valid).usage-logs-sections.tsx: PassescompactEmptyto theActiveSessionsListused on the logs page.Issues found:
ActiveSessionsListwhencompactEmpty=truebecause the compact path is gated on!isLoading, so the component renders at full card height during the initial fetch before collapsing.activeFilterCountonly increments for a date range when bothstartTimeandendTimeare set, buthasStatsFilterschecks each independently — a partial date range will show the stats panel without incrementing the filter badge.Separatorcomponent inusage-logs-stats-panel.tsxshadows shadcn'sSeparatorin name only; no runtime impact but may confuse future maintainers.Confidence Score: 4/5
src/components/customs/active-sessions-list.tsx(layout shift withcompactEmpty) andsrc/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsx(date range counting inactiveFilterCount).Important Files Changed
statsFiltersinuseMemo, and auto-expands the panel on initial load when URL-based filters are detected. Minor inconsistency found betweenactiveFilterCountandhasStatsFiltersfor partial date ranges.cnimport correctly removed; localSeparatorcomponent has a naming collision risk with shadcn'sSeparator(cosmetic only).compactEmptyprop for a compact single-row empty state. Logic is placed after all hooks (valid). A layout shift occurs during initial load because the compact path is gated on!isLoading, causing the component to briefly render at full card height before collapsing.compactEmptyprop toActiveSessionsList. Straightforward and correct.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Logs Page Mount] --> B{URL has filters?} B -- Yes --> C[filterAutoExpandRef fires\nsetIsFiltersOpen true] B -- No --> D[isFiltersOpen = false\nFilter panel collapsed] C --> E[Filter panel expanded] D --> F{User clicks trigger row} F --> G[isFiltersOpen toggles] G --> H{activeFilterCount > 0\nand panel collapsed?} H -- Yes --> I[Show active filter Badge] H -- No --> J[No badge] E --> K{hasStatsFilters?} D --> K K -- Yes --> L[UsageLogsStatsPanel\nInline stats bar] K -- No --> M[Stats panel hidden] N[ActiveSessionsList compactEmpty=true] --> O{isLoading?} O -- true --> P[Full card layout\nLoading spinner] O -- false --> Q{totalCount === 0?} Q -- Yes --> R[Compact single-row\nempty state] Q -- No --> S[Full card with sessions]Last reviewed commit: eed4b90