From d6e0942487dbb902370c48d0a9e657e27f364a54 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 25 Aug 2026 13:20:28 -0500 Subject: [PATCH] feat(filters): show a tag's own colour on its active filter chip (#185) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tag carries a colour, and every place a tag appears shows it — a product card, the filter drawer's control, the admin taxonomy screen — except the removable chips beside the Filters button, which rendered every filter as a default grey. Picking `vintage` from a control that showed it in red produced a grey chip of the same name right next to it. Only tags get a colour, because only tags have one. Category, price, favorites and status keep the default, and that asymmetry is the point: in a row mixing four kinds of filter, colour now means "this is a tag". Nothing depends on it — every chip still carries its label — so this reads the same to anyone who cannot distinguish the colours. The close control inherits the tag's text colour, so a coloured chip gets a matching cross rather than a grey one on a coloured ground. A tag not yet in the loaded options has no colour to use and keeps the default, which is the same window the existing `Tag {id}` label fallback covers. The test asserts the chip's colour equals the same tag's colour on a product card, rather than asserting it is red. The colour is derived from the tag's name and free to change; what must hold is that a tag looks like itself wherever it appears, and comparing the two places says that directly. Confirmed to fail without the change — the old grey chip sets no colour class at all. Verified visually as well as by assertion: three tags selected together render in the drawer, in the chip row and on the card in the same colours. Closes #185 --- frontend/src/components/ActiveFilterChips.tsx | 16 +++++++++- frontend/tests/e2e/filters.spec.ts | 29 +++++++++++++++++++ frontend/tests/e2e/pages/StorefrontPage.ts | 16 ++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/ActiveFilterChips.tsx b/frontend/src/components/ActiveFilterChips.tsx index 32a5afe..f12c508 100644 --- a/frontend/src/components/ActiveFilterChips.tsx +++ b/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) => ( { event.preventDefault(); diff --git a/frontend/tests/e2e/filters.spec.ts b/frontend/tests/e2e/filters.spec.ts index e37686a..f9c26f5 100644 --- a/frontend/tests/e2e/filters.spec.ts +++ b/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(); diff --git a/frontend/tests/e2e/pages/StorefrontPage.ts b/frontend/tests/e2e/pages/StorefrontPage.ts index 4bcb533..a7b4f7b 100644 --- a/frontend/tests/e2e/pages/StorefrontPage.ts +++ b/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-` 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 { await this.activeFilters.getByRole('button', { name: 'Clear all' }).click(); }