Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Sorting table by ordinal value works #989

Merged
merged 1 commit into from
Mar 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion app/configurator/components/datatable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
} from "@/graphql/query-hooks";
import SvgIcChevronDown from "@/icons/components/IcChevronDown";
import { useLocale } from "@/locales/use-locale";
import { uniqueMapBy } from "@/utils/uniqueMapBy";

const DimensionLabel = ({
dimension,
Expand Down Expand Up @@ -72,9 +73,16 @@ export const PreviewTable = ({
const sortedObservations = useMemo(() => {
if (sortBy !== undefined) {
const compare = sortDirection === "asc" ? ascending : descending;
const valuesIndex = uniqueMapBy(sortBy.values, (x) => x.label);
const convert = isNumericalMeasure(sortBy)
? (d: string) => +d
: (d: string) => d;
: (d: string) => {
const value = valuesIndex.get(d);
if (value?.position) {
return value.position;
}
return d;
};

return [...observations].sort((a, b) =>
compare(
Expand Down
15 changes: 2 additions & 13 deletions app/utils/sorting-values.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { DimensionValue } from "@/domain/data";

import { DataCubeObservationsQuery } from "../graphql/query-hooks";

import { uniqueMapBy } from "./uniqueMapBy";

const maybeInt = (value?: string | number): number | string => {
if (!value) {
return Infinity;
Expand All @@ -21,19 +23,6 @@ const maybeInt = (value?: string | number): number | string => {
return maybeInt;
};

const uniqueMapBy = <T, K>(arr: T[], keyFn: (t: T) => K) => {
const res = new Map<K, T>();
for (const item of arr) {
const key = keyFn(item);
if (res.has(key)) {
console.log(`uniqueMapBy: duplicate detected ${key}, ignoring it`);
} else {
res.set(key, item);
}
}
return res;
};

export const makeDimensionValueSorters = (
dimension:
| NonNullable<
Expand Down
12 changes: 12 additions & 0 deletions app/utils/uniqueMapBy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export const uniqueMapBy = <T, K>(arr: T[], keyFn: (t: T) => K) => {
const res = new Map<K, T>();
for (const item of arr) {
const key = keyFn(item);
if (res.has(key)) {
console.log(`uniqueMapBy: duplicate detected ${key}, ignoring it`);
} else {
res.set(key, item);
}
}
return res;
};
8 changes: 8 additions & 0 deletions e2e/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ export const createActions = ({

await selectors.datasetPreview.loaded();
},
sortBy: (columnName: string) => {
return page
.locator(`th[role=columnheader]:has-text("${columnName}") svg`)
.click();
},
},
chart: {
createFrom: async (
Expand All @@ -46,6 +51,9 @@ export const createActions = ({

await selectors.chart.loaded(chartLoadedOptions);
},
switchToTableView: async () => {
await (await selectors.chart.tablePreviewSwitch()).click();
},
},
/** Actions on MUI elements, options, selects, dialogs */
mui: {
Expand Down
16 changes: 16 additions & 0 deletions e2e/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,19 @@ export const createSelectors = ({ screen, page, within }: Ctx) => {
.first()
.waitFor({ timeout: 20 * 1000 }),
cells: () => page.locator("table td"),
headerCell: async (columnName: string) => {
return await page.locator(
`th[role=columnheader]:text("${columnName}")`
);
},
columnCells: async (columnName: string) => {
const headerCells = page.locator("th[role=columnheader]");
const headerTexts = await headerCells.allInnerTexts();
const columnIndex = headerTexts.findIndex((t) => t === columnName);
return page
.locator("tbody")
.locator(`td:nth-child(${columnIndex + 1})`);
},
},
panels: {
left: () => screen.getByTestId("panel-left"),
Expand Down Expand Up @@ -82,6 +95,9 @@ export const createSelectors = ({ screen, page, within }: Ctx) => {
await sleep(500); // Waits for tile to fade in
}
},
tablePreviewSwitch: async () => {
return await screen.findByText("Switch to table view");
},
},
};
return selectors;
Expand Down
38 changes: 38 additions & 0 deletions e2e/sorting.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,41 @@ test("Map legend categorical values sorting", async ({
"very high danger",
]);
});

const uniqueWithoutSorting = <T>(arr: T[]) => {
const res: T[] = [];
for (let i = 0; i < arr.length; i++) {
const prev = i > 0 ? arr[i - 1] : undefined;
const cur = arr[i];
if (prev !== cur) {
res.push(cur);
}
}
return res;
};

test("Map legend preview table sorting", async ({
actions,
selectors,
page,
}) => {
await actions.chart.createFrom(
"https://environment.ld.admin.ch/foen/gefahren-waldbrand-warnung/1",
"Int"
);
await actions.editor.changeChartType("Map");
await selectors.chart.loaded();

await actions.chart.switchToTableView();
await actions.datasetPreview.sortBy("Danger ratings");
const cells = await selectors.datasetPreview.columnCells("Danger ratings");

const texts = await cells.allInnerTexts();
expect(uniqueWithoutSorting(texts)).toEqual([
"low danger",
"moderate danger",
"considerable danger",
"high danger",
"very high danger",
]);
});