fix(filters): give the category tree selectable values, and drive both controls by search (#139)
Three things the e2e run found.
`toTreeData` still emitted `key`, which is how an antd `Tree` identifies a node and not how a `TreeSelect` selects one. Nothing could be picked, and `treeNodeFilterProp="title"` had nothing to filter against. It now emits `value`, matching the admin's CategoryTreeSelect.
The page object typed the name before clicking it, in both controls. Not for realism: the option lists are virtualized, so against a database holding hundreds of categories the wanted row never renders until a search narrows to it, and scrolling to it would be testing the virtual list rather than the filter.
Tag options are matched by class rather than by role, for the reason AdminInventory.toggleStatus already records — antd renders an invisible role="listbox" shim beside the real list, so getByRole('option') resolves to something zero-sized that can never be clicked. The drawer's own title is a plain element rather than a heading, so the click that closes an option list lands on the Favorites heading instead.
The multi-category URL assertion accepts the comma either percent-encoded or literal. URLSearchParams encodes it, which is how the `tags` parameter has always looked, and both spellings parse.
This commit is contained in:
@@ -8,7 +8,6 @@ import InputNumber from 'antd/es/input-number';
|
||||
import Empty from 'antd/es/empty';
|
||||
import Switch from 'antd/es/switch';
|
||||
import Grid from 'antd/es/grid';
|
||||
import type { DataNode } from 'antd/es/tree';
|
||||
import type { FilterOptions } from '../api';
|
||||
import { ItemFilters, buildCategoryTree, CategoryNode } from '../filters';
|
||||
|
||||
@@ -22,9 +21,18 @@ type Props = Readonly<{
|
||||
resultCount: number;
|
||||
}>;
|
||||
|
||||
function toTreeData(nodes: CategoryNode[]): DataNode[] {
|
||||
// `value` rather than `key`: this fed an antd `Tree`, which identifies nodes by
|
||||
// key, and now feeds a `TreeSelect`, which selects and searches by value. The
|
||||
// shape matches the admin's CategoryTreeSelect so the two stay comparable.
|
||||
interface CategoryTreeOption {
|
||||
value: number;
|
||||
title: string;
|
||||
children?: CategoryTreeOption[];
|
||||
}
|
||||
|
||||
function toTreeData(nodes: CategoryNode[]): CategoryTreeOption[] {
|
||||
return nodes.map((node) => ({
|
||||
key: node.id,
|
||||
value: node.id,
|
||||
title: node.name,
|
||||
children: node.children.length ? toTreeData(node.children) : undefined
|
||||
}));
|
||||
|
||||
@@ -107,7 +107,12 @@ test.describe('Storefront filters', () => {
|
||||
|
||||
// Both ride in the one parameter the filter has always used, so links
|
||||
// written before it went multi-valued still mean what they meant.
|
||||
await expect(page).toHaveURL(/category=\d+,\d+/);
|
||||
//
|
||||
// The separator arrives percent-encoded because URLSearchParams encodes a
|
||||
// comma, which is how the `tags` parameter has always looked too. Either
|
||||
// spelling parses, so the assertion accepts both rather than pinning the
|
||||
// encoding.
|
||||
await expect(page).toHaveURL(/category=\d+(,|%2C)\d+/);
|
||||
});
|
||||
|
||||
test('each selected category gets its own removable chip', async ({ storefront, filterDrawer }) => {
|
||||
|
||||
@@ -7,9 +7,18 @@ import { Locator, Page, expect } from '@playwright/test';
|
||||
* while the list is open, so every choose/toggle here opens the list, acts, and
|
||||
* closes it again.
|
||||
*
|
||||
* Each one types the name before clicking it. Not for realism — the lists are
|
||||
* virtualized, so against a database holding hundreds of categories the wanted
|
||||
* row is never rendered until a search narrows to it. Scrolling to it would be
|
||||
* testing the virtual list rather than the filter.
|
||||
*
|
||||
* Categories keep their hierarchy — a category filter matches the node and
|
||||
* everything filed beneath it — so their options are `treeitem`. Tags are flat,
|
||||
* so theirs are `option`.
|
||||
* everything filed beneath it — so their options are `treeitem`.
|
||||
*
|
||||
* Tags are flat, and their options are matched by class rather than by role,
|
||||
* for the reason AdminInventory.toggleStatus records: antd renders an invisible
|
||||
* role="listbox" shim beside the real list, so getByRole('option') resolves to
|
||||
* something zero-sized that can never be clicked.
|
||||
*
|
||||
* The two follow opposite rules, which is the thing several tests exist to pin
|
||||
* down: categories are OR (any of the selected branches), tags are AND (the
|
||||
@@ -33,7 +42,9 @@ export class FilterDrawer {
|
||||
this.favoritesOnlySwitch = page.getByRole('switch', { name: 'Only my favorites' });
|
||||
this.categorySelect = page.getByRole('combobox', { name: 'Filter by category' });
|
||||
this.tagSelect = page.getByRole('combobox', { name: 'Filter by tags' });
|
||||
this.title = page.getByRole('heading', { name: 'Filters' });
|
||||
// Somewhere inside the drawer that is inert and never covered by an
|
||||
// option list, which opens downward from the controls below it.
|
||||
this.title = page.getByRole('heading', { name: 'Favorites' });
|
||||
}
|
||||
|
||||
category(name: string): Locator {
|
||||
@@ -41,7 +52,7 @@ export class FilterDrawer {
|
||||
}
|
||||
|
||||
tag(name: string): Locator {
|
||||
return this.page.getByRole('option', { name });
|
||||
return this.page.locator(`.ant-select-item-option[title="${name}"]`);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -55,6 +66,17 @@ export class FilterDrawer {
|
||||
await this.categorySelect.click();
|
||||
}
|
||||
|
||||
/**
|
||||
* Narrows to one category and clicks it, with the list already open.
|
||||
*
|
||||
* Selecting clears the search box, so consecutive calls each start from the
|
||||
* unfiltered list.
|
||||
*/
|
||||
private async pickCategory(name: string): Promise<void> {
|
||||
await this.categorySelect.fill(name);
|
||||
await this.category(name).click();
|
||||
}
|
||||
|
||||
/**
|
||||
* Closes whichever option list is open, without closing the drawer.
|
||||
*
|
||||
@@ -67,7 +89,7 @@ export class FilterDrawer {
|
||||
|
||||
async chooseCategory(name: string): Promise<void> {
|
||||
await this.openCategoryList();
|
||||
await this.category(name).click();
|
||||
await this.pickCategory(name);
|
||||
await this.closeOptionList();
|
||||
}
|
||||
|
||||
@@ -75,7 +97,7 @@ export class FilterDrawer {
|
||||
async chooseCategories(...names: string[]): Promise<void> {
|
||||
await this.openCategoryList();
|
||||
for (const name of names) {
|
||||
await this.category(name).click();
|
||||
await this.pickCategory(name);
|
||||
}
|
||||
await this.closeOptionList();
|
||||
}
|
||||
@@ -83,6 +105,7 @@ export class FilterDrawer {
|
||||
/** Clicking a selected option in a multi-select deselects it, so this toggles. */
|
||||
async toggleTag(name: string): Promise<void> {
|
||||
await this.tagSelect.click();
|
||||
await this.tagSelect.fill(name);
|
||||
await this.tag(name).click();
|
||||
await this.closeOptionList();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user