From 6dac99d746f4d0991b00fe8aec50c6ea1ef1ae39 Mon Sep 17 00:00:00 2001 From: Cameron Custer <73217097+cameroncuster@users.noreply.github.com> Date: Thu, 23 Jul 2026 21:13:30 +0000 Subject: [PATCH] refactor(tables): extract shared behavior --- src/lib/components/ContestTable.svelte | 220 +++-------------- src/lib/components/ProblemDisplay.svelte | 70 ++---- src/lib/components/ProblemTable.svelte | 232 +++--------------- .../ResponsiveTableContainer.svelte | 82 +++++++ .../components/TableFeedbackButtons.svelte | 94 +++++++ src/lib/utils/table.ts | 86 +++++++ src/routes/contests/+page.svelte | 75 ++---- tests/table-markup.test.ts | 49 ++-- tests/table-utils.test.ts | 106 ++++++++ 9 files changed, 491 insertions(+), 523 deletions(-) create mode 100644 src/lib/components/ResponsiveTableContainer.svelte create mode 100644 src/lib/components/TableFeedbackButtons.svelte create mode 100644 src/lib/utils/table.ts create mode 100644 tests/table-utils.test.ts diff --git a/src/lib/components/ContestTable.svelte b/src/lib/components/ContestTable.svelte index fc7430e..43193db 100644 --- a/src/lib/components/ContestTable.svelte +++ b/src/lib/components/ContestTable.svelte @@ -3,7 +3,17 @@ import type { Contest } from '$lib/services/contest'; import { formatDuration } from '$lib/services/contest'; import { user } from '$lib/services/auth'; import { createEventDispatcher } from 'svelte'; +import { + cycleTableState, + getDifficultyAriaSort, + getDifficultySortLabel, + getSortedAuthors, + nextSortDirection, + type SortDirection +} from '$lib/utils/table'; import RecommendersFilter from './RecommendersFilter.svelte'; +import ResponsiveTableContainer from './ResponsiveTableContainer.svelte'; +import TableFeedbackButtons from './TableFeedbackButtons.svelte'; // Event dispatcher const dispatch = createEventDispatcher(); @@ -25,17 +35,17 @@ $: isAuthenticated = !!$user; // Filter states let difficultyFilter: number | null = null; -let difficultySortDirection: 'asc' | 'desc' | null = null; +let difficultySortDirection: SortDirection = null; let participatedFilterState: 'participated' | 'not-participated' | 'all' = 'all'; let authorFilter: string | null = null; let typeFilterState: 'all' | 'icpc' | 'codeforces' = 'all'; +const PARTICIPATION_FILTER_STATES = ['all', 'participated', 'not-participated'] as const; +const TYPE_FILTER_STATES = ['all', 'icpc', 'codeforces'] as const; + // Get unique authors for filter dropdown // If allAuthors is provided, use it; otherwise, fall back to extracting from current contests -$: uniqueAuthors = - allAuthors.length > 0 - ? [...allAuthors].sort() - : [...new Set(contests.map((contest) => contest.addedBy))].sort(); +$: uniqueAuthors = getSortedAuthors(contests, allAuthors); // Apply filters to contests $: filteredContests = contests.filter((contest) => { @@ -91,66 +101,27 @@ $: typeFilterLabel = ? 'Filter by contest type (showing ICPC)' : 'Filter by contest type (showing Codeforces)'; -$: difficultyAriaSort = ( - difficultySortDirection === 'asc' - ? 'ascending' - : difficultySortDirection === 'desc' - ? 'descending' - : 'none' -) as 'ascending' | 'descending' | 'none'; - -$: difficultySortLabel = - difficultySortDirection === 'asc' - ? 'Difficulty, sorted ascending. Activate to sort descending.' - : difficultySortDirection === 'desc' - ? 'Difficulty, sorted descending. Activate to clear sorting.' - : 'Difficulty, not sorted. Activate to sort ascending.'; +$: difficultyAriaSort = getDifficultyAriaSort(difficultySortDirection); +$: difficultySortLabel = getDifficultySortLabel(difficultySortDirection); -// Handle filter changes function handleParticipatedFilter() { - if (participatedFilterState === 'all') { - participatedFilterState = 'participated'; - } else if (participatedFilterState === 'participated') { - participatedFilterState = 'not-participated'; - } else { - participatedFilterState = 'all'; - } - - // Dispatch event to parent component + participatedFilterState = cycleTableState( + participatedFilterState, + PARTICIPATION_FILTER_STATES + ); dispatch('filterParticipated', { state: participatedFilterState }); } -// Handle contest type filter function handleTypeFilter() { - if (typeFilterState === 'all') { - typeFilterState = 'icpc'; - } else if (typeFilterState === 'icpc') { - typeFilterState = 'codeforces'; - } else { - typeFilterState = 'all'; - } - - // Dispatch event to parent component + typeFilterState = cycleTableState(typeFilterState, TYPE_FILTER_STATES); dispatch('filterType', { type: typeFilterState }); } -// Handle difficulty sort function handleDifficultySort() { - // Toggle sort direction: null -> asc -> desc -> null - if (difficultySortDirection === null) { - difficultySortDirection = 'asc'; - } else if (difficultySortDirection === 'asc') { - difficultySortDirection = 'desc'; - } else { - difficultySortDirection = null; - } - - // Dispatch event to parent component + difficultySortDirection = nextSortDirection(difficultySortDirection); dispatch('sortDifficulty', { direction: difficultySortDirection }); } -// Function to handle author filter is now handled directly in the RecommendersFilter component - // Generate star rating display function getDifficultyStars(difficulty: number | undefined): string { if (difficulty === undefined) return ''; @@ -185,10 +156,7 @@ function getDifficultyLabel(difficulty: number | undefined): string { } -
-
+ @@ -459,86 +427,15 @@ function getDifficultyLabel(difficulty: number | undefined): string { @@ -552,58 +449,9 @@ function getDifficultyLabel(difficulty: number | undefined): string { {/each}
{#if contest.id} - {@const hasLiked = userFeedback[contest.id] === 'like'} - {@const hasDisliked = userFeedback[contest.id] === 'dislike'} - - - - - - + onLike(contest.id!, isLike)} + /> {/if}
-
-
+ diff --git a/src/lib/components/TableFeedbackButtons.svelte b/src/lib/components/TableFeedbackButtons.svelte new file mode 100644 index 0000000..b953b2a --- /dev/null +++ b/src/lib/components/TableFeedbackButtons.svelte @@ -0,0 +1,94 @@ + + + + + + + diff --git a/src/lib/utils/table.ts b/src/lib/utils/table.ts new file mode 100644 index 0000000..990bc8b --- /dev/null +++ b/src/lib/utils/table.ts @@ -0,0 +1,86 @@ +export type SortDirection = 'asc' | 'desc' | null; + +export type ScoreSortable = { + id?: string; + likes: number; + dislikes: number; +}; + +export type DifficultySortable = { + difficulty?: number; +}; + +export type Authored = { + addedBy: string; +}; + +export function cycleTableState(current: T, states: readonly T[]): T { + const currentIndex = states.indexOf(current); + return states[(currentIndex + 1) % states.length]; +} + +export function nextSortDirection(direction: SortDirection): SortDirection { + return cycleTableState(direction, [null, 'asc', 'desc']); +} + +export function getDifficultyAriaSort( + direction: SortDirection +): 'ascending' | 'descending' | 'none' { + if (direction === 'asc') return 'ascending'; + if (direction === 'desc') return 'descending'; + return 'none'; +} + +export function getDifficultySortLabel(direction: SortDirection): string { + if (direction === 'asc') { + return 'Difficulty, sorted ascending. Activate to sort descending.'; + } + if (direction === 'desc') { + return 'Difficulty, sorted descending. Activate to clear sorting.'; + } + return 'Difficulty, not sorted. Activate to sort ascending.'; +} + +export function sortByScore( + items: readonly T[], + direction: Exclude +): T[] { + const itemsByScore = new Map(); + + for (const item of items) { + const score = item.likes - item.dislikes; + const group = itemsByScore.get(score) ?? []; + group.push(item); + itemsByScore.set(score, group); + } + + for (const group of itemsByScore.values()) { + group.sort((a, b) => (a.id && b.id ? a.id.localeCompare(b.id) : 0)); + } + + return [...itemsByScore.keys()] + .sort((a, b) => (direction === 'asc' ? a - b : b - a)) + .flatMap((score) => itemsByScore.get(score) ?? []); +} + +export function sortByDifficulty( + items: readonly T[], + direction: SortDirection, + resetSort: (items: readonly T[]) => T[] +): T[] { + if (direction === null) return resetSort(items); + + return [...items].sort((a, b) => { + const difference = (a.difficulty ?? 0) - (b.difficulty ?? 0); + return direction === 'asc' ? difference : -difference; + }); +} + +export function getSortedAuthors( + items: readonly T[], + providedAuthors: readonly string[] = [] +): string[] { + return providedAuthors.length > 0 + ? [...providedAuthors].sort() + : [...new Set(items.map((item) => item.addedBy))].sort(); +} diff --git a/src/routes/contests/+page.svelte b/src/routes/contests/+page.svelte index 4bea146..c7773f0 100644 --- a/src/routes/contests/+page.svelte +++ b/src/routes/contests/+page.svelte @@ -12,6 +12,12 @@ import type { Contest } from '$lib/services/contest'; import type { PageData } from './$types'; import { user } from '$lib/services/auth'; import { SvelteSet } from 'svelte/reactivity'; +import { + getSortedAuthors, + sortByDifficulty, + sortByScore, + type SortDirection +} from '$lib/utils/table'; // Contests provided by the server-side load so the initial render (including // SSR) ships with data instead of waiting for a client-side fetch. @@ -72,7 +78,7 @@ function updateAvailableAuthors(): void { const contestsWithoutAuthorFilter = getContestsWithoutAuthorFilter(); // Update available authors based on current filters - availableAuthors = [...new Set(contestsWithoutAuthorFilter.map((c) => c.addedBy))].sort(); + availableAuthors = getSortedAuthors(contestsWithoutAuthorFilter); } // Function to handle participation filter @@ -144,70 +150,21 @@ async function loadContests() { } } -// Function to sort contests by difficulty function sortContestsByDifficulty( - contestsToSort: Contest[], - direction: 'asc' | 'desc' | null + contestsToSort: readonly Contest[], + direction: SortDirection ): Contest[] { - if (direction === null) { - // If no direction specified, return to default sort (by likes) - return sortContestsByLikes(contestsToSort, 'desc'); - } - - return [...contestsToSort].sort((a, b) => { - // Handle undefined difficulties - const diffA = a.difficulty ?? 0; - const diffB = b.difficulty ?? 0; - - // Sort based on direction - return direction === 'asc' ? diffA - diffB : diffB - diffA; - }); + return sortByDifficulty(contestsToSort, direction, (contestsToReset) => + sortContestsByLikes(contestsToReset, 'desc') + ); } -// Function to calculate contest score (likes - dislikes) -function calculateScore(contest: Contest): number { - return contest.likes - contest.dislikes; -} - -// Function to sort contests by score (likes - dislikes) function sortContestsByLikes( - contestsToSort: Contest[], - direction: 'asc' | 'desc' | null + contestsToSort: readonly Contest[], + direction: SortDirection ): Contest[] { - if (direction === null) { - // If no direction specified, return to default sort (original order) - return [...contests]; - } - - // Group contests by score - const contestsByScore: Record = {}; - - // Calculate score for each contest and group them - contestsToSort.forEach((contest) => { - const score = calculateScore(contest); - if (!contestsByScore[score]) { - contestsByScore[score] = []; - } - contestsByScore[score].push(contest); - }); - - // Sort contests within each score group by ID for consistency - Object.values(contestsByScore).forEach((group) => { - group.sort((a, b) => { - if (a.id && b.id) { - return a.id.localeCompare(b.id); - } - return 0; - }); - }); - - // Get all scores and sort them based on direction - const scores = Object.keys(contestsByScore) - .map(Number) - .sort((a, b) => (direction === 'asc' ? a - b : b - a)); - - // Flatten the groups in order of score - return scores.flatMap((score) => contestsByScore[score]); + if (direction === null) return [...contests]; + return sortByScore(contestsToSort, direction); } // Handle difficulty sort event diff --git a/tests/table-markup.test.ts b/tests/table-markup.test.ts index 297c883..30c1f92 100644 --- a/tests/table-markup.test.ts +++ b/tests/table-markup.test.ts @@ -25,6 +25,10 @@ import { fileURLToPath } from 'node:url'; import { dirname, join } from 'node:path'; const __dirname = dirname(fileURLToPath(import.meta.url)); +const FEEDBACK_BUTTONS = readFileSync( + join(__dirname, '../src/lib/components/TableFeedbackButtons.svelte'), + 'utf8' +); const COMPONENTS = { ProblemTable: readFileSync(join(__dirname, '../src/lib/components/ProblemTable.svelte'), 'utf8'), ContestTable: readFileSync(join(__dirname, '../src/lib/components/ContestTable.svelte'), 'utf8') @@ -63,29 +67,28 @@ for (const [name, src] of Object.entries(COMPONENTS)) { ); }); - // Bug #2 guard: every like/dislike button carries an aria-label and - // aria-pressed. Both tables render exactly two such buttons (like + dislike). - test(`[${name}] like/dislike buttons expose aria-label + aria-pressed`, () => { - const likeLabels = [...src.matchAll(/aria-label=\{`Like/g)].length; - const dislikeLabels = [...src.matchAll(/aria-label=\{`Dislike/g)].length; - const pressed = [...src.matchAll(/aria-pressed=\{has(Liked|Disliked)\}/g)].length; - assert.equal(likeLabels, 1, 'expected one like button with an aria-label'); - assert.equal(dislikeLabels, 1, 'expected one dislike button with an aria-label'); - assert.equal(pressed, 2, 'expected aria-pressed on both the like and dislike buttons'); + test(`[${name}] renders the shared feedback controls`, () => { + assert.match(src, / { - assert.match(src, /aria-label=\{`Like[^`]*,\s*\$\{[^}]*\.likes\}/); - assert.match(src, /aria-label=\{`Dislike[^`]*,\s*\$\{[^}]*\.dislikes\}/); - }); +// Bug #2 guard: the shared buttons retain the complete accessible state once, +// rather than duplicating a fragile implementation in each table. +test('[TableFeedbackButtons] like/dislike buttons expose aria-label + aria-pressed', () => { + assert.equal([...FEEDBACK_BUTTONS.matchAll(/aria-label=\{`Like/g)].length, 1); + assert.equal([...FEEDBACK_BUTTONS.matchAll(/aria-label=\{`Dislike/g)].length, 1); + assert.equal([...FEEDBACK_BUTTONS.matchAll(/aria-pressed=\{has(Liked|Disliked)\}/g)].length, 2); +}); - // The decorative thumb glyphs must be hidden from assistive tech so the - // aria-label is the sole accessible name. Both buttons use an ; require - // at least the two like/dislike glyphs to be aria-hidden. - test(`[${name}] decorative like/dislike glyphs are aria-hidden`, () => { - const hidden = [...src.matchAll(/class="stroke-2"\s+aria-hidden="true"/g)].length; - assert.ok(hidden >= 2, `expected >= 2 aria-hidden thumb glyphs, found ${hidden}`); - }); -} +test('[TableFeedbackButtons] aria-labels include the feedback counts', () => { + assert.match(FEEDBACK_BUTTONS, /aria-label=\{`Like[^`]*,\s*\$\{likes\}/); + assert.match(FEEDBACK_BUTTONS, /aria-label=\{`Dislike[^`]*,\s*\$\{dislikes\}/); +}); + +test('[TableFeedbackButtons] decorative glyphs are aria-hidden', () => { + const hidden = [...FEEDBACK_BUTTONS.matchAll(/class="stroke-2"\s+aria-hidden="true"/g)].length; + assert.equal(hidden, 2); +}); diff --git a/tests/table-utils.test.ts b/tests/table-utils.test.ts new file mode 100644 index 0000000..990131c --- /dev/null +++ b/tests/table-utils.test.ts @@ -0,0 +1,106 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + cycleTableState, + getDifficultyAriaSort, + getDifficultySortLabel, + getSortedAuthors, + nextSortDirection, + sortByDifficulty, + sortByScore +} from '../src/lib/utils/table.ts'; + +type Item = { + id?: string; + likes: number; + dislikes: number; + difficulty?: number; + addedBy: string; +}; + +const items: Item[] = [ + { id: 'z', likes: 5, dislikes: 2, difficulty: 3, addedBy: 'zoe' }, + { id: 'b', likes: 4, dislikes: 1, difficulty: undefined, addedBy: 'amy' }, + { id: 'a', likes: 3, dislikes: 0, difficulty: 1, addedBy: 'amy' }, + { id: 'low', likes: 1, dislikes: 2, difficulty: 5, addedBy: 'max' } +]; + +test('sortByScore orders by net score and breaks equal-score ties by ID', () => { + assert.deepEqual( + sortByScore(items, 'desc').map((item) => item.id), + ['a', 'b', 'z', 'low'] + ); + assert.deepEqual( + sortByScore(items, 'asc').map((item) => item.id), + ['low', 'a', 'b', 'z'] + ); +}); + +test('sortByScore preserves insertion order for tied items without two IDs', () => { + const tied = [ + { ...items[0], id: undefined }, + { ...items[1], id: 'b' } + ]; + + assert.deepEqual( + sortByScore(tied, 'desc').map((item) => item.id), + [undefined, 'b'] + ); +}); + +test('sorting returns a new array without mutating the input', () => { + const originalOrder = items.map((item) => item.id); + assert.notEqual(sortByScore(items, 'desc'), items); + assert.deepEqual( + items.map((item) => item.id), + originalOrder + ); +}); + +test('sortByDifficulty treats missing difficulty as zero and supports reset sorting', () => { + const resetCalls: (readonly Item[])[] = []; + const reset = (values: readonly Item[]) => { + resetCalls.push(values); + return [...values].reverse(); + }; + + assert.deepEqual( + sortByDifficulty(items, 'asc', reset).map((item) => item.id), + ['b', 'a', 'z', 'low'] + ); + assert.deepEqual( + sortByDifficulty(items, 'desc', reset).map((item) => item.id), + ['low', 'z', 'a', 'b'] + ); + assert.deepEqual( + sortByDifficulty(items, null, reset).map((item) => item.id), + ['low', 'a', 'b', 'z'] + ); + assert.equal(resetCalls.length, 1); +}); + +test('table state cycles through each value and wraps', () => { + const states = ['all', 'included', 'excluded'] as const; + assert.equal(cycleTableState('all', states), 'included'); + assert.equal(cycleTableState('included', states), 'excluded'); + assert.equal(cycleTableState('excluded', states), 'all'); + + assert.equal(nextSortDirection(null), 'asc'); + assert.equal(nextSortDirection('asc'), 'desc'); + assert.equal(nextSortDirection('desc'), null); +}); + +test('difficulty sort accessibility state matches the next interaction', () => { + assert.equal(getDifficultyAriaSort(null), 'none'); + assert.equal(getDifficultyAriaSort('asc'), 'ascending'); + assert.equal(getDifficultyAriaSort('desc'), 'descending'); + assert.match(getDifficultySortLabel(null), /not sorted.*sort ascending/i); + assert.match(getDifficultySortLabel('asc'), /sorted ascending.*sort descending/i); + assert.match(getDifficultySortLabel('desc'), /sorted descending.*clear sorting/i); +}); + +test('getSortedAuthors deduplicates derived authors and prefers provided options', () => { + assert.deepEqual(getSortedAuthors(items), ['amy', 'max', 'zoe']); + assert.deepEqual(getSortedAuthors(items, ['zoe', 'amy']), ['amy', 'zoe']); + assert.deepEqual(getSortedAuthors(items, []), ['amy', 'max', 'zoe']); +});