From 85023a68bf4a2132328fee37c58ed64e9064cefa Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 31 Jul 2026 09:51:46 -0400 Subject: [PATCH] fix(form): address mobile admin review findings --- src/server/templates/admin.js | 49 ++++++------------ tests/test_generate_form.py | 98 ++++++++++++++++++++++------------- 2 files changed, 79 insertions(+), 68 deletions(-) diff --git a/src/server/templates/admin.js b/src/server/templates/admin.js index f64f6ae..897a5da 100644 --- a/src/server/templates/admin.js +++ b/src/server/templates/admin.js @@ -182,10 +182,11 @@ function editTotal() { }, 0); } -function populateOrderItems(container, order, isEditing) { +function populateOrderItems(container, order, isEditing, separatorFactory) { if (isEditing) { Object.keys(editQuantities).forEach(name => { container.appendChild(buildEditQtyRow(name, editQuantities[name] || 0)); + if (separatorFactory) container.appendChild(separatorFactory()); }); const availableMeals = MEALS.filter( m => !Object.prototype.hasOwnProperty.call(editQuantities, m.name) @@ -196,15 +197,18 @@ function populateOrderItems(container, order, isEditing) { return '$' + editTotal().toFixed(2); } - order.items.forEach(item => { - const row = document.createElement('div'); - row.className = 'admin-item'; + order.items.forEach((item, itemIdx) => { + if (separatorFactory && itemIdx > 0) { + container.appendChild(separatorFactory()); + } + const row = separatorFactory ? container : document.createElement('div'); + if (!separatorFactory) row.className = 'admin-item'; row.appendChild(document.createTextNode(item.name + ' ×' + item.quantity + ' ')); const sub = document.createElement('span'); sub.className = 'admin-muted'; sub.textContent = '($' + item.subtotal.toFixed(2) + ')'; row.appendChild(sub); - container.appendChild(row); + if (!separatorFactory) container.appendChild(row); }); return '$' + order.total.toFixed(2); } @@ -317,33 +321,14 @@ function renderAdminTable(data) { const itemsTd = document.createElement('td'); itemsTd.className = 'items-cell'; - let totalHtml; - if (isEditing) { - const editNames = Object.keys(editQuantities); - editNames.forEach(name => { - itemsTd.appendChild(buildEditQtyRow(name, editQuantities[name] || 0)); - itemsTd.appendChild(document.createElement('br')); - }); - const availableMeals = MEALS.filter(m => !Object.prototype.hasOwnProperty.call(editQuantities, m.name)); - if (availableMeals.length > 0) { - itemsTd.appendChild(buildAddItemSelect(availableMeals)); - } - totalHtml = '$' + editTotal().toFixed(2); - tr.appendChild(empTd); - tr.appendChild(itemsTd); - } else { - order.items.forEach((item, itemIdx) => { - if (itemIdx > 0) itemsTd.appendChild(document.createElement('br')); - itemsTd.appendChild(document.createTextNode(item.name + ' ×' + item.quantity + ' ')); - const sub = document.createElement('span'); - sub.className = 'admin-muted'; - sub.textContent = '($' + item.subtotal.toFixed(2) + ')'; - itemsTd.appendChild(sub); - }); - totalHtml = '$' + order.total.toFixed(2); - tr.appendChild(empTd); - tr.appendChild(itemsTd); - } + const totalHtml = populateOrderItems( + itemsTd, + order, + isEditing, + () => document.createElement('br') + ); + tr.appendChild(empTd); + tr.appendChild(itemsTd); const totalTd = document.createElement('td'); totalTd.className = 'total-cell'; diff --git a/tests/test_generate_form.py b/tests/test_generate_form.py index 9a73024..c778671 100644 --- a/tests/test_generate_form.py +++ b/tests/test_generate_form.py @@ -686,40 +686,44 @@ class TestGenerateFormPlaywright: has_touch=True, is_mobile=True, ) - page = context.new_page() - page.route( - "**/api/form-status/**", - lambda route: route.fulfill( - status=200, - content_type="application/json", - body='{"status":"open"}', - ), - ) - page.route( - "**/api/roster", - lambda route: route.fulfill( - status=200, - content_type="application/json", - body='{"employees":[]}', - ), - ) - page.route("**/api/admin/orders**", handle_admin) - page.on( - "dialog", - lambda dialog: ( - state["dialogs"].append(dialog.message), - dialog.accept(), - ), - ) - page.goto(out.as_uri()) - page.evaluate( - "() => { window.showAdmin(); window.loadWeekOrders('2026-W30'); }" - ) - page.wait_for_selector(".admin-order-card") - yield page, state, hostile_name - context.close() + try: + page = context.new_page() + page.route( + "**/api/form-status/**", + lambda route: route.fulfill( + status=200, + content_type="application/json", + body='{"status":"open"}', + ), + ) + page.route( + "**/api/roster**", + lambda route: route.fulfill( + status=200, + content_type="application/json", + body='{"employees":[]}', + ), + ) + page.route("**/api/admin/orders**", handle_admin) + page.on( + "dialog", + lambda dialog: ( + state["dialogs"].append(dialog.message), + dialog.accept() + if state.get("accept_dialogs", True) + else dialog.dismiss(), + ), + ) + page.goto(out.as_uri()) + page.evaluate( + "() => { window.showAdmin(); window.loadWeekOrders('2026-W30'); }" + ) + page.wait_for_selector(".admin-order-card") + yield page, state, hostile_name + finally: + context.close() - def test_mobile_admin_card_complete_workflow(self, mobile_admin_page): + def test_mobile_admin_card_initial_render_and_overflow(self, mobile_admin_page): page, state, hostile_name = mobile_admin_page card = page.locator(".admin-order-card") @@ -727,13 +731,17 @@ class TestGenerateFormPlaywright: assert not page.locator(".admin-table-wrap").is_visible() assert hostile_name in card.inner_text() assert "hostile@example.com" in card.inner_text() - assert "Chicken Bowl ×1" in card.inner_text() + assert "Chicken Bowl ×1" in card.inner_text() # noqa: RUF001 assert "$12.00" in card.inner_text() assert page.evaluate( "() => document.documentElement.scrollWidth === " "document.documentElement.clientWidth" ) + def test_mobile_admin_card_edit_and_save(self, mobile_admin_page): + page, state, hostile_name = mobile_admin_page + card = page.locator(".admin-order-card") + card.locator('[data-admin-action="start-edit"]').click() page.wait_for_selector('.admin-order-card [data-admin-action="save-edit"]') minus = page.locator( @@ -755,11 +763,14 @@ class TestGenerateFormPlaywright: page.locator('.admin-order-card [data-admin-action="save-edit"]').click() page.wait_for_function( "() => document.querySelector('.admin-order-card')" - '.textContent.includes("Luis\' Lomo Saltado ×2")' + '.textContent.includes("Luis\' Lomo Saltado ×2")' # noqa: RUF001 ) assert state["orders"][0]["employee_name"] == hostile_name assert state["orders"][0]["total"] == 41.0 + def test_mobile_admin_card_cancel(self, mobile_admin_page): + page, _, _ = mobile_admin_page + page.locator('.admin-order-card [data-admin-action="start-edit"]').click() page.wait_for_selector('.admin-order-card [data-admin-action="cancel-edit"]') page.locator( @@ -770,6 +781,9 @@ class TestGenerateFormPlaywright: '.admin-order-card [data-admin-action="start-edit"]' ).is_visible() + def test_mobile_admin_card_responsive_breakpoints(self, mobile_admin_page): + page, _, _ = mobile_admin_page + for width in (320, 375, 767): page.set_viewport_size({"width": width, "height": 800}) assert page.locator(".admin-order-card").is_visible() @@ -787,7 +801,9 @@ class TestGenerateFormPlaywright: assert page.locator(".admin-table-wrap").is_visible() assert not page.locator(".admin-order-card").is_visible() - page.set_viewport_size({"width": 375, "height": 800}) + def test_mobile_admin_card_save_failure_and_retry(self, mobile_admin_page): + page, state, _ = mobile_admin_page + state["fail_put"] = True page.locator('.admin-order-card [data-admin-action="start-edit"]').click() page.wait_for_selector('.admin-order-card [data-admin-action="save-edit"]') @@ -801,6 +817,16 @@ class TestGenerateFormPlaywright: page.wait_for_selector('.admin-order-card [data-admin-action="start-edit"]') assert page.locator("#admin-status-alert").inner_text() == "" + def test_mobile_admin_card_delete(self, mobile_admin_page): + page, state, hostile_name = mobile_admin_page + + orders_before = list(state["orders"]) + state["accept_dialogs"] = False + page.locator('.admin-order-card [data-admin-action="delete"]').click() + assert page.locator(".admin-order-card").is_visible() + assert state["orders"] == orders_before + + state["accept_dialogs"] = True page.locator('.admin-order-card [data-admin-action="delete"]').click() page.wait_for_selector("#admin-empty:not(.is-hidden)") assert hostile_name in state["dialogs"][-1]