From 8726ab60b6f98770c14361bddc6900ec2dec5add Mon Sep 17 00:00:00 2001
From: beatz174-bit <beatz174@gmail.com>
Date: Mon, 24 Nov 2025 10:02:28 +1000
Subject: [PATCH] Improve pick item delete confirmation flow

---
 e2e/picklist.spec.ts                | 36 +++++++++++++++++++++--------
 src/components/PickItemRow.test.tsx | 21 +++++++++++++++--
 src/components/PickItemRow.tsx      | 18 +++++++--------
 3 files changed, 55 insertions(+), 20 deletions(-)

diff --git a/e2e/picklist.spec.ts b/e2e/picklist.spec.ts
index 046f7fd..2d6b72e 100644
--- a/e2e/picklist.spec.ts
+++ b/e2e/picklist.spec.ts
@@ -299,20 +299,38 @@ test.describe('Active pick list', () => {
   });
 
   test('removes a product from the pick list when deleted', async ({ page }) => {
+    await page.setViewportSize({ width: 430, height: 932 });
     await navigateToNewPickList(page);
     await addProductToPickList(page, additionalProduct);
 
-    const productRowDeleteButton = page
-      .getByText(additionalProduct)
-      .locator('xpath=ancestor::div[contains(@class, "MuiStack-root")]//button[@aria-label="Delete item"]')
-      .first();
+    const productRow = page.getByText(additionalProduct).first();
 
-    await expect(page.getByText(additionalProduct).first()).toBeVisible();
-    await productRowDeleteButton.click();
+    await expect(productRow).toBeVisible();
+    await productRow.click();
 
-    await expect(page.getByRole('dialog', { name: 'Delete item' })).toBeVisible();
-    await page.getByRole('button', { name: 'Delete', exact: true }).click();
+    const controlsDialog = page.getByRole('dialog', { name: additionalProduct });
+    await expect(controlsDialog).toBeVisible();
 
-    await expect(page.getByText(additionalProduct).first()).not.toBeVisible();
+    await controlsDialog.getByRole('button', { name: 'Delete item' }).click();
+
+    const confirmDialog = page.getByRole('dialog', { name: 'Delete item' });
+    await expect(confirmDialog).toBeVisible();
+
+    const cancelButton = confirmDialog.getByRole('button', { name: 'Cancel' });
+    await expect(cancelButton).toBeVisible();
+    await cancelButton.click();
+
+    await expect(confirmDialog).not.toBeVisible();
+    await expect(controlsDialog).not.toBeVisible();
+
+    await productRow.click();
+    await expect(page.getByRole('dialog', { name: additionalProduct })).toBeVisible();
+    await page.getByRole('button', { name: 'Delete item' }).click();
+
+    const deleteButton = page.getByRole('button', { name: 'Delete', exact: true });
+    await expect(deleteButton).toBeVisible();
+    await deleteButton.click();
+
+    await expect(page.getByText(additionalProduct)).toHaveCount(0);
   });
 });
diff --git a/src/components/PickItemRow.test.tsx b/src/components/PickItemRow.test.tsx
index 59e2020..94e6450 100644
--- a/src/components/PickItemRow.test.tsx
+++ b/src/components/PickItemRow.test.tsx
@@ -1,10 +1,23 @@
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
-import { describe, expect, it, vi } from 'vitest';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { PickItemRow } from './PickItemRow';
 import { PickItem } from '../models/PickItem';
 import { Product } from '../models/Product';
 
+const mockMatchMedia = (matches: boolean) => {
+  window.matchMedia = vi.fn().mockImplementation((query) => ({
+    matches,
+    media: query,
+    onchange: null,
+    addListener: vi.fn(),
+    removeListener: vi.fn(),
+    addEventListener: vi.fn(),
+    removeEventListener: vi.fn(),
+    dispatchEvent: vi.fn(),
+  }));
+};
+
 const baseItem: PickItem = {
   id: 'item-1',
   pick_list_id: 'list-1',
@@ -29,6 +42,10 @@ const baseProduct: Product = {
 };
 
 describe('PickItemRow', () => {
+  beforeEach(() => {
+    mockMatchMedia(false);
+  });
+
   it('asks for confirmation before deleting a product from the pick list', async () => {
     const onDelete = vi.fn();
     const user = userEvent.setup();
@@ -52,7 +69,7 @@ describe('PickItemRow', () => {
     ).toBeVisible();
     expect(onDelete).not.toHaveBeenCalled();
 
-    await user.click(screen.getByRole('button', { name: /confirm delete/i }));
+    await user.click(screen.getByRole('button', { name: /^delete$/i }));
 
     expect(onDelete).toHaveBeenCalledTimes(1);
   });
diff --git a/src/components/PickItemRow.tsx b/src/components/PickItemRow.tsx
index cf409ff..32161ec 100644
--- a/src/components/PickItemRow.tsx
+++ b/src/components/PickItemRow.tsx
@@ -49,9 +49,14 @@ export const PickItemRow = ({
     onStatusChange(checked ? 'picked' : 'pending');
   };
 
+  const closeDialogs = () => {
+    setIsConfirmOpen(false);
+    setIsControlsOpen(false);
+  };
+
   const handleConfirmDelete = () => {
     onDelete();
-    setIsConfirmOpen(false);
+    closeDialogs();
   };
 
   const handleRowClick = () => {
@@ -263,7 +268,7 @@ export const PickItemRow = ({
 
       <Dialog
         open={isConfirmOpen}
-        onClose={() => setIsConfirmOpen(false)}
+        onClose={closeDialogs}
         aria-labelledby="confirm-delete-title"
         aria-describedby="confirm-delete-description"
       >
@@ -274,15 +279,10 @@ export const PickItemRow = ({
           </DialogContentText>
         </DialogContent>
         <DialogActions>
-          <Button onClick={() => setIsConfirmOpen(false)} aria-label="Cancel delete" autoFocus>
+          <Button onClick={closeDialogs} autoFocus>
             Cancel
           </Button>
-          <Button
-            color="error"
-            variant="contained"
-            onClick={handleConfirmDelete}
-            aria-label="Confirm delete"
-          >
+          <Button color="error" variant="contained" onClick={handleConfirmDelete}>
             Delete
           </Button>
         </DialogActions>