fix empty-value sorting edge cases and simplify sort components

This commit is contained in:
Gregor Vostrak
2026-08-26 18:11:24 +02:00
parent c9330e6cb8
commit 1edb940557
9 changed files with 67 additions and 126 deletions

View File

@@ -13,7 +13,7 @@ import {
archiveProjectViaApi,
updateOrganizationSettingViaApi,
} from './utils/api';
import { clearTableState, getTableRowNames } from './utils/table';
import { clearTableState, getSeededRowOrder } from './utils/table';
async function goToProjectsOverview(page: Page) {
await page.goto(PLAYWRIGHT_BASE_URL + '/projects');
@@ -654,14 +654,12 @@ test('test that projects without a client or estimate are ordered by name at the
page,
ctx,
}) => {
// The seeding below has to cross two second boundaries, which eats into the default
// per-test budget.
test.slow();
// Seeded a second apart: created_at only has second precision and same-second rows
// fall back to a random UUID order. The spacing makes the API order of the clientless
// rows (created_at desc: ZZZ, AAA) deterministic and different from the alphabetical
// order the name tie-break should produce.
await createProjectViaApi(ctx, { name: 'AAA Tiebreak Project' });
await page.waitForTimeout(1100);
await createProjectViaApi(ctx, { name: 'BBB Tiebreak Project' });
await page.waitForTimeout(1100);
await createProjectViaApi(ctx, { name: 'ZZZ Tiebreak Project' });
const clientAardvark = await createClientViaApi(ctx, { name: 'Aardvark Co' });
@@ -688,17 +686,11 @@ test('test that projects without a client or estimate are ordered by name at the
const seeded = [
'AAA Tiebreak Project',
'BBB Tiebreak Project',
'MMM Tiebreak Project',
'NNN Tiebreak Project',
'ZZZ Tiebreak Project',
];
const getOrder = async () => {
const rowNames = await getTableRowNames(table);
return rowNames
.map((rowName) => seeded.find((name) => rowName.includes(name)))
.filter((name): name is string => Boolean(name));
};
const getOrder = () => getSeededRowOrder(table, seeded);
// -- Client: empty rows last in both directions, alphabetical among themselves --
const clientHeader = table.locator('.select-none', { hasText: 'Client' }).first();
@@ -709,7 +701,6 @@ test('test that projects without a client or estimate are ordered by name at the
'MMM Tiebreak Project',
'NNN Tiebreak Project',
'AAA Tiebreak Project',
'BBB Tiebreak Project',
'ZZZ Tiebreak Project',
]);
@@ -720,7 +711,6 @@ test('test that projects without a client or estimate are ordered by name at the
'NNN Tiebreak Project',
'MMM Tiebreak Project',
'AAA Tiebreak Project',
'BBB Tiebreak Project',
'ZZZ Tiebreak Project',
]);
@@ -733,7 +723,6 @@ test('test that projects without a client or estimate are ordered by name at the
'NNN Tiebreak Project',
'MMM Tiebreak Project',
'AAA Tiebreak Project',
'BBB Tiebreak Project',
'ZZZ Tiebreak Project',
]);
@@ -744,7 +733,6 @@ test('test that projects without a client or estimate are ordered by name at the
'MMM Tiebreak Project',
'NNN Tiebreak Project',
'AAA Tiebreak Project',
'BBB Tiebreak Project',
'ZZZ Tiebreak Project',
]);
});

View File

@@ -15,6 +15,16 @@ export async function getTableRowNames(table: Locator): Promise<string[]> {
return names;
}
/**
* The visual order of the given seeded names within the table, ignoring any other rows.
*/
export async function getSeededRowOrder(table: Locator, seeded: string[]): Promise<string[]> {
const rowNames = await getTableRowNames(table);
return rowNames
.map((rowName) => seeded.find((name) => rowName.includes(name)))
.filter((name): name is string => Boolean(name));
}
/**
* Drop a table's persisted sort/filter state so a test starts from the defaults.
*/

View File

@@ -1,5 +1,4 @@
<script setup lang="ts">
import { computed } from 'vue';
import TableHeading from '@/Components/Common/TableHeading.vue';
import SortableTableHeaderCell from '@/Components/Common/SortableTableHeaderCell.vue';
import type { SortColumn, SortDirection } from '@/Components/Common/Client/ClientTable.vue';
@@ -10,20 +9,9 @@ const props = defineProps<{
descFirstColumns: ReadonlySet<SortColumn>;
}>();
const emit = defineEmits<{
defineEmits<{
sort: [column: SortColumn];
}>();
// Bound once per cell instead of repeating the three sort props on every column.
const sortState = computed(() => ({
sortColumn: props.sortColumn,
sortDirection: props.sortDirection,
descFirstColumns: props.descFirstColumns,
}));
function handleSort(column: SortColumn) {
emit('sort', column);
}
</script>
<template>
@@ -31,14 +19,17 @@ function handleSort(column: SortColumn) {
<SortableTableHeaderCell
class="pr-3 pl-4 sm:pl-6 lg:pl-8 3xl:pl-12"
column="name"
v-bind="sortState"
@sort="handleSort">
v-bind="props"
@sort="$emit('sort', $event)">
Name
</SortableTableHeaderCell>
<SortableTableHeaderCell column="projects_count" v-bind="sortState" @sort="handleSort">
<SortableTableHeaderCell
column="projects_count"
v-bind="props"
@sort="$emit('sort', $event)">
Projects
</SortableTableHeaderCell>
<SortableTableHeaderCell column="status" v-bind="sortState" @sort="handleSort">
<SortableTableHeaderCell column="status" v-bind="props" @sort="$emit('sort', $event)">
Status
</SortableTableHeaderCell>
<div class="relative py-1.5 pl-3 pr-4 sm:pr-6 lg:pr-8 3xl:pr-12">

View File

@@ -1,5 +1,4 @@
<script setup lang="ts">
import { computed } from 'vue';
import TableHeading from '@/Components/Common/TableHeading.vue';
import SortableTableHeaderCell from '@/Components/Common/SortableTableHeaderCell.vue';
import type { SortColumn, SortDirection } from '@/Components/Common/Member/MemberTable.vue';
@@ -10,20 +9,9 @@ const props = defineProps<{
descFirstColumns: ReadonlySet<SortColumn>;
}>();
const emit = defineEmits<{
defineEmits<{
sort: [column: SortColumn];
}>();
// Bound once per cell instead of repeating the three sort props on every column.
const sortState = computed(() => ({
sortColumn: props.sortColumn,
sortDirection: props.sortDirection,
descFirstColumns: props.descFirstColumns,
}));
function handleSort(column: SortColumn) {
emit('sort', column);
}
</script>
<template>
@@ -31,20 +19,23 @@ function handleSort(column: SortColumn) {
<SortableTableHeaderCell
class="pr-3 pl-4 sm:pl-6 lg:pl-8 3xl:pl-12"
column="name"
v-bind="sortState"
@sort="handleSort">
v-bind="props"
@sort="$emit('sort', $event)">
Name
</SortableTableHeaderCell>
<SortableTableHeaderCell column="email" v-bind="sortState" @sort="handleSort">
<SortableTableHeaderCell column="email" v-bind="props" @sort="$emit('sort', $event)">
Email
</SortableTableHeaderCell>
<SortableTableHeaderCell column="role" v-bind="sortState" @sort="handleSort">
<SortableTableHeaderCell column="role" v-bind="props" @sort="$emit('sort', $event)">
Role
</SortableTableHeaderCell>
<SortableTableHeaderCell column="billable_rate" v-bind="sortState" @sort="handleSort">
<SortableTableHeaderCell
column="billable_rate"
v-bind="props"
@sort="$emit('sort', $event)">
Billable Rate
</SortableTableHeaderCell>
<SortableTableHeaderCell column="status" v-bind="sortState" @sort="handleSort">
<SortableTableHeaderCell column="status" v-bind="props" @sort="$emit('sort', $event)">
Status
</SortableTableHeaderCell>
<div class="relative py-1.5 pl-3 pr-4 sm:pr-6 lg:pr-8 3xl:pr-12 bg-row-heading-background">

View File

@@ -87,7 +87,7 @@ const columns = computed<SortableColumnDef<Project, SortColumn>[]>(() => [
{
id: 'billable_rate',
sortDescFirst: true,
accessorFn: (row: Project) => row.billable_rate ?? 0,
accessorFn: (row: Project) => row.billable_rate,
},
{
id: 'status',

View File

@@ -1,6 +1,7 @@
<script setup lang="ts" generic="TColumn extends string">
import { computed, useAttrs } from 'vue';
import { twMerge } from 'tailwind-merge';
import type { ClassValue } from 'clsx';
import { cn } from '@/lib/utils';
import { ChevronUpIcon, ChevronDownIcon } from '@heroicons/vue/16/solid';
import type { SortDirection } from '@/utils/useSortableTable';
@@ -29,24 +30,15 @@ const isChevronDown = computed(() => {
});
const cellClass = computed(() =>
twMerge(
cn(
'px-3 py-1.5 text-left text-text-tertiary cursor-pointer hover:bg-secondary hover:text-text-primary transition-colors select-none flex items-center gap-1',
attrs.class as string | undefined
attrs.class as ClassValue
)
);
const passthroughAttrs = computed(() => {
const { class: _class, ...rest } = attrs;
return rest;
});
</script>
<template>
<button
v-bind="passthroughAttrs"
type="button"
:class="cellClass"
@click="emit('sort', column)">
<button type="button" :class="cellClass" @click="emit('sort', column)">
<slot></slot>
<span class="sr-only">
{{

View File

@@ -1,5 +1,4 @@
<script setup lang="ts">
import { computed } from 'vue';
import TableHeading from '@/Components/Common/TableHeading.vue';
import SortableTableHeaderCell from '@/Components/Common/SortableTableHeaderCell.vue';
import type { SortColumn, SortDirection } from '@/Components/Common/Tag/TagTable.vue';
@@ -10,20 +9,9 @@ const props = defineProps<{
descFirstColumns: ReadonlySet<SortColumn>;
}>();
const emit = defineEmits<{
defineEmits<{
sort: [column: SortColumn];
}>();
// Bound once per cell instead of repeating the three sort props on every column.
const sortState = computed(() => ({
sortColumn: props.sortColumn,
sortDirection: props.sortDirection,
descFirstColumns: props.descFirstColumns,
}));
function handleSort(column: SortColumn) {
emit('sort', column);
}
</script>
<template>
@@ -31,8 +19,8 @@ function handleSort(column: SortColumn) {
<SortableTableHeaderCell
class="pr-3 pl-4 sm:pl-6 lg:pl-8 3xl:pl-12"
column="name"
v-bind="sortState"
@sort="handleSort">
v-bind="props"
@sort="$emit('sort', $event)">
Name
</SortableTableHeaderCell>
<div class="relative py-1.5 pl-3 pr-4 sm:pr-6 lg:pr-8 3xl:pr-12">

View File

@@ -1,5 +1,4 @@
<script setup lang="ts">
import { computed } from 'vue';
import TableHeading from '@/Components/Common/TableHeading.vue';
import SortableTableHeaderCell from '@/Components/Common/SortableTableHeaderCell.vue';
import type { SortColumn, SortDirection } from '@/Components/Common/Task/TaskTable.vue';
@@ -10,20 +9,9 @@ const props = defineProps<{
descFirstColumns: ReadonlySet<SortColumn>;
}>();
const emit = defineEmits<{
defineEmits<{
sort: [column: SortColumn];
}>();
// Bound once per cell instead of repeating the three sort props on every column.
const sortState = computed(() => ({
sortColumn: props.sortColumn,
sortDirection: props.sortDirection,
descFirstColumns: props.descFirstColumns,
}));
function handleSort(column: SortColumn) {
emit('sort', column);
}
</script>
<template>
@@ -31,14 +19,14 @@ function handleSort(column: SortColumn) {
<SortableTableHeaderCell
class="pr-3 pl-4 sm:pl-6 lg:pl-8 3xl:pl-12"
column="name"
v-bind="sortState"
@sort="handleSort">
v-bind="props"
@sort="$emit('sort', $event)">
Task Name
</SortableTableHeaderCell>
<SortableTableHeaderCell column="spent_time" v-bind="sortState" @sort="handleSort">
<SortableTableHeaderCell column="spent_time" v-bind="props" @sort="$emit('sort', $event)">
Total Time
</SortableTableHeaderCell>
<SortableTableHeaderCell column="progress" v-bind="sortState" @sort="handleSort">
<SortableTableHeaderCell column="progress" v-bind="props" @sort="$emit('sort', $event)">
Progress
</SortableTableHeaderCell>
<div class="px-3 py-1.5 text-left text-text-tertiary">Status</div>

View File

@@ -13,26 +13,23 @@ import { computed, type ComputedRef } from 'vue';
export type SortDirection = 'asc' | 'desc';
/**
* The comparator every sortable column gets: a row whose accessor returns `undefined`
* sorts to the bottom in both directions, and two such rows compare as equal so the
* tie-break decides their order.
*
* TanStack cannot express that combination. Its `sortUndefined: 'last'` keeps empty rows
* at the bottom but returns a non-zero result even when both rows are empty, which
* swallows the tie-break and leaves those rows in API (created_at) order; its default of
* `1` returns zero for that case but flips empty rows to the top when descending.
* Comparator for every sortable column: empty values (`null`/`undefined`) sort last in
* both directions and compare as equal, so the tie-break orders them. TanStack's own
* `sortUndefined` cannot do this: `'last'` bypasses the tie-break and the default flips
* empty rows to the top when descending.
*/
function sortEmptyLast<TData>(getDirection: () => SortDirection): SortingFn<TData> {
function sortEmptyLast<TData>(getSorting: () => SortingState): SortingFn<TData> {
return (rowA: Row<TData>, rowB: Row<TData>, columnId: string) => {
const a = rowA.getValue(columnId);
const b = rowB.getValue(columnId);
if (a === undefined && b === undefined) {
if (a == null && b == null) {
return 0;
}
if (a === undefined || b === undefined) {
const emptyLast = a === undefined ? 1 : -1;
return getDirection() === 'desc' ? -emptyLast : emptyLast;
if (a == null || b == null) {
const emptyLast = a == null ? 1 : -1;
const desc = getSorting().find((entry) => entry.id === columnId)?.desc ?? false;
return desc ? -emptyLast : emptyLast;
}
return typeof a === 'string' || typeof b === 'string'
? sortingFns.alphanumeric(rowA, rowB, columnId)
@@ -41,13 +38,13 @@ function sortEmptyLast<TData>(getDirection: () => SortDirection): SortingFn<TDat
}
/**
* A column definition whose `id` has to be one of the table's sortable columns, so a
* renamed or mistyped id is a compile error rather than a column that silently stops
* sorting: TanStack drops sort entries for ids it cannot resolve, leaving the rows in
* their original order with no chevron and no error.
* Requires `id` to be one of the table's sortable columns, so a mistyped id is a compile
* error instead of a column that silently stops sorting. `sortingFn` is forbidden
* because the composable always installs its own comparator.
*/
export type SortableColumnDef<TData, TColumn extends string> = ColumnDef<TData, unknown> & {
id: TColumn;
sortingFn?: never;
};
export function useSortableTable<TData, TColumn extends string>(options: {
@@ -72,15 +69,11 @@ export function useSortableTable<TData, TColumn extends string>(options: {
]);
const resolvedColumns = computed<ColumnDef<TData, unknown>[]>(() =>
options.columns().map((column) =>
column.sortingFn
? column
: {
...column,
sortUndefined: false as const,
sortingFn: sortEmptyLast<TData>(options.sortDirection),
}
)
options.columns().map((column) => ({
...column,
sortUndefined: false as const,
sortingFn: sortEmptyLast<TData>(() => sorting.value),
}))
);
const descFirstColumns = computed<ReadonlySet<TColumn>>(