fix(view tables): make ordering by a column visible and correct - #675
Merged
Conversation
Clicking a column header showed the "updating" spinner and gave no sign of what had been ordered (issue #673). The spinner: a sort is a reorder of rows the page already has, but the click goes through Ajax like any other, and the client-side indicator lights up for every Ajax call started inside a view panel. Worse, the response re-renders the header with fresh markup ids, so the completion handler could no longer find the panel and the spinner ran on until the 30s backstop. Sort calls are now left out of the indicator, and the panel a call belongs to is remembered from its start, so no control that disappears in its own response can strand a spinner again. The sign: Wicket marks the header cell with wicket_orderUp/wicket_orderDown but style.css had no rules for those classes, so nothing on screen changed except the row order. A caret now sits after the header label, solid on the ordered column and on hover for the others. Columns with no header label (the actions column among them) are no longer sortable: an empty header was a live sort link, and the actions column offered to order by a property no row has. Ordering itself compared cells as text, which puts "10" above "9". Values that are numbers are now compared as numbers, and text with numbers in it run by run, so "Session #9" comes before "Session #34" as well. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UToMqDvcVRgpwHojAx69t1
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.
Fixes #673.
Clicking a column header showed the "updating" spinner and gave no sign of what had been ordered.
The spinner
A sort is a reorder of rows the page already has — no query is made — but the click goes through Ajax like any other, and the client-side indicator in
nanodash.jslights up for every Ajax call started inside a view panel. Worse, the response re-renders the header with fresh markup ids, so at/ajax/call/completethe trigger element is gone and the panel could not be found again; once shown, the spinner ran on until the 30s backstop, which is why a fast reorder looked like a long refresh.isTableReordering; header sort links are the only Ajax triggers inside a<th>).pendingCalls), so no control that disappears in its own response can strand a spinner again — this also covers paging links.The sign that something happened
Wicket marks the header cell with
wicket_orderUp/wicket_orderDown/wicket_orderNone, butstyle.csshad no rules for those classes, so nothing on screen changed except the row order. A caret now sits after the header label: solid ▲/▼ on the ordered column, greyed and only on hover for the others, with the space reserved so nothing shifts.Columns with no header label are no longer sortable: an empty header was a live sort link, and the actions column offered to order the table by a property no row has.
Numeric ordering
Cells were compared with
compareToIgnoreCase, i.e. digit by digit, so"10"came out above"9". The newUtils.compareValuescompares values that are numbers throughout as numbers (decimals included — run-by-run comparison would put1.5below1.25), and otherwise falls back to text order with each run of digits compared by value, soSession #9<Session #34andv1.9.0<v1.10.0. Leading zeros are skipped, case is ignored as before, and ISO dates keep sorting correctly.Verification
Driven in a browser against a local instance:
wicket_orderUp/Downwith a visible caret, and the actions headers are no longer links;0, 1, 1, …ascending and73, 12, 12, 12, 10, 9, 8, 8, 7, 7descending (text order gave9, 8, 8, 73, 7, 7, 12, …).Four comparator tests added to
UtilsTest; full suite 1236 tests, 0 failures.🤖 Generated with Claude Code
https://claude.ai/code/session_01UToMqDvcVRgpwHojAx69t1