From 628e5b09d110d91986fee9e299cdadbe52a03e89 Mon Sep 17 00:00:00 2001 From: isra el Date: Sun, 19 Jul 2026 00:17:16 +0300 Subject: [PATCH] feat: rework the SMS detail modal around chips and per-field copy The bottom of the modal was a label/value table, and most of it repeated the header: Direction restated the To/From already shown with its arrow icon, and Number restated the number in the title. What remained was two facts wrapped in table furniture. Those facts are now inline chips: status, device, and the gateway ID in a mono chip. The redundant rows are gone. Copy moved from a single footer button to one button per field, next to the thing it copies: the number in the header, the message body, and the gateway ID. A lone "Copy text" button left the target to be inferred, and it could only ever copy one of the three. Reused the existing shared CopyButton rather than adding another, which also brought its copied-state feedback and toast. It had no accessible name though, so every icon-only copy button was announced identically. It now names itself after what it copies, which is what lets the new test address them individually. Also overrode the dialog primitive's mobile text centring in this header: it left the title on the left and the timestamp centred beneath it. E2e reads the clipboard after each button and asserts all three return their own value. Co-Authored-By: Claude Fable 5 --- .../message-history/sms-details-dialog.tsx | 121 +++++++++--------- web/components/shared/copy-button.tsx | 61 +++++---- web/e2e/message-history.spec.ts | 46 +++++++ 3 files changed, 138 insertions(+), 90 deletions(-) diff --git a/web/app/(app)/dashboard/(components)/message-history/sms-details-dialog.tsx b/web/app/(app)/dashboard/(components)/message-history/sms-details-dialog.tsx index df96a7a..63cdd04 100644 --- a/web/app/(app)/dashboard/(components)/message-history/sms-details-dialog.tsx +++ b/web/app/(app)/dashboard/(components)/message-history/sms-details-dialog.tsx @@ -1,6 +1,6 @@ 'use client' -import { useState, type ReactNode } from 'react' +import { useState } from 'react' import { Dialog, DialogContent, @@ -9,14 +9,8 @@ import { DialogTitle, } from '@/components/ui/dialog' import { Button } from '@/components/ui/button' -import { - ArrowDownLeft, - ArrowUpRight, - Copy, - MessageSquare, - Reply, -} from 'lucide-react' -import { toast } from '@/hooks/use-toast' +import { ArrowDownLeft, ArrowUpRight, MessageSquare, Reply, Smartphone } from 'lucide-react' +import { CopyButton } from '@/components/shared/copy-button' import { getStatusBadge } from './utils' import { messageDate, messageDirection } from './group' import { toExactLabel } from '@/components/shared/relative-time' @@ -34,17 +28,9 @@ type SmsDetailsDialogProps = { fallbackDeviceId?: string } -function Row({ label, children }: { label: string; children: ReactNode }) { - return ( -
-
{label}
-
{children}
-
- ) -} - // Ordered by what people open this for: the message itself first, then the -// metadata that explains it. +// few facts that explain it. Copy actions sit next to the thing they copy, +// rather than a single footer button whose target had to be inferred. export default function SmsDetailsDialog({ message, open, @@ -61,19 +47,17 @@ export default function SmsDetailsDialog({ : message.sender || 'Unknown' const date = messageDate(message) const composerDeviceId = message.device?._id || fallbackDeviceId - - const handleCopyMessage = () => { - if (message?.message) { - navigator.clipboard.writeText(message.message) - toast({ title: 'Message copied to clipboard!' }) - } - } + const deviceName = [message.device?.brand, message.device?.model] + .filter(Boolean) + .join(' ') return ( <> - - + + {/* text-left overrides the primitive's mobile centring, which left + the title on the left and the date centred under it. */} + {isSent ? 'To' : 'From'} {counterparty} + {/* Exact time as text: the hover tooltip in the list is not reachable on touch. */} @@ -102,40 +91,55 @@ export default function SmsDetailsDialog({ {/* The message body leads: it is the reason this dialog is open. */} -
- {message.message} +
+
+ {message.message} +
+
-
- - - {statusBadge.icon} - {statusBadge.label} + {/* Facts as chips rather than a label/value table: direction and + number were already in the header, so the table mostly repeated + itself. */} +
+ + {statusBadge.icon} + {statusBadge.label} + + + {deviceName && ( + + + {deviceName} - - {isSent ? 'Sent' : 'Received'} - {counterparty} - - {message.device?.brand || message.device?.model - ? `${message.device?.brand ?? ''} ${message.device?.model ?? ''}`.trim() - : 'Not recorded'} - + )} + {message.gatewayMessageId && ( - - + + ID + {message.gatewayMessageId} - + + )} -
+
{(message.errorCode || message.errorMessage) && ( -
+

Delivery failed

@@ -152,16 +156,7 @@ export default function SmsDetailsDialog({
)} -
- +
{isSent ? ( - ); -} \ No newline at end of file + ) +} diff --git a/web/e2e/message-history.spec.ts b/web/e2e/message-history.spec.ts index cc14f04..2724274 100644 --- a/web/e2e/message-history.spec.ts +++ b/web/e2e/message-history.spec.ts @@ -187,6 +187,52 @@ test.describe('message history (mocked API, no real backend)', () => { await expect(dialog.getByText(/\d{4} at \d{1,2}:\d{2}/)).toBeVisible() }) + test('each detail field copies its own value', async ({ page, context }) => { + await authenticate(context) + await mockApi(page) + await context.grantPermissions(['clipboard-read', 'clipboard-write']) + + await page.route('**/api/v1/gateway/devices/*/messages*', (route) => + route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + data: [ + { + _id: 'copy_1', + recipient: '+15557654321', + message: 'Copy me exactly', + status: 'delivered', + type: 'sent', + gatewayMessageId: '665f1c2a9b1e4a0012ab34cd', + device: { _id: 'device_1', brand: 'Google', model: 'Pixel 8' }, + requestedAt: new Date().toISOString(), + createdAt: new Date().toISOString(), + }, + ], + meta: { total: 1, page: 1, limit: 20, totalPages: 1 }, + }), + }) + ) + + await page.goto('/dashboard/messaging/history') + await page.getByRole('button', { name: /Copy me exactly/ }).click() + + const dialog = page.getByRole('dialog') + const read = () => page.evaluate(() => navigator.clipboard.readText()) + + // One button per field, each named for what it copies, replacing a single + // footer "Copy text" whose target had to be inferred. + await dialog.getByRole('button', { name: 'Copy number' }).click() + expect(await read()).toBe('+15557654321') + + await dialog.getByRole('button', { name: 'Copy message' }).click() + expect(await read()).toBe('Copy me exactly') + + await dialog.getByRole('button', { name: 'Copy gateway ID' }).click() + expect(await read()).toBe('665f1c2a9b1e4a0012ab34cd') + }) + test('replying from a received message keeps a device selected', async ({ page, context,