fix(audit): keep blank rows last when sorting results tables descending - #166
Open
evanclan wants to merge 1 commit into
Open
fix(audit): keep blank rows last when sorting results tables descending#166evanclan wants to merge 1 commit into
evanclan wants to merge 1 commit into
Conversation
The Pages and Performance tables sorted with their own nullableNumberSort / nullableStringSort, which return ±1 when one side is blank. TanStack negates a comparator's result on a descending column, so those blanks jumped to the top of the table on the second header click. On the Lighthouse score columns the blank rows are exactly the failed audits, so sorting Perf/A11y/SEO descending filled the first screen with rows that have no scores at all. src/client/components/table/nullSafeSort.ts already solves this for the Brand Lookup tables by reading the sort direction off the cell and pre-compensating for the flip. Reuse numericNullsLast here, add a stringNullsLast sibling for the URL/Title columns, and drop the two direction-blind copies.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sorting any nullable column in the Site Audit Pages or Performance table descending moves the blank rows to the top instead of leaving them at the bottom. The two comparators those tables use return
±1for a blank side, and TanStack negates a comparator's result on a descending column, so the intent is inverted on the second header click.This reuses the helper the repo already has for exactly this problem —
src/client/components/table/nullSafeSort.ts— instead of keeping a second, direction-blind copy. 11 sortable columns are affected.Problem
getSortedRowModelapplies the descending flip after calling the column'ssortingFn(table-core@8.21.3):So a comparator that hard-codes "blank goes after" gets "blank goes before" on desc:
Verified against a real table instance — with rows
score: [5, null, 10]and the column sorted desc, the old comparator returns[null, 10, 5].The user-visible sting is on the Performance tab.
isLighthouseFailureis defined as "has no category scores" (AuditResultsTableFilterLogic.ts:60-71), so the null rows are the failed audits. Clicking Perf, A11y or SEO to see your worst pages first fills the top of the table with rows that have no scores at all, pushing the real data below the fold. Same for LCP / CLS / INP / TTFB, for Status and Title on the Pages tab (pages that aren't HTML documents have no title), and for the Performance URL column.Note that
sortUndefined: "last"is not an option here: it returns early before the flip, so it is direction-safe, but it only inspectsundefinedand these columns carrynullstraight from the DB.Solution
nullSafeSort.tsalready solves this for the Brand Lookup tables (added infaf7a29) by reading the column's sort direction off the cell and pre-compensating for the flip. This PR:stringNullsLast, a sibling of the existingnumericNullsLast, for the URL/Title columns — empty strings count as blank, preserving the old!abehaviour;blankLasthelper used by both;nullableNumberSort/nullableStringSort, so there is one implementation of this rule rather than two.There is a second working pattern in the repo —
RankTrackingColumns.tsxmapsaccessorFn: (row) => row.cpc ?? undefinedand letssortUndefined: "last"do the work. That would also fix these tables, but it needs anaccessorFnon all 11 columns, so reusing the existing comparator seemed like the smaller change. Happy to switch if you'd rather converge on the rank-tracking approach.Out of scope
KeywordSuggestionStep.tsx:70-124has the same class of issue via sentinels (position ?? 999,searchVolume ?? 0), so unranked suggestions lead on a descending position sort. Different feature and it needs a call on whether0should mean "unknown" for volume/traffic, so I left it alone.Test plan
src/client/components/table/nullSafeSort.test.tsdrives a real headlesscreateTable+getSortedRowModel, so it exercises TanStack's own desc flip rather than asserting my reading of it. Covers nulls-last in both directions,0not treated as blank, blanks keeping their relative order, and""counting as blank.pnpm test— 92 files, 740 tests passing.pnpm ci:check— prettier, knip,tsc --noEmit(both projects) andoxlint --type-awareall clean.Related
src/client/components/table/nullSafeSort.ts— the existing fix for this bug class, used by the Brand Lookup citation tables.src/client/features/rank-tracking/RankTrackingColumns.tsx— thenull->undefined+sortUndefined: "last"variant of the same fix.Made with Cursor