Merge pull request 'feat(filters): show a tag's own colour on its active filter chip (#185)' (#187) from feature/185-tag-chip-colour into main

Reviewed-on: #187

frontend/src/components/ActiveFilterChips.tsx

@@ -25,7 +25,11 @@ export default function ActiveFilterChips({
 }: Props) {
   if (!hasActiveFilters(filters)) return null;
 
-  const chips: { key: string; label: string; onRemove: () => void }[] = [];
+  // `color` only ever set for tags, which are the only filter with one. That
+  // makes colour in this row mean "this is a tag", which is a useful thing for
+  // a row mixing four kinds of filter to say — and nothing depends on it, since
+  // every chip still carries its label.
+  const chips: { key: string; label: string; color?: string; onRemove: () => void }[] = [];
 
   // Listed first so it matches the drawer's ordering, and because it is the
   // chip most worth noticing when a customer wonders why the grid looks short.
@@ -59,6 +63,11 @@ export default function ActiveFilterChips({
     chips.push({
       key: `tag-${tagId}`,
       label: tag?.name ?? `Tag ${tagId}`,
+      // The same colour the drawer's control and the product cards show, so a
+      // tag looks like itself wherever it appears. Undefined while
+      // /api/filters is still loading, which is the case the label fallback
+      // above already covers — an uncoloured chip beats a missing one.
+      color: tag?.color,
       onRemove: () => onChange({ ...filters, tagIds: filters.tagIds.filter((id) => id !== tagId) })
     });
   }
@@ -93,6 +102,11 @@ export default function ActiveFilterChips({
       {chips.map((chip) => (
         <Tag
           key={chip.key}
+          // Undefined for every filter that has no colour of its own, which is
+          // antd's default rendering — the same as before this distinguished
+          // tags. The custom close icon below inherits the tag's text colour,
+          // so a coloured chip gets a matching cross rather than a grey one.
+          color={chip.color}
           closable
           onClose={(event) => {
             event.preventDefault();

frontend/tests/e2e/filters.spec.ts

@@ -193,6 +193,35 @@ test.describe('Storefront filters', () => {
     await expect(storefront.removeFilterChip(NAMES.furniture)).toBeVisible();
   });
 
+  // #185. The requirement is not "the chip is red", which would pin a colour
+  // that is derived from the tag's name and free to change — it is that a tag
+  // looks like itself wherever it appears. So the chip is compared against the
+  // same tag on a product card rather than against a literal.
+  test('shows a selected tag in its own colour, the same one the card uses', async ({
+    storefront,
+    filterDrawer
+  }) => {
+    await storefront.goto();
+    await storefront.openFilters();
+    await filterDrawer.toggleTag(NAMES.vintage);
+    await filterDrawer.close();
+
+    const chip = storefront.filterChip(NAMES.vintage);
+    await expect(chip).toBeVisible();
+
+    const onCard = storefront.cardTag(NAMES.midItem, NAMES.vintage);
+    await expect(onCard).toBeVisible();
+
+    const colourClass = (classes: string | null): string | undefined =>
+      (classes ?? '').split(/\s+/).find((name) => /^ant-tag-[a-z]+$/.test(name));
+
+    const chipColour = colourClass(await chip.getAttribute('class'));
+    // Coloured at all: before #185 every chip in this row rendered grey, which
+    // sets no colour class and would leave this undefined.
+    expect(chipColour).toBeDefined();
+    expect(chipColour).toBe(colourClass(await onCard.getAttribute('class')));
+  });
+
   test("shows an item's tags on its card", async ({ storefront, filterDrawer }) => {
     await storefront.goto();
     await storefront.openFilters();

frontend/tests/e2e/pages/StorefrontPage.ts

@@ -135,6 +135,22 @@ export class StorefrontPage {
     return this.page.getByRole('button', { name: `Remove filter ${name}` });
   }
 
+  /**
+   * The chip itself rather than its remove button, for asserting how it looks.
+   *
+   * antd expresses a tag's colour as an `ant-tag-<colour>` class, so the chip
+   * element is what carries it — the button inside only inherits the text
+   * colour.
+   */
+  filterChip(name: string): Locator {
+    return this.activeFilters.locator('.ant-tag').filter({ hasText: name });
+  }
+
+  /** A tag as rendered on a product card, which is where its colour is set. */
+  cardTag(itemName: string, tagName: string): Locator {
+    return this.card(itemName).locator('.ant-tag').filter({ hasText: tagName });
+  }
+
   async clearAllFilters(): Promise<void> {
     await this.activeFilters.getByRole('button', { name: 'Clear all' }).click();
   }