health score toggle ui

src/features/catalogue/BulkImportDrawer.tsx

@@ -0,0 +1,210 @@
+import { useState } from 'react';
+import { useQuery } from '@tanstack/react-query';
+import { Switch } from '@astryxdesign/core/Switch';
+import { nutritionApi } from '@/api/nutrition';
+import type { CatalogueProduct } from '@/api/types';
+import { Drawer } from '@/features/store-admin/Drawer';
+import { DrawerButton, Note } from '@/features/store-admin/drawerKit';
+import { BAND_LABEL, isEdible, present } from '@/features/store-admin/healthScore';
+import { catalogueKey } from '@/api/catalogue';
+
+/**
+ * Adding several products at once, and deciding the health score for each.
+ *
+ * ── Why a drawer here and not on the single-product path ────────────────────
+ *
+ * A single product already has a drawer — the catalogue detail one — which shows
+ * its score and carries the toggle, so a second panel there was repeating what
+ * was already on screen. A batch has no such drawer, so this is the first place
+ * the question can be asked at all.
+ *
+ * ── Why the toggles are per product ─────────────────────────────────────────
+ *
+ * Because a batch is not one decision. Three products can easily be two that
+ * should show a rating and one that should not — a 47%-confidence match on a
+ * product the shopkeeper knows is mis-matched — and a single switch for the
+ * whole selection forces that into a lie either way.
+ *
+ * ── Why each row shows its score ────────────────────────────────────────────
+ *
+ * The same reason the detail drawer does: "show health score?" against a product
+ * name is unanswerable. Beside "22/100 · matched at 47%" it answers itself. The
+ * rows that have no score say so and carry no toggle, so nobody sets a control
+ * that decides nothing.
+ */
+export function BulkImportDrawer({
+  products,
+  actionLabel,
+  isBusy,
+  defaultShowScore,
+  onCancel,
+  onConfirm,
+}: {
+  products: CatalogueProduct[];
+  actionLabel: string;
+  isBusy: boolean;
+  /** The screen's setting, which every row starts at. */
+  defaultShowScore: boolean;
+  onCancel: () => void;
+  /**
+   * Keyed by `catalogueKey`. A product missing from the map has no score, and
+   * the import says nothing about it rather than sending a choice nobody made.
+   */
+  onConfirm: (choices: Map<string, boolean>) => void;
+}) {
+  /*
+    Only the rows the shopkeeper has actually moved.
+
+    Starting empty rather than pre-filling every product with the default means
+    "untouched" and "deliberately set to the default" stay the same thing, which
+    they are — and a row whose score never loads cannot end up contributing a
+    choice about a score nobody saw.
+  */
+  const [choices, setChoices] = useState<Map<string, boolean>>(new Map());
+
+  const choiceFor = (key: string) => choices.get(key) ?? defaultShowScore;
+  const setChoice = (key: string, show: boolean) =>
+    setChoices((previous) => new Map(previous).set(key, show));
+
+  function setAll(show: boolean) {
+    setChoices(new Map(products.map((product) => [catalogueKey(product), show])));
+  }
+
+  return (
+    <Drawer
+      title={`Add ${products.length} products`}
+      subtitle="Choose which of these show a health score"
+      width={520}
+      onClose={onCancel}
+      isFooterSpread
+      footer={
+        <>
+          <DrawerButton label="Cancel" variant="ghost" onClick={onCancel} />
+          <DrawerButton
+            label={isBusy ? 'Adding…' : `${actionLabel} (${products.length})`}
+            variant="primary"
+            isDisabled={isBusy}
+            onClick={() => onConfirm(choices)}
+          />
+        </>
+      }
+    >
+      <Note>
+        The nutrition figures are shown either way — this is only the rating. Any of these can be
+        changed later from the product.
+      </Note>
+
+      {/* For the ordinary case, where the whole batch goes the same way. Without
+          it a shopkeeper who wants forty products' ratings off has forty clicks
+          to make, which is how a feature becomes one nobody uses. */}
+      <div style={{ display: 'flex', gap: 8, margin: '12px 0 4px' }}>
+        <DrawerButton label="Show all" variant="secondary" onClick={() => setAll(true)} />
+        <DrawerButton label="Hide all" variant="secondary" onClick={() => setAll(false)} />
+      </div>
+
+      <div style={{ display: 'grid' }}>
+        {products.map((product) => (
+          <BulkImportRow
+            key={catalogueKey(product)}
+            product={product}
+            isShown={choiceFor(catalogueKey(product))}
+            onChange={(show) => setChoice(catalogueKey(product), show)}
+          />
+        ))}
+      </div>
+    </Drawer>
+  );
+}
+
+/**
+ * One product in the batch, with its score and its toggle.
+ *
+ * Each row asks for its own score. They are the same lookups the product drawer
+ * would make, cached for an hour by the query client and shared with it — so
+ * opening a product after a batch costs nothing, and re-opening this drawer
+ * costs nothing either.
+ */
+function BulkImportRow({
+  product,
+  isShown,
+  onChange,
+}: {
+  product: CatalogueProduct;
+  isShown: boolean;
+  onChange: (show: boolean) => void;
+}) {
+  const brand = (product.brand ?? '').trim();
+  const imageId = (product.image_id ?? '').trim();
+
+  // Non-food never has a score — the service has rated insecticide 80/100, so
+  // an allowlist gates it — and asking about one is a request for nothing.
+  const isFood = isEdible(product.category);
+  const query = useQuery({
+    queryKey: ['nutrition', brand, imageId],
+    queryFn: () => nutritionApi.forProduct(brand, imageId),
+    enabled: Boolean(brand && imageId && isFood),
+    staleTime: 60 * 60_000,
+    refetchOnWindowFocus: false,
+    retry: false,
+  });
+
+  const shown = present(query.data ?? null);
+  const hasScore = isFood && !shown.isEmpty && !shown.isPending && shown.score !== null;
+  const isChecking = isFood && Boolean(brand && imageId) && query.isLoading;
+
+  return (
+    <div
+      style={{
+        display: 'flex',
+        alignItems: 'center',
+        gap: 12,
+        padding: '11px 0',
+        borderTop: '1px solid var(--color-line, #e0e4ea)',
+      }}
+    >
+      <div style={{ flex: 1, minWidth: 0 }}>
+        <div
+          style={{
+            fontSize: 13,
+            fontWeight: 500,
+            color: 'var(--color-ink-1)',
+            overflow: 'hidden',
+            textOverflow: 'ellipsis',
+            whiteSpace: 'nowrap',
+          }}
+        >
+          {product.product_name}
+        </div>
+        <div style={{ fontSize: 11.5, color: 'var(--color-ink-4)', marginTop: 2 }}>
+          {isChecking
+            ? 'Checking…'
+            : hasScore
+              ? `${shown.display}/100 · ${shown.band ? BAND_LABEL[shown.band] : ''}${
+                  shown.caveat ? ` · ${shown.caveat.match(/\d+%/)?.[0] ?? ''} match` : ''
+                }`
+              : isFood
+                ? 'No health score yet'
+                : 'Not food — no health score'}
+        </div>
+      </div>
+
+      {hasScore ? (
+        /* The label is hidden visually and kept for a screen reader: the product
+           name sitting beside it is what a sighted reader uses, and repeating
+           "Show health score for Britannia Good Day…" on every row would be the
+           same sentence forty times down a list. */
+        <Switch
+          label={`Show the health score for ${product.product_name}`}
+          isLabelHidden
+          value={isShown}
+          onChange={onChange}
+          size="sm"
+        />
+      ) : (
+        /* No toggle, and the space kept, so the rows do not jag left and right
+           down the list as scores resolve at different speeds. */
+        <span style={{ width: 44 }} />
+      )}
+    </div>
+  );
+}

src/features/catalogue/CatalogueBrowser.tsx

@@ -1,6 +1,7 @@
 import { useEffect, useMemo, useState, type ReactNode } from 'react';
 import { useMutation, useQueryClient } from '@tanstack/react-query';
 import { Button } from '@astryxdesign/core/Button';
+import { Switch } from '@astryxdesign/core/Switch';
 import { EmptyState } from '@astryxdesign/core/EmptyState';
 import { HStack } from '@astryxdesign/core/HStack';
 import { IconButton } from '@astryxdesign/core/IconButton';
@@ -27,6 +28,7 @@ import {
 } from '@/queries/hooks';
 import { CatalogueCard } from './CatalogueCard';
 import { CatalogueSidebar } from './CatalogueSidebar';
+import { BulkImportDrawer } from './BulkImportDrawer';
 import { CatalogueDetailDrawer } from './CatalogueDetailDrawer';
 
 const PAGE_SIZE = 24;
@@ -93,6 +95,30 @@ export function CatalogueBrowser({
   const [debounced, setDebounced] = useState('');
   const [busy, setBusy] = useState<string | null>(null);
   const [justImported, setJustImported] = useState<Set<string>>(new Set());
+  /*
+    Whether products added from this screen show their health score.
+
+    Two of the three ways to add a product never open the drawer — a card's Add
+    is one click and a bulk add is one click for forty — so without this they
+    had no say at all, and "showing" was decided for them.
+
+    One control for the whole screen rather than a question per product: a
+    prompt on every card add is the thing that was taken out, and a toggle asked
+    once for forty products is a question nobody answers honestly. The drawer's
+    own toggle still wins for the product it is open on, because somebody
+    reading that score has better information than this default does.
+  */
+  const [showScoreOnAdd, setShowScoreOnAdd] = useState(true);
+  /*
+    The batch waiting on its per-product choices.
+
+    A selection is not one decision: three products can be two whose rating
+    should show and one whose should not, and a single switch for the whole set
+    forces that into a lie either way. Unlike the single-product path, a batch
+    has no drawer of its own — so this is the first place the question can be
+    asked at all rather than a second panel repeating one.
+  */
+  const [batch, setBatch] = useState<CatalogueProduct[] | null>(null);
   const [open, setOpen] = useState<CatalogueProduct | null>(null);
   const [category, setCategory] = useState('');
   const [page, setPage] = useState(1);
@@ -198,15 +224,22 @@ export function CatalogueBrowser({
     */
     mutationFn: async ({
       products,
-      showHealthScore,
+      choices,
     }: {
       products: CatalogueProduct[];
-      showHealthScore: boolean | undefined;
+      /* Keyed by `catalogueKey`. A product missing from the map takes the
+         screen's setting — the import request carries one value per row, so a
+         mixed batch is one call, not one per choice. */
+      choices: Map<string, boolean>;
     }) => {
       const ids = await aisleIds();
       return productsApi.importFromCatalogue(
         products.map((product) =>
-          importRowFor(product, aisleIdForCategory(categoryNameFor(product), ids), showHealthScore),
+          importRowFor(
+            product,
+            aisleIdForCategory(categoryNameFor(product), ids),
+            choices.get(catalogueKey(product)) ?? showScoreOnAdd,
+          ),
         ),
       );
     },
@@ -331,19 +364,22 @@ export function CatalogueBrowser({
     };
   }
 
+  /*
+    Add straight from the card, in one click, as it has always been.
+
+    No question asked here, deliberately. The health score toggle lives in the
+    product's own drawer, where the score and its confidence are on screen — and
+    a card has neither, so a toggle on this path would be asking somebody to
+    judge a rating they cannot see.
+
+    The product imports with its score showing, which is the default and what
+    every product did before the toggle existed. Anyone who wants it off opens
+    the product and turns it off, which is the same control in the same place as
+    changing their mind later.
+  */
   async function importDirect(product: CatalogueProduct) {
     if (!tenantid || !locationid) return;
-
+    await confirmImport(product, showScoreOnAdd);
-    /*
-      Opens the product rather than importing on the spot.
-
-      The drawer already shows this product's health score and how confident
-      the match is, and the import now carries a decision about exactly that —
-      so the question is asked where the answer is visible. A separate
-      confirmation would be a second panel repeating what this one already
-      renders.
-    */
-    setOpen(product);
   }
 
   /*
@@ -542,6 +578,20 @@ export function CatalogueBrowser({
                       </Text>
                     </label>
 
+                    {/* The decision for everything added from this screen.
+
+                        Beside the add controls rather than in a settings panel,
+                        because it is read at the moment it applies — somebody
+                        about to add forty products can see what those forty will
+                        do. Ticked by default, which is what every product did
+                        before the toggle existed. */}
+                    <Switch
+                      label="Show health score on products I add"
+                      value={showScoreOnAdd}
+                      onChange={setShowScoreOnAdd}
+                      size="sm"
+                    />
+
                     {selection.count > 0 ? (
                       <HStack gap={1} align="center" wrap="wrap">
                         <Button
@@ -561,16 +611,13 @@ export function CatalogueBrowser({
                           size="sm"
                           isLoading={importMany.isPending}
                           isDisabled={importMany.isPending}
-                          /* No per-product question here: a toggle asked once
+                          /* The whole batch takes the screen's setting — the
-                             for forty products is a question nobody answers
+                             checkbox beside this button. Asking per product
-                             honestly. A batch imports showing the score, which
+                             across a selection of forty is a question nobody
-                             is the default, and any of them can be changed
+                             answers honestly, and any of them can still be
-                             afterwards from the product. */
+                             changed afterwards from the product. */
                           onClick={() =>
-                            importMany.mutate({
+                            setBatch(rows.filter((product) => selection.has(product.id)))
-                              products: rows.filter((product) => selection.has(product.id)),
-                              showHealthScore: undefined,
-                            })
                           }
                         />
                       </HStack>
@@ -622,6 +669,21 @@ export function CatalogueBrowser({
         </VStack>
       </div>
 
+      {batch && batch.length > 0 ? (
+        <BulkImportDrawer
+          products={batch}
+          actionLabel={actionLabel}
+          isBusy={importMany.isPending}
+          defaultShowScore={showScoreOnAdd}
+          onCancel={() => setBatch(null)}
+          onConfirm={(choices) => {
+            const products = batch;
+            setBatch(null);
+            importMany.mutate({ products, choices });
+          }}
+        />
+      ) : null}
+
       {open ? (
         <CatalogueDetailDrawer
           product={open}

src/features/catalogue/CatalogueCard.tsx

@@ -1,5 +1,5 @@
 import { useState, type MouseEvent } from 'react';
-import { Check, ChevronLeft, ChevronRight, Eye, ImageOff, Plus } from 'lucide-react';
+import { Check, ChevronLeft, ChevronRight, ImageOff, Plus } from 'lucide-react';
 import type { CatalogueProduct } from '@/api/types';
 
 /**
@@ -145,13 +145,6 @@ export function CatalogueCard({
             what the packaging itself carries in larger type than we could. */}
         {isImported ? <span className="pcard-owned">In your list</span> : null}
 
-        {/* The action rail. One icon, because one is all we have a use for —
-            the reference's wishlist and compare have nothing behind them. */}
-        <span className="pcard-rail">
-          <span className="pcard-railbtn" aria-hidden="true">
-            <Eye size={14} />
-          </span>
-        </span>
 
         {/* The photo switcher. Only when there is more than one to switch to —
             an arrow that does nothing is worse than no arrow, and most of a

src/features/store-admin/HealthScorePanel.tsx

@@ -1,5 +1,6 @@
 import { useEffect } from 'react';
 import { useQuery } from '@tanstack/react-query';
+import { Switch } from '@astryxdesign/core/Switch';
 import { AlertTriangle, Check, ExternalLink, Leaf } from 'lucide-react';
 import { nutritionApi } from '@/api/nutrition';
 import type { Product } from '@/api/types';
@@ -112,7 +113,31 @@ export function HealthScorePanel({
     retry: false,
   });
 
-  // Past the hook, so the count is the same on every render.
+  const shown = present(query.data ?? null);
+
+  /*
+    Tell the caller whether there is a score, once the lookup has settled.
+
+    In an effect rather than during render, because this sets state in a parent
+    and doing that mid-render is React's "cannot update a component while
+    rendering a different one". Guarded on `isLoading` so a drawer does not
+    briefly see "no score" for a product that has one and hide its toggle.
+
+    ABOVE EVERY RETURN IN THIS COMPONENT, and that placement is the whole point.
+    React counts hooks per instance, so a hook sitting behind an early return is
+    called on some renders and not others — "rendered more hooks than during the
+    previous render", which does not degrade the panel, it unmounts the screen.
+    This file has now produced that crash twice: once from a `return null` above
+    `useQuery`, and once from this effect sitting below three returns. Nothing
+    goes between `useQuery` and the first `return` but plain computation.
+  */
+  const hasScore = !shown.isEmpty && !shown.isPending && shown.score !== null;
+  useEffect(() => {
+    if (query.isLoading) return;
+    onScoreResolved?.(hasScore);
+  }, [query.isLoading, hasScore, onScoreResolved]);
+
+  // Past every hook, so the count is the same on every render.
   if (!isFood) return null;
 
   /*
@@ -146,22 +171,6 @@ export function HealthScorePanel({
     );
   }
 
-  const shown = present(query.data ?? null);
-
-  /*
-    Tell the caller whether there is a score, once the lookup has settled.
-
-    In an effect rather than during render: this sets state in a parent, and
-    doing that while rendering is the React warning about updating one
-    component from inside another. Guarded on `isLoading` so a drawer does not
-    briefly see "no score" for a product that has one and hide its toggle.
-  */
-  const hasScore = !shown.isEmpty && !shown.isPending && shown.score !== null;
-  useEffect(() => {
-    if (query.isLoading) return;
-    onScoreResolved?.(hasScore);
-  }, [query.isLoading, hasScore, onScoreResolved]);
-
   /*
     This shop has turned the score off.
 
@@ -317,10 +326,14 @@ export function HealthScorePanel({
 /**
  * Turning the score on or off for this shop.
  *
- * A plain checkbox rather than a switch, because it sits inside a panel of
+ * The design system's `Switch`, not a hand-rolled checkbox: it carries the
- * facts and a switch reads as a setting screen. The wording says what a
+ * site's focus ring, its disabled and loading states, its label and description
- * customer sees, not what a column holds — "show to customers" is the decision;
+ * typography, and its dark-mode colours — none of which an `<input
- * `showhealthscore` is the implementation.
+ * type="checkbox">` with inline styles has, and all of which the rest of the
+ * console already has.
+ *
+ * The wording says what a customer sees rather than what a column holds. "Show
+ * to customers" is the decision; `showhealthscore` is the implementation.
  */
 function ShowScoreToggle({
   isShown,
@@ -330,30 +343,15 @@ function ShowScoreToggle({
   onChange: (show: boolean) => void;
 }) {
   return (
-    <label
+    <div style={{ marginTop: 12 }}>
-      style={{
+      <Switch
-        display: 'flex',
+        label="Show this health score to customers"
-        alignItems: 'flex-start',
+        description="Appears on this product in your catalogue and in the customer app. The nutrition figures are shown either way."
-        gap: 10,
+        value={isShown}
-        marginTop: 10,
+        onChange={onChange}
-        cursor: 'pointer',
+        size="sm"
-      }}
+        labelSpacing="spread"
-    >
-      <input
-        type="checkbox"
-        checked={isShown}
-        onChange={(event) => onChange(event.target.checked)}
-        style={{ marginTop: 3, width: 16, height: 16, cursor: 'pointer' }}
       />
-      <span style={{ display: 'grid', gap: 3 }}>
+    </div>
-        <span style={{ fontSize: 12.5, fontWeight: 600, color: 'var(--color-ink-1)' }}>
-          Show this health score to customers
-        </span>
-        <span style={{ fontSize: 11.5, lineHeight: 1.5, color: 'var(--color-ink-4)' }}>
-          Appears on this product in your catalogue and in the customer app. The nutrition
-          figures are shown either way.
-        </span>
-      </span>
-    </label>
   );
 }

src/features/store-admin/healthScoreHooks.test.ts

@@ -0,0 +1,66 @@
+import { strict as assert } from 'node:assert';
+import { readFileSync } from 'node:fs';
+import { test } from 'node:test';
+
+/*
+Every hook in HealthScorePanel is called on every render.
+
+This file has crashed the catalogue twice with "rendered more hooks than during
+the previous render" — once from a `return null` sitting above `useQuery`, and
+once from a `useEffect` added below three early returns. Both times the symptom
+was the same and it is not a degraded panel: React unmounts the tree, so the
+health score disappears from EVERY product until the page is reloaded, which
+reads as the feature having been switched off.
+
+React's own lint rule catches this and is not wired into this project, so the
+guard is here: a source check, because the bug is structural and visible in the
+text. Nothing goes between the first hook and the first `return` but plain
+computation.
+*/
+
+const source = readFileSync(
+  new URL('./HealthScorePanel.tsx', import.meta.url),
+  'utf8',
+);
+
+/** Line numbers, 1-based, of every line matching the pattern. */
+function linesMatching(pattern: RegExp): number[] {
+  return source
+    .split('\n')
+    .map((line, index) => (pattern.test(line) ? index + 1 : 0))
+    .filter((n) => n > 0);
+}
+
+test('no hook is called after an early return', () => {
+  const hooks = linesMatching(/\buse[A-Z]\w*\(/);
+  assert.ok(hooks.length > 0, 'found no hooks at all — has the file moved?');
+
+  // `return` at the component's own indentation. A `return` nested inside a
+  // callback or a helper is indented further and is not an early exit from the
+  // component.
+  const returns = linesMatching(/^ {2}return\b/);
+  assert.ok(returns.length > 0, 'found no early returns — has the file moved?');
+
+  const lastHook = Math.max(...hooks);
+  const firstReturn = Math.min(...returns);
+
+  assert.ok(
+    lastHook < firstReturn,
+    `a hook on line ${lastHook} is called after the first return on line ${firstReturn}. ` +
+      'React counts hooks per component instance, so one behind a return is called on ' +
+      'some renders and not others — which unmounts the screen rather than hiding a panel.',
+  );
+});
+
+test('the early returns that caused this are still after the hooks', () => {
+  // Named explicitly, because these are the two that have actually shipped
+  // broken: the edibility guard and the resolved-score effect.
+  const hooks = linesMatching(/\buse[A-Z]\w*\(/);
+  const edibility = linesMatching(/^ {2}if \(!isFood\) return null;/);
+  assert.equal(edibility.length, 1, 'the edibility guard has moved or been renamed');
+
+  assert.ok(
+    Math.max(...hooks) < edibility[0]!,
+    'the edibility guard is back above a hook — this is the exact shape of the first crash',
+  );
+});

src/index.css

@@ -1826,7 +1826,7 @@ main {
   cursor: pointer;
 }
 
-.pcard-tick:focus-within {
+.pcard-tick:has(:focus-visible) {
   outline: 2px solid var(--color-brand);
   outline-offset: 1px;
 }