From f5094343dcc8464e3b1773f6390d11aa4a22af51 Mon Sep 17 00:00:00 2001 From: beatz174-bit Date: Tue, 25 Nov 2025 07:40:59 +1000 Subject: [PATCH] chore: remove packaging filter radios --- e2e/picklist.spec.ts | 38 +-- src/screens/ActivePickListScreen.test.tsx | 293 +--------------------- src/screens/ActivePickListScreen.tsx | 147 ++--------- 3 files changed, 31 insertions(+), 447 deletions(-) diff --git a/e2e/picklist.spec.ts b/e2e/picklist.spec.ts index 22f1203..0c708fc 100644 --- a/e2e/picklist.spec.ts +++ b/e2e/picklist.spec.ts @@ -264,42 +264,12 @@ test.describe('Active pick list', () => { await expect(showPickedToggle).toBeChecked(); }); - test('disables packaging filters when statuses are mixed and filters by packaging when enabled', async ({ page }) => { + test('omits global packaging filter controls', async ({ page }) => { await navigateToNewPickList(page); - await addProductToPickList(page, additionalProduct); - await addProductToPickList(page, secondaryProduct); - - const cartonToggle = page.getByRole('button', { name: /Switch to carton packaging/i }).first(); - await cartonToggle.click(); - - const cartonsRadio = page.getByRole('radio', { name: 'Cartons' }); - const unitsRadio = page.getByRole('radio', { name: 'Units' }); - - await expect(cartonsRadio).toBeEnabled(); - await expect(unitsRadio).toBeEnabled(); - - await unitsRadio.click(); - await expect(unitsRadio).toBeChecked(); - await expect(page.getByText(additionalProduct)).toHaveCount(0); - await expect(page.getByText(secondaryProduct)).toBeVisible(); - - await cartonsRadio.click(); - await expect(cartonsRadio).toBeChecked(); - await expect(page.getByText(secondaryProduct)).toHaveCount(0); - await expect(page.getByText(additionalProduct)).toBeVisible(); - - const statusToggles = page.getByLabel('Toggle picked status'); - await statusToggles.first().check(); - - await expect(cartonsRadio).toBeDisabled(); - await expect(unitsRadio).toBeDisabled(); - await expect(page.getByRole('radio', { name: 'All' })).toBeChecked(); - - const showPickedToggle = page.getByRole('checkbox', { name: 'Show picked' }); - await showPickedToggle.click(); - await expect(cartonsRadio).toBeDisabled(); - await expect(unitsRadio).toBeDisabled(); + await expect(page.getByText('Filter list by packaging')).toHaveCount(0); + await expect(page.getByRole('radio', { name: 'Cartons' })).toHaveCount(0); + await expect(page.getByRole('radio', { name: 'Units' })).toHaveCount(0); }); test('toggles packaging type and persists the selection', async ({ page }) => { diff --git a/src/screens/ActivePickListScreen.test.tsx b/src/screens/ActivePickListScreen.test.tsx index bf12880..857d655 100644 --- a/src/screens/ActivePickListScreen.test.tsx +++ b/src/screens/ActivePickListScreen.test.tsx @@ -1,5 +1,5 @@ import { MemoryRouter, Route, Routes } from 'react-router-dom'; -import { render, screen, waitFor, within } from '@testing-library/react'; +import { render, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { describe, expect, it, vi, beforeEach } from 'vitest'; import { ActivePickListScreen } from './ActivePickListScreen'; @@ -500,177 +500,6 @@ describe('ActivePickListScreen product search', () => { expect(itemLabels).toEqual(['Apple Juice', 'Chips', 'Cola', 'Cola']); }); - it('disables packaging filters when the visible list has mixed pick statuses', () => { - pickItemsMock.mockReturnValue([ - { - id: 'item-1', - pick_list_id: 'list-1', - product_id: 'prod-1', - quantity: 1, - is_carton: false, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - { - id: 'item-2', - pick_list_id: 'list-1', - product_id: 'prod-2', - quantity: 1, - is_carton: false, - status: 'picked', - created_at: 0, - updated_at: 0, - }, - ]); - - render( - - - } /> - - , - ); - - expect(screen.getByRole('radio', { name: /cartons/i })).toBeDisabled(); - expect(screen.getByRole('radio', { name: /units/i })).toBeDisabled(); - }); - - it('disables packaging filters when show picked is unchecked', async () => { - pickItemsMock.mockReturnValue([ - { - id: 'item-1', - pick_list_id: 'list-1', - product_id: 'prod-1', - quantity: 1, - is_carton: true, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - { - id: 'item-2', - pick_list_id: 'list-1', - product_id: 'prod-2', - quantity: 1, - is_carton: false, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - ]); - - const user = userEvent.setup(); - - render( - - - } /> - - , - ); - - const cartonsRadio = screen.getByRole('radio', { name: /cartons/i }); - const unitsRadio = screen.getByRole('radio', { name: /units/i }); - expect(cartonsRadio).toBeEnabled(); - expect(unitsRadio).toBeEnabled(); - - const togglePicked = screen.getByLabelText(/show picked/i); - await user.click(togglePicked); - - expect(cartonsRadio).toBeDisabled(); - expect(unitsRadio).toBeDisabled(); - }); - - it('resets the packaging filter when show picked is unchecked', async () => { - pickItemsMock.mockReturnValue([ - { - id: 'item-1', - pick_list_id: 'list-1', - product_id: 'prod-1', - quantity: 1, - is_carton: true, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - { - id: 'item-2', - pick_list_id: 'list-1', - product_id: 'prod-2', - quantity: 1, - is_carton: false, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - ]); - - const user = userEvent.setup(); - - render( - - - } /> - - , - ); - - await user.click(screen.getByRole('radio', { name: /cartons/i })); - expect(screen.getByRole('radio', { name: /cartons/i })).toBeChecked(); - - await user.click(screen.getByLabelText(/show picked/i)); - - await waitFor(() => expect(screen.getByRole('radio', { name: /all/i })).toBeChecked()); - expect(screen.getByRole('radio', { name: /cartons/i })).toBeDisabled(); - expect(screen.getByRole('radio', { name: /units/i })).toBeDisabled(); - }); - - it('resets the filter when the selected packaging type is unavailable', async () => { - pickItemsMock.mockReturnValue([ - { - id: 'item-1', - pick_list_id: 'list-1', - product_id: 'prod-1', - quantity: 1, - is_carton: true, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - { - id: 'item-2', - pick_list_id: 'list-1', - product_id: 'prod-2', - quantity: 1, - is_carton: false, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - ]); - - const user = userEvent.setup(); - - render( - - - } /> - - , - ); - - await user.click(screen.getByRole('radio', { name: /cartons/i })); - expect(screen.getByRole('radio', { name: /cartons/i })).toBeChecked(); - - await user.click(screen.getByLabelText(/show picked/i)); - - await waitFor(() => expect(screen.getByRole('radio', { name: /all/i })).toBeChecked()); - expect(screen.getByRole('radio', { name: /cartons/i })).toBeDisabled(); - expect(screen.getByRole('radio', { name: /units/i })).toBeDisabled(); - }); - - it('hides picked items when show picked is unchecked', async () => { pickItemsMock.mockReturnValue([ { @@ -718,126 +547,6 @@ describe('ActivePickListScreen product search', () => { expect(screen.getByText('Cola')).toBeVisible(); }); - it('filters the visible list by packaging type and keeps the selection active', async () => { - pickItemsMock.mockReturnValue([ - { - id: 'item-1', - pick_list_id: 'list-1', - product_id: 'prod-1', - quantity: 1, - is_carton: true, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - { - id: 'item-2', - pick_list_id: 'list-1', - product_id: 'prod-2', - quantity: 1, - is_carton: false, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - ]); - - const user = userEvent.setup(); - - render( - - - } /> - - , - ); - - const cartonsRadio = screen.getByRole('radio', { name: /cartons/i }); - const unitsRadio = screen.getByRole('radio', { name: /units/i }); - - await user.click(unitsRadio); - expect(unitsRadio).toBeChecked(); - expect(screen.getByText('Chips')).toBeVisible(); - expect(screen.queryByText('Cola')).not.toBeInTheDocument(); - - await user.click(cartonsRadio); - expect(cartonsRadio).toBeChecked(); - expect(screen.getByText('Cola')).toBeVisible(); - expect(screen.queryByText('Chips')).not.toBeInTheDocument(); - }); - - it('keeps packaging filters enabled when all items share the same status', () => { - pickItemsMock.mockReturnValue([ - { - id: 'item-1', - pick_list_id: 'list-1', - product_id: 'prod-1', - quantity: 1, - is_carton: true, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - { - id: 'item-2', - pick_list_id: 'list-1', - product_id: 'prod-2', - quantity: 1, - is_carton: false, - status: 'pending', - created_at: 0, - updated_at: 0, - }, - ]); - - render( - - - } /> - - , - ); - - expect(screen.getByRole('radio', { name: /cartons/i })).toBeEnabled(); - expect(screen.getByRole('radio', { name: /units/i })).toBeEnabled(); - }); - - it('keeps packaging filters enabled when every item is picked', () => { - pickItemsMock.mockReturnValue([ - { - id: 'item-1', - pick_list_id: 'list-1', - product_id: 'prod-1', - quantity: 1, - is_carton: true, - status: 'picked', - created_at: 0, - updated_at: 0, - }, - { - id: 'item-2', - pick_list_id: 'list-1', - product_id: 'prod-2', - quantity: 1, - is_carton: false, - status: 'picked', - created_at: 0, - updated_at: 0, - }, - ]); - - render( - - - } /> - - , - ); - - expect(screen.getByRole('radio', { name: /cartons/i })).toBeEnabled(); - expect(screen.getByRole('radio', { name: /units/i })).toBeEnabled(); - }); - it('disables show picked toggle when all items are picked', async () => { pickItemsMock.mockReturnValue([ { diff --git a/src/screens/ActivePickListScreen.tsx b/src/screens/ActivePickListScreen.tsx index 0a187ad..87b43a4 100644 --- a/src/screens/ActivePickListScreen.tsx +++ b/src/screens/ActivePickListScreen.tsx @@ -3,12 +3,9 @@ import { Button, Checkbox, Container, - FormControl, FormControlLabel, IconButton, InputAdornment, - Radio, - RadioGroup, Stack, TextField, Tooltip, @@ -37,7 +34,6 @@ export const ActivePickListScreen = () => { const navigate = useNavigate(); const [selectedProduct, setSelectedProduct] = useState(null); const [query, setQuery] = useState(''); - const [itemFilter, setItemFilter] = useState<'all' | 'cartons' | 'units'>('all'); const [showPicked, setShowPicked] = useState(true); const [itemState, setItemState] = useState(items); @@ -45,50 +41,17 @@ export const ActivePickListScreen = () => { setItemState(items); }, [items]); - // Reworked visible/filter logic: - // - Keep `itemFilter` as the single source-of-truth for the radio selection. - // - Compute items after applying `showPicked` and `itemFilter`, and derive - // disabled flags from the actual visible items to avoid races / feedback loops. + // Reworked visible logic: const itemsAfterShowPicked = useMemo( () => (showPicked ? itemState : itemState.filter((item) => item.status !== 'picked')), [itemState, showPicked], ); - const hasCartonItemsOverall = useMemo( - () => itemsAfterShowPicked.some((item) => item.is_carton), - [itemsAfterShowPicked], - ); - const hasUnitItemsOverall = useMemo( - () => itemsAfterShowPicked.some((item) => !item.is_carton), - [itemsAfterShowPicked], - ); - - const itemsAfterPackagingFilter = useMemo(() => { - if (itemFilter === 'cartons') return itemsAfterShowPicked.filter((item) => item.is_carton); - if (itemFilter === 'units') return itemsAfterShowPicked.filter((item) => !item.is_carton); - return itemsAfterShowPicked; - }, [itemsAfterShowPicked, itemFilter]); - - const hasPickedItemsVisible = useMemo( - () => itemsAfterPackagingFilter.some((item) => item.status === 'picked'), - [itemsAfterPackagingFilter], - ); - const hasUnpickedItemsVisible = useMemo( - () => itemsAfterPackagingFilter.some((item) => item.status !== 'picked'), - [itemsAfterPackagingFilter], - ); - const allItemsPicked = useMemo( () => itemState.length > 0 && itemState.every((item) => item.status === 'picked'), [itemState], ); - const packagingFiltersDisabled = useMemo( - // packaging filters disabled when showPicked is false, or visible items contain mixed statuses - () => !showPicked || (hasPickedItemsVisible && hasUnpickedItemsVisible), - [hasPickedItemsVisible, hasUnpickedItemsVisible, showPicked], - ); - const productMap = useMemo(() => { const map = new Map(); products.forEach((product) => { @@ -103,23 +66,6 @@ export const ActivePickListScreen = () => { [areas, pickList?.area_id], ); - // Keep a sorted list for available items after applying showPicked (used for other UX) - const sortedItems = useMemo(() => { - return [...itemsAfterShowPicked].sort((a, b) => { - const timeA = a.created_at ?? a.updated_at ?? 0; - const timeB = b.created_at ?? b.updated_at ?? 0; - - if (timeA !== timeB) { - return timeA - timeB; - } - - const nameA = normalizeName(productMap.get(a.product_id)?.name ?? ''); - const nameB = normalizeName(productMap.get(b.product_id)?.name ?? ''); - - return nameA.localeCompare(nameB, undefined, { sensitivity: 'base' }); - }); - }, [itemsAfterShowPicked, productMap]); - const sortedProducts = useMemo(() => { const dedupedById = new Map(); @@ -203,24 +149,9 @@ export const ActivePickListScreen = () => { } }, [allItemsPicked, showPicked]); - // Sanitize itemFilter whenever availability changes or packaging is disabled. - useEffect(() => { - if (packagingFiltersDisabled) { - if (itemFilter !== 'all') setItemFilter('all'); - return; - } - - if (itemFilter === 'cartons' && !hasCartonItemsOverall) { - setItemFilter(hasUnitItemsOverall ? 'units' : 'all'); - } else if (itemFilter === 'units' && !hasUnitItemsOverall) { - setItemFilter(hasCartonItemsOverall ? 'cartons' : 'all'); - } - }, [packagingFiltersDisabled, itemFilter, hasCartonItemsOverall, hasUnitItemsOverall]); - - // Sort the items that are actually visible (after showPicked + packaging filter) + // Sort the items that are actually visible (after showPicked) const visibleItems = useMemo(() => { - // itemsAfterPackagingFilter is already computed after showPicked & packaging - const arr = [...itemsAfterPackagingFilter]; + const arr = [...itemsAfterShowPicked]; arr.sort((a, b) => { const timeA = a.created_at ?? a.updated_at ?? 0; const timeB = b.created_at ?? b.updated_at ?? 0; @@ -230,7 +161,7 @@ export const ActivePickListScreen = () => { return nameA.localeCompare(nameB, undefined, { sensitivity: 'base' }); }); return arr; - }, [itemsAfterPackagingFilter, productMap]); + }, [itemsAfterShowPicked, productMap]); const updateItemState = (itemId: string, updater: (item: PickItem) => PickItem) => { setItemState((current) => current.map((item) => (item.id === itemId ? updater(item) : item))); @@ -426,55 +357,29 @@ export const ActivePickListScreen = () => { Selecting a product immediately adds it to the pick list. - - - Filter list by packaging - - - setItemFilter(value as 'all' | 'cartons' | 'units')} - sx={{ flexGrow: 1 }} - > - } label="All" /> - } - label="Cartons" - disabled={packagingFiltersDisabled} - /> - } - label="Units" - disabled={packagingFiltersDisabled} - /> - - - setShowPicked(event.target.checked)} - disabled={allItemsPicked} - /> - } - label="Show picked" - /> - - + + + setShowPicked(event.target.checked)} + disabled={allItemsPicked} + /> + } + label="Show picked" + /> + - + {pickList?.notes ? (