From 7dc281a978ab76e3ead08d7bb4efd27ea1d7c5d8 Mon Sep 17 00:00:00 2001 From: Jason Chiu Date: Wed, 15 Apr 2026 19:00:59 -0700 Subject: [PATCH 1/2] feat: extract shared object detail fields and fix ResourcePicker height clipping - Extract buildObjectDetailFields() into objectService.ts as shared utility - Refactor ObjectDetailScreen to use shared utility, reducing code duplication - Add additionalOverhead prop to ResourcePicker for tabbed wrapper height calculation - Add objectService tests (14 tests) Co-Authored-By: Claude Opus 4.6 --- src/components/ResourcePicker.tsx | 9 +- src/screens/ObjectDetailScreen.tsx | 66 +------------ src/services/objectService.ts | 52 +++++++++++ .../__tests__/services/objectService.test.ts | 93 +++++++++++++++++++ 4 files changed, 156 insertions(+), 64 deletions(-) create mode 100644 tests/__tests__/services/objectService.test.ts diff --git a/src/components/ResourcePicker.tsx b/src/components/ResourcePicker.tsx index 519193ae..48a660d6 100644 --- a/src/components/ResourcePicker.tsx +++ b/src/components/ResourcePicker.tsx @@ -82,6 +82,9 @@ export interface ResourcePickerConfig { /** Label for the create new action (default: "Create new") */ createNewLabel?: string; + + /** Additional lines of overhead from wrapper components (e.g., tab headers) */ + additionalOverhead?: number; } export interface ResourcePickerProps { @@ -134,7 +137,11 @@ export function ResourcePicker({ // Calculate overhead for viewport height // Matches list pages: breadcrumb(4) + table chrome(4) + stats(2) + nav tips(2) + buffer(1) = 13 - const overhead = 13 + search.getSearchOverhead() + extraOverhead; + const overhead = + 13 + + search.getSearchOverhead() + + extraOverhead + + (config.additionalOverhead || 0); const { viewportHeight, terminalWidth } = useViewportHeight({ overhead, minHeight: 5, diff --git a/src/screens/ObjectDetailScreen.tsx b/src/screens/ObjectDetailScreen.tsx index 06163944..3f9b22e6 100644 --- a/src/screens/ObjectDetailScreen.tsx +++ b/src/screens/ObjectDetailScreen.tsx @@ -15,13 +15,13 @@ import { import { getClient } from "../utils/client.js"; import { ResourceDetailPage, - formatTimestamp, type DetailSection, type ResourceOperation, } from "../components/ResourceDetailPage.js"; import { getObject, deleteObject, + buildObjectDetailFields, formatFileSize, } from "../services/objectService.js"; import { useResourceDetail } from "../hooks/useResourceDetail.js"; @@ -175,68 +175,8 @@ export function ObjectDetailScreen({ objectId }: ObjectDetailScreenProps) { // Build detail sections const detailSections: DetailSection[] = []; - // Basic details section - const basicFields = []; - if (storageObject.content_type) { - basicFields.push({ - label: "Content Type", - value: storageObject.content_type, - }); - } - if (storageObject.size_bytes !== undefined) { - basicFields.push({ - label: "Size", - value: formatFileSize(storageObject.size_bytes), - }); - } - if (storageObject.state) { - basicFields.push({ - label: "State", - value: storageObject.state, - }); - } - if (storageObject.is_public !== undefined) { - basicFields.push({ - label: "Public", - value: storageObject.is_public ? "Yes" : "No", - }); - } - if (storageObject.create_time_ms) { - basicFields.push({ - label: "Created", - value: formatTimestamp(storageObject.create_time_ms), - }); - } - - // TTL / Expires - show remaining time before auto-deletion - if (storageObject.delete_after_time_ms) { - const now = Date.now(); - const remainingMs = storageObject.delete_after_time_ms - now; - - let ttlValue: string; - let ttlColor = colors.text; - - if (remainingMs <= 0) { - ttlValue = "Expired"; - ttlColor = colors.error; - } else { - const remainingMinutes = Math.floor(remainingMs / 60000); - if (remainingMinutes < 60) { - ttlValue = `${remainingMinutes}m remaining`; - ttlColor = remainingMinutes < 10 ? colors.warning : colors.text; - } else { - const hours = Math.floor(remainingMinutes / 60); - const mins = remainingMinutes % 60; - ttlValue = `${hours}h ${mins}m remaining`; - } - } - - basicFields.push({ - label: "Expires", - value: {ttlValue}, - }); - } - + // Basic details section — reuse shared field builder + const basicFields = buildObjectDetailFields(storageObject); if (basicFields.length > 0) { detailSections.push({ title: "Details", diff --git a/src/services/objectService.ts b/src/services/objectService.ts index b4c2647b..8093c3c0 100644 --- a/src/services/objectService.ts +++ b/src/services/objectService.ts @@ -118,6 +118,58 @@ export async function deleteObject(id: string): Promise { await client.objects.delete(id); } +/** + * Build standard detail fields for a storage object. + * Shared between ObjectDetailScreen and AgentDetailScreen. + */ +export function buildObjectDetailFields( + obj: StorageObjectView, +): { label: string; value: string }[] { + const fields: { label: string; value: string }[] = []; + + if (obj.content_type) { + fields.push({ label: "Content Type", value: obj.content_type }); + } + if (obj.size_bytes !== undefined && obj.size_bytes !== null) { + fields.push({ label: "Size", value: formatFileSize(obj.size_bytes) }); + } + if (obj.state) { + fields.push({ label: "State", value: obj.state }); + } + if (obj.is_public !== undefined) { + fields.push({ label: "Public", value: obj.is_public ? "Yes" : "No" }); + } + if (obj.create_time_ms) { + fields.push({ + label: "Created", + value: new Date(obj.create_time_ms).toLocaleString(), + }); + } + if (obj.delete_after_time_ms) { + const remainingMs = obj.delete_after_time_ms - Date.now(); + if (remainingMs <= 0) { + fields.push({ label: "Expires", value: "Expired" }); + } else { + const remainingMinutes = Math.floor(remainingMs / 60000); + if (remainingMinutes < 60) { + fields.push({ + label: "Expires", + value: `${remainingMinutes}m remaining`, + }); + } else { + const hours = Math.floor(remainingMinutes / 60); + const mins = remainingMinutes % 60; + fields.push({ + label: "Expires", + value: `${hours}h ${mins}m remaining`, + }); + } + } + } + + return fields; +} + /** * Format file size in human-readable format */ diff --git a/tests/__tests__/services/objectService.test.ts b/tests/__tests__/services/objectService.test.ts new file mode 100644 index 00000000..07e4aff3 --- /dev/null +++ b/tests/__tests__/services/objectService.test.ts @@ -0,0 +1,93 @@ +import { + formatFileSize, + buildObjectDetailFields, +} from "../../../src/services/objectService.js"; +import type { StorageObjectView } from "../../../src/store/objectStore.js"; + +describe("formatFileSize", () => { + it("returns Unknown for null", () => { + expect(formatFileSize(null)).toBe("Unknown"); + }); + + it("returns Unknown for undefined", () => { + expect(formatFileSize(undefined)).toBe("Unknown"); + }); + + it("formats bytes", () => { + expect(formatFileSize(500)).toBe("500 B"); + }); + + it("formats kilobytes", () => { + expect(formatFileSize(1024)).toBe("1.00 KB"); + }); + + it("formats megabytes", () => { + expect(formatFileSize(1024 * 1024)).toBe("1.00 MB"); + }); + + it("formats gigabytes", () => { + expect(formatFileSize(1024 * 1024 * 1024)).toBe("1.00 GB"); + }); + + it("formats zero bytes", () => { + expect(formatFileSize(0)).toBe("0 B"); + }); +}); + +describe("buildObjectDetailFields", () => { + const baseObject: StorageObjectView = { + id: "obj_123", + name: "test.txt", + content_type: "text/plain", + create_time_ms: 1700000000000, + state: "READY", + size_bytes: 1024, + }; + + it("includes content type", () => { + const fields = buildObjectDetailFields(baseObject); + expect(fields.find((f) => f.label === "Content Type")?.value).toBe( + "text/plain", + ); + }); + + it("includes formatted size", () => { + const fields = buildObjectDetailFields(baseObject); + expect(fields.find((f) => f.label === "Size")?.value).toBe("1.00 KB"); + }); + + it("includes state", () => { + const fields = buildObjectDetailFields(baseObject); + expect(fields.find((f) => f.label === "State")?.value).toBe("READY"); + }); + + it("includes public field when set", () => { + const fields = buildObjectDetailFields({ ...baseObject, is_public: true }); + expect(fields.find((f) => f.label === "Public")?.value).toBe("Yes"); + }); + + it("includes created timestamp", () => { + const fields = buildObjectDetailFields(baseObject); + expect(fields.find((f) => f.label === "Created")).toBeDefined(); + }); + + it("includes expires when delete_after_time_ms is set", () => { + const future = Date.now() + 3600000; // 1 hour from now + const fields = buildObjectDetailFields({ + ...baseObject, + delete_after_time_ms: future, + }); + const expiresField = fields.find((f) => f.label === "Expires"); + expect(expiresField).toBeDefined(); + expect(expiresField?.value).toContain("remaining"); + }); + + it("shows Expired for past delete_after_time_ms", () => { + const past = Date.now() - 1000; + const fields = buildObjectDetailFields({ + ...baseObject, + delete_after_time_ms: past, + }); + expect(fields.find((f) => f.label === "Expires")?.value).toBe("Expired"); + }); +}); From 14c09a1ee63d57fbdc9d1ce994f386ba00cda4e8 Mon Sep 17 00:00:00 2001 From: Jason Chiu Date: Wed, 22 Apr 2026 12:08:44 -0700 Subject: [PATCH 2/2] fix: restore TTL coloring and timestamp format in buildObjectDetailFields Add color field to ObjectDetailField for semantic coloring (error/warning) on expiry values. Use formatTimestamp for Created field to include relative time. Map color names to theme values in ObjectDetailScreen. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/screens/ObjectDetailScreen.tsx | 9 ++++++++- src/services/objectService.ts | 16 ++++++++++++---- tests/__tests__/services/objectService.test.ts | 16 ++++++++++++++-- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/screens/ObjectDetailScreen.tsx b/src/screens/ObjectDetailScreen.tsx index 3f9b22e6..cb91bd4c 100644 --- a/src/screens/ObjectDetailScreen.tsx +++ b/src/screens/ObjectDetailScreen.tsx @@ -176,7 +176,14 @@ export function ObjectDetailScreen({ objectId }: ObjectDetailScreenProps) { const detailSections: DetailSection[] = []; // Basic details section — reuse shared field builder - const basicFields = buildObjectDetailFields(storageObject); + const colorMap: Record = { + error: colors.error, + warning: colors.warning, + }; + const basicFields = buildObjectDetailFields(storageObject).map((f) => ({ + ...f, + color: f.color ? (colorMap[f.color] ?? f.color) : undefined, + })); if (basicFields.length > 0) { detailSections.push({ title: "Details", diff --git a/src/services/objectService.ts b/src/services/objectService.ts index 8093c3c0..656331c0 100644 --- a/src/services/objectService.ts +++ b/src/services/objectService.ts @@ -2,6 +2,7 @@ * Object Service - Handles all storage object API calls */ import { getClient } from "../utils/client.js"; +import { formatTimestamp } from "../utils/time.js"; import type { StorageObjectView } from "../store/objectStore.js"; export interface ListObjectsOptions { @@ -118,14 +119,20 @@ export async function deleteObject(id: string): Promise { await client.objects.delete(id); } +export interface ObjectDetailField { + label: string; + value: string; + color?: string; +} + /** * Build standard detail fields for a storage object. * Shared between ObjectDetailScreen and AgentDetailScreen. */ export function buildObjectDetailFields( obj: StorageObjectView, -): { label: string; value: string }[] { - const fields: { label: string; value: string }[] = []; +): ObjectDetailField[] { + const fields: ObjectDetailField[] = []; if (obj.content_type) { fields.push({ label: "Content Type", value: obj.content_type }); @@ -142,19 +149,20 @@ export function buildObjectDetailFields( if (obj.create_time_ms) { fields.push({ label: "Created", - value: new Date(obj.create_time_ms).toLocaleString(), + value: formatTimestamp(obj.create_time_ms) ?? "", }); } if (obj.delete_after_time_ms) { const remainingMs = obj.delete_after_time_ms - Date.now(); if (remainingMs <= 0) { - fields.push({ label: "Expires", value: "Expired" }); + fields.push({ label: "Expires", value: "Expired", color: "error" }); } else { const remainingMinutes = Math.floor(remainingMs / 60000); if (remainingMinutes < 60) { fields.push({ label: "Expires", value: `${remainingMinutes}m remaining`, + color: remainingMinutes < 10 ? "warning" : undefined, }); } else { const hours = Math.floor(remainingMinutes / 60); diff --git a/tests/__tests__/services/objectService.test.ts b/tests/__tests__/services/objectService.test.ts index 07e4aff3..e17a8860 100644 --- a/tests/__tests__/services/objectService.test.ts +++ b/tests/__tests__/services/objectService.test.ts @@ -82,12 +82,24 @@ describe("buildObjectDetailFields", () => { expect(expiresField?.value).toContain("remaining"); }); - it("shows Expired for past delete_after_time_ms", () => { + it("shows Expired with error color for past delete_after_time_ms", () => { const past = Date.now() - 1000; const fields = buildObjectDetailFields({ ...baseObject, delete_after_time_ms: past, }); - expect(fields.find((f) => f.label === "Expires")?.value).toBe("Expired"); + const expiresField = fields.find((f) => f.label === "Expires"); + expect(expiresField?.value).toBe("Expired"); + expect(expiresField?.color).toBe("error"); + }); + + it("shows warning color when expiry is under 10 minutes", () => { + const soon = Date.now() + 5 * 60000; // 5 minutes from now + const fields = buildObjectDetailFields({ + ...baseObject, + delete_after_time_ms: soon, + }); + const expiresField = fields.find((f) => f.label === "Expires"); + expect(expiresField?.color).toBe("warning"); }); });