feat(filters): show a tag's own colour on its active filter chip (#185) #194
@@ -25,7 +25,11 @@ export default function ActiveFilterChips({
|
|||||||
}: Props) {
|
}: Props) {
|
||||||
if (!hasActiveFilters(filters)) return null;
|
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
|
// 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.
|
// chip most worth noticing when a customer wonders why the grid looks short.
|
||||||
@@ -59,6 +63,11 @@ export default function ActiveFilterChips({
|
|||||||
chips.push({
|
chips.push({
|
||||||
key: `tag-${tagId}`,
|
key: `tag-${tagId}`,
|
||||||
label: tag?.name ?? `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) })
|
onRemove: () => onChange({ ...filters, tagIds: filters.tagIds.filter((id) => id !== tagId) })
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -93,6 +102,11 @@ export default function ActiveFilterChips({
|
|||||||
{chips.map((chip) => (
|
{chips.map((chip) => (
|
||||||
<Tag
|
<Tag
|
||||||
key={chip.key}
|
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
|
closable
|
||||||
onClose={(event) => {
|
onClose={(event) => {
|
||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
|
|||||||
@@ -193,6 +193,35 @@ test.describe('Storefront filters', () => {
|
|||||||
await expect(storefront.removeFilterChip(NAMES.furniture)).toBeVisible();
|
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 }) => {
|
test("shows an item's tags on its card", async ({ storefront, filterDrawer }) => {
|
||||||
await storefront.goto();
|
await storefront.goto();
|
||||||
await storefront.openFilters();
|
await storefront.openFilters();
|
||||||
|
|||||||
@@ -135,6 +135,22 @@ export class StorefrontPage {
|
|||||||
return this.page.getByRole('button', { name: `Remove filter ${name}` });
|
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> {
|
async clearAllFilters(): Promise<void> {
|
||||||
await this.activeFilters.getByRole('button', { name: 'Clear all' }).click();
|
await this.activeFilters.getByRole('button', { name: 'Clear all' }).click();
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user