Refactoring Agent
454195cdfb
Build and Publish Mana Loop Docker Image / build-and-publish (push) Successful in 3m16s
283 lines
9.7 KiB
Markdown
283 lines
9.7 KiB
Markdown
# Task 13 Context: Fix LootTab Nesting (Remove Redundant Layers)
|
|
|
|
## Task Description
|
|
Fix LootTab nesting (remove redundant layers) (PRIORITY 3b)
|
|
|
|
## Source Files
|
|
|
|
### 1. `/src/components/game/tabs/LootTab.tsx` (48 lines)
|
|
|
|
**Full Content:**
|
|
```typescript
|
|
'use client';
|
|
|
|
import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card';
|
|
import { Badge } from '@/components/ui/badge';
|
|
import type { GameStore } from '@/lib/game/store';
|
|
import { LootInventoryDisplay } from '@/components/game/LootInventory';
|
|
|
|
export interface LootTabProps {
|
|
store: GameStore;
|
|
}
|
|
|
|
export function LootTab({ store }: LootTabProps) {
|
|
const inventory = store.lootInventory;
|
|
const elements = store.elements;
|
|
const equipmentInstances = store.equipmentInstances;
|
|
|
|
// Count items for badge
|
|
const materialCount = Object.values(inventory.materials).reduce((a, b) => a + b, 0);
|
|
const blueprintCount = inventory.blueprints.length;
|
|
const equipmentCount = Object.keys(equipmentInstances).length;
|
|
const totalItems = materialCount + blueprintCount + equipmentCount;
|
|
|
|
return (
|
|
<div className="space-y-4">
|
|
<Card className="bg-gray-900/80 border-gray-700">
|
|
<CardHeader className="pb-2">
|
|
<CardTitle className="text-amber-400 text-sm flex items-center gap-2">
|
|
💎 Loot Inventory
|
|
<Badge className="ml-auto bg-gray-800 text-gray-300">
|
|
{totalItems} items
|
|
</Badge>
|
|
</CardTitle>
|
|
</CardHeader>
|
|
<CardContent>
|
|
<LootInventoryDisplay
|
|
inventory={inventory}
|
|
elements={elements}
|
|
equipmentInstances={equipmentInstances}
|
|
onDeleteMaterial={store.deleteMaterial}
|
|
onDeleteEquipment={store.deleteEquipmentInstance}
|
|
/>
|
|
</CardContent>
|
|
</Card>
|
|
</div>
|
|
);
|
|
}
|
|
|
|
LootTab.displayName = "LootTab";
|
|
```
|
|
|
|
**Key Observations - LootTab Redundant Wrapper:**
|
|
- Uses `Card` component from `@/components/ui/card` with header "💎 Loot Inventory"
|
|
- Shows a badge with total items count
|
|
- Wraps `LootInventoryDisplay` inside `CardContent`
|
|
- **This creates the outer layer of nesting**
|
|
|
|
---
|
|
|
|
### 2. `/src/components/game/LootInventory.tsx` (499 lines)
|
|
|
|
**Component Interface:**
|
|
```typescript
|
|
interface LootInventoryProps {
|
|
inventory: LootInventoryType;
|
|
elements?: Record<string, ElementState>;
|
|
equipmentInstances?: Record<string, EquipmentInstance>;
|
|
onDeleteMaterial?: (materialId: string, amount: number) => void;
|
|
onDeleteEquipment?: (instanceId: string) => void;
|
|
}
|
|
```
|
|
|
|
**Main Component Export:**
|
|
```typescript
|
|
export function LootInventoryDisplay({
|
|
inventory,
|
|
elements,
|
|
equipmentInstances = {},
|
|
onDeleteMaterial,
|
|
onDeleteEquipment,
|
|
}: LootInventoryProps) {
|
|
// ... state and handlers ...
|
|
|
|
// Check if we have anything to show
|
|
const hasItems = totalItems > 0 || essenceCount > 0;
|
|
|
|
if (!hasItems) {
|
|
return (
|
|
<GameCard variant="default" className="w-full">
|
|
<div className="flex items-center gap-2 mb-2">
|
|
<Gem className="w-4 h-4 text-[var(--mana-light)]" />
|
|
<h3 className="text-[var(--mana-light)] game-panel-title text-xs uppercase tracking-wider">
|
|
Inventory
|
|
</h3>
|
|
</div>
|
|
<div className="text-[var(--text-muted)] text-sm text-center py-4">
|
|
No items collected yet. Defeat floors and guardians to find loot!
|
|
</div>
|
|
</GameCard>
|
|
);
|
|
}
|
|
|
|
// ... handlers ...
|
|
|
|
return (
|
|
<>
|
|
<GameCard variant="default" className="w-full">
|
|
<div className="flex items-center gap-2 mb-3">
|
|
<Gem className="w-4 h-4 text-[var(--mana-light)]" />
|
|
<h3 className="text-[var(--mana-light)] game-panel-title text-xs uppercase tracking-wider">
|
|
Inventory
|
|
</h3>
|
|
<Badge
|
|
className="ml-auto bg-[var(--bg-sunken)] text-[var(--text-secondary)] text-xs border-[var(--border-subtle)]"
|
|
aria-label={`${totalItems} items in inventory`}
|
|
>
|
|
{totalItems} items
|
|
</Badge>
|
|
</div>
|
|
|
|
{/* Search and Filter Controls */}
|
|
{/* ... */}
|
|
|
|
{/* Filter Tabs */}
|
|
{/* ... */}
|
|
|
|
<Separator className="bg-[var(--border-subtle)] mb-3" />
|
|
|
|
<ScrollArea className="h-64 w-full">
|
|
{/* Materials, Essence, Blueprints, Equipment sections */}
|
|
{/* ... */}
|
|
</ScrollArea>
|
|
</GameCard>
|
|
|
|
{/* Delete Confirmation Dialog */}
|
|
<AlertDialog open={!!deleteConfirm} onOpenChange={() => setDeleteConfirm(null)}>
|
|
{/* ... */}
|
|
</AlertDialog>
|
|
</>
|
|
);
|
|
}
|
|
```
|
|
|
|
**Key Observations - LootInventory Redundant Wrapper:**
|
|
- Uses `GameCard` component (from `@/components/ui/game-card`)
|
|
- Has its own header with "Inventory" title and `Gem` icon
|
|
- Shows a badge with total items count (duplicating LootTab's badge)
|
|
- Contains all the actual content: search, filters, items display
|
|
- **This creates the inner layer of nesting**
|
|
|
|
---
|
|
|
|
## 3. Duplicate Headings/Wrappers Issue
|
|
|
|
### Redundant Card Nesting:
|
|
```
|
|
LootTab (Outer Card)
|
|
├── CardHeader: "💎 Loot Inventory" + Badge: "{totalItems} items"
|
|
└── CardContent
|
|
└── LootInventoryDisplay (Inner GameCard)
|
|
├── Header: "Inventory" + Badge: "{totalItems} items" ← DUPLICATE
|
|
├── Search/Filter Controls
|
|
├── Items Display
|
|
└── Delete Dialog
|
|
```
|
|
|
|
### Specific Code Duplication:
|
|
|
|
**LootTab.tsx (lines 24-33) - Outer Header:**
|
|
```tsx
|
|
<CardHeader className="pb-2">
|
|
<CardTitle className="text-amber-400 text-sm flex items-center gap-2">
|
|
💎 Loot Inventory
|
|
<Badge className="ml-auto bg-gray-800 text-gray-300">
|
|
{totalItems} items
|
|
</Badge>
|
|
</CardTitle>
|
|
</CardHeader>
|
|
```
|
|
|
|
**LootInventory.tsx (lines 191-202) - Inner Header (DUPLICATE):**
|
|
```tsx
|
|
<div className="flex items-center gap-2 mb-3">
|
|
<Gem className="w-4 h-4 text-[var(--mana-light)]" />
|
|
<h3 className="text-[var(--mana-light)] game-panel-title text-xs uppercase tracking-wider">
|
|
Inventory
|
|
</h3>
|
|
<Badge
|
|
className="ml-auto bg-[var(--bg-sunken)] text-[var(--text-secondary)] text-xs border-[var(--border-subtle)]"
|
|
aria-label={`${totalItems} items in inventory`}
|
|
>
|
|
{totalItems} items
|
|
</Badge>
|
|
</div>
|
|
```
|
|
|
|
### Redundant Badge Count:
|
|
- LootTab shows: `{totalItems} items`
|
|
- LootInventoryDisplay also shows: `{totalItems} items`
|
|
- Both calculate the same `totalItems` value
|
|
|
|
---
|
|
|
|
## 4. Current Component Hierarchy
|
|
|
|
```
|
|
Game.tsx / Main Game Layout
|
|
│
|
|
└── Tabs Component (renders active tab)
|
|
│
|
|
└── LootTab ({ store })
|
|
│
|
|
├── Card (bg-gray-900/80 border-gray-700)
|
|
│ ├── CardHeader
|
|
│ │ └── CardTitle: "💎 Loot Inventory" + Badge
|
|
│ └── CardContent
|
|
│ │
|
|
│ └── LootInventoryDisplay ({ inventory, elements, equipmentInstances, ... })
|
|
│ │
|
|
│ ├── GameCard (variant="default" className="w-full")
|
|
│ │ ├── Header: "Inventory" + Badge + Search/Filter
|
|
│ │ ├── Separator
|
|
│ │ ├── ScrollArea
|
|
│ │ │ ├── Materials Section
|
|
│ │ │ ├── Essence Section
|
|
│ │ │ ├── Blueprints Section
|
|
│ │ │ └── Equipment Section
|
|
│ │ └── (content)
|
|
│ │
|
|
│ └── AlertDialog (Delete Confirmation)
|
|
│
|
|
└── (end Card)
|
|
```
|
|
|
|
---
|
|
|
|
## 5. Comparison with Other Tabs
|
|
|
|
Looking at other tabs in `/src/components/game/tabs/`:
|
|
|
|
- **EquipmentTab.tsx**: Uses `GameCard` directly without an outer `Card` wrapper
|
|
- **CraftingTab.tsx**: Uses multiple `GameCard` components for different sections, no outer `Card`
|
|
- **GolemancyTab.tsx**: Uses `GameCard` directly
|
|
- **LabTab.tsx**: Uses `GameCard` directly
|
|
|
|
**Pattern**: Most tabs render their content directly using `GameCard` components without an additional `Card` wrapper from the tab itself.
|
|
|
|
---
|
|
|
|
## 6. Summary of Issues to Fix
|
|
|
|
1. **Redundant Card Wrapper in LootTab**: The `Card` + `CardHeader` + `CardContent` wrapper in LootTab.tsx is unnecessary since LootInventoryDisplay already provides its own `GameCard` wrapper.
|
|
|
|
2. **Duplicate Header**: Both LootTab and LootInventoryDisplay show similar headers with:
|
|
- Title text ("Loot Inventory" vs "Inventory")
|
|
- Item count badge
|
|
- Icon (emoji 💎 vs Gem icon)
|
|
|
|
3. **Double Wrapping**: Content is wrapped in two card-like components:
|
|
- Outer: `Card` from `@/components/ui/card`
|
|
- Inner: `GameCard` from `@/components/ui/game-card`
|
|
|
|
4. **Solution Direction**: Remove the outer `Card` wrapper from LootTab.tsx and let LootInventoryDisplay handle the card styling, OR remove the inner `GameCard` from LootInventoryDisplay and keep only the outer wrapper.
|
|
|
|
---
|
|
|
|
## 7. Files That Would Need Modification
|
|
|
|
1. **`/src/components/game/tabs/LootTab.tsx`** - Remove outer Card wrapper, pass props directly to LootInventoryDisplay
|
|
2. **`/src/components/game/LootInventory.tsx`** - Potentially remove GameCard wrapper if keeping LootTab's Card, or keep as-is if removing LootTab's Card
|
|
|
|
**Recommended Approach**: Remove the outer `Card` wrapper from `LootTab.tsx` and let `LootInventoryDisplay` handle the full display (since it already has a complete GameCard wrapper with header, controls, and content).
|