Enhance UI #468
Enhance UI #468
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRemoves the Details tab and its component, updates the page Tabs layout, adds a new ContainerMetadataCard shown in Overview, extends the Container type with mounts and labels, and adds corresponding i18n keys. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Container [id] Page
participant Tabs as Tabs UI
participant Overview as OverviewTab
participant Meta as ContainerMetadataCard
participant Types as containerApi Types
User->>Page: Open container view
Page->>Tabs: Render tab list (overview, images, terminal, logs)
Note over Page,Tabs: Details tab removed
User->>Tabs: Select "Overview"
Tabs->>Overview: Render OverviewTab
Overview->>Meta: Pass container prop
Meta->>Types: Read container fields (id, image, mounts, labels, ...)
alt mounts/labels present
Meta-->>User: Render Mounts list and Label badges
else absent
Meta-->>User: Omit sections
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
view/app/containers/[id]/components/ContainerDetailsLoading.tsx (1)
17-17: Remove leftover blank lines from the Details tab removal.These blank lines appear to be formatting artifacts left over from removing the Details tab. Cleaning them up will improve code consistency.
Apply this diff to remove the extraneous whitespace:
<TabsList> <TabsTrigger value="overview">Overview</TabsTrigger> <TabsTrigger value="logs">Logs</TabsTrigger> - </TabsList></TabsContent> - </Tabs>Also applies to: 65-65
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
view/app/containers/[id]/components/ContainerDetailsLoading.tsx(2 hunks)view/app/containers/[id]/components/ContainerMetadataCard.tsx(1 hunks)view/app/containers/[id]/components/DetailsTab.tsx(0 hunks)view/app/containers/[id]/components/OverviewTab.tsx(2 hunks)view/app/containers/[id]/page.tsx(2 hunks)view/redux/services/container/containerApi.ts(1 hunks)
💤 Files with no reviewable changes (1)
- view/app/containers/[id]/components/DetailsTab.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
view/redux/services/container/containerApi.ts (1)
api/internal/features/container/types/container_types.go (1)
Container(3-17)
view/app/containers/[id]/page.tsx (2)
view/app/containers/[id]/components/OverviewTab.tsx (1)
OverviewTab(13-108)view/app/containers/[id]/components/LogsTab.tsx (1)
LogsTab(13-31)
view/app/containers/[id]/components/OverviewTab.tsx (1)
view/app/containers/[id]/components/ContainerMetadataCard.tsx (1)
ContainerMetadataCard(12-85)
view/app/containers/[id]/components/ContainerMetadataCard.tsx (1)
view/redux/services/container/containerApi.ts (1)
Container(14-35)
| {/* New Metadata Card */} | ||
| <div className="col-span-1 md:col-span-2"> | ||
| <ContainerMetadataCard container={container} /> | ||
| </div> |
There was a problem hiding this comment.
Avoid duplicating Overview content.
ContainerMetadataCard repeats Status, Created, Command, and Ports that are already rendered by the four existing cards above, so the Overview now shows the same facts twice. Please either retire the overlapping cards or trim the metadata card down to only surface the additional fields (e.g. ID, image, mounts, labels) before shipping.
@@
- {/* Status */}
- <div className="flex justify-between items-center">
- <span className="text-sm text-muted-foreground font-medium">Status</span>
- <Badge variant={container.status === 'running' ? 'default' : 'secondary'}>
- {container.status}
- </Badge>
- </div>
-
- {/* Created */}
- <div className="flex justify-between items-center">
- <span className="text-sm text-muted-foreground font-medium">Created</span>
- <span className="text-sm">
- {formatDistanceToNow(new Date(container.created), { addSuffix: true })}
- </span>
- </div>
-
@@
- {/* Ports */}
- {container?.ports?.length > 0 && (
- <div className="col-span-1 sm:col-span-2">
- <span className="text-sm text-muted-foreground font-medium">Ports</span>
- <div className="flex flex-wrap gap-2 mt-1">
- {container.ports.map((port, index) => (
- <Badge key={`${port.private_port}-${port.public_port}-${index}`} variant="outline">
- {port.public_port} → {port.private_port} ({port.type})
- </Badge>
- ))}
- </div>
- </div>
- )}
+ {/* Labels */}
+ {Object.keys(container.labels ?? {}).length > 0 && (
+ <div className="col-span-1 sm:col-span-2">
+ <span className="text-sm text-muted-foreground font-medium">
+ {t('containers.labels')}
+ </span>
+ <div className="flex flex-wrap gap-2 mt-1">
+ {Object.entries(container.labels).map(([key, value]) => (
+ <Badge key={key} variant="outline">
+ {key}: {value}
+ </Badge>
+ ))}
+ </div>
+ </div>
+ )}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In view/app/containers/[id]/components/OverviewTab.tsx around lines 101 to 104,
the new ContainerMetadataCard duplicates information shown by the four existing
cards (Status, Created, Command, Ports); remove that duplication by trimming
ContainerMetadataCard to only render additional metadata (e.g., ID, Image,
Mounts, Labels) or by removing the card entirely and moving those extra fields
into a single non-duplicative component; update ContainerMetadataCard to drop
Status/Created/Command/Ports rendering (or add props to conditionally hide them)
and ensure the Overview layout still displays the four existing cards unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
view/app/containers/[id]/components/ContainerMetadataCard.tsx (2)
5-5: Remove the unused import.The
formatDistanceToNowfunction is imported but never used in the component.Apply this diff to remove the unused import:
-import { formatDistanceToNow } from 'date-fns'; import { Container } from '@/redux/services/container/containerApi';
49-53: Consider a more stable key for mount list items.The current implementation uses the array index as the key (line 50). While acceptable for static read-only lists, consider using a composite key based on mount properties for better React reconciliation:
- {container.mounts.map((mount, idx) => ( - <li key={idx}> + {container.mounts.map((mount) => ( + <li key={`${mount.source}-${mount.destination}`}> {mount.source} → {mount.destination} </li> ))}This ensures each mount has a unique, stable identifier even if the mounts array is modified in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
view/app/containers/[id]/components/ContainerMetadataCard.tsx(1 hunks)view/lib/i18n/locales/en.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
view/app/containers/[id]/components/ContainerMetadataCard.tsx (2)
view/redux/services/container/containerApi.ts (1)
Container(14-35)view/hooks/use-translation.ts (1)
useTranslation(6-62)
🔇 Additional comments (3)
view/lib/i18n/locales/en.json (1)
85-92: LGTM! Translation keys properly support the metadata card.The new translation keys are well-structured and follow the existing pattern in the locale file. All keys correspond to labels used in the
ContainerMetadataCardcomponent.view/app/containers/[id]/components/ContainerMetadataCard.tsx (2)
7-7: Excellent! The i18n concern from the previous review has been addressed.The component now properly integrates translation support for all user-facing strings:
- Imports and uses the
useTranslationhook- All labels use
t()calls with corresponding keys from the locale file- Ensures proper localization for non-English locales
Also applies to: 14-14, 21-21, 29-29, 37-37, 46-46, 62-62
43-56: LGTM! Proper null safety for optional fields.The component correctly uses optional chaining for
mounts(line 43) and nullish coalescing forlabels(line 59), ensuring robust handling of potentially missing data. The conditional rendering prevents empty sections from displaying.Also applies to: 59-72
|
@raghavyuva , can you review it and make it merge if everything works well. |
2 participants
Summary by CodeRabbit
New Features
Refactor
Localization