fix(Grid): sorting of columns identified by a value selector instead of a field#1316
Open
nebojsa-peric wants to merge 2 commits into
Open
fix(Grid): sorting of columns identified by a value selector instead of a field#1316nebojsa-peric wants to merge 2 commits into
nebojsa-peric wants to merge 2 commits into
Conversation
…of a field
- prepareData no longer fabricates a { field: undefined, direction: undefined }
sorter when the sortField/sortDirection bindings are declared but empty; this
crashed renderHeader (direction.toLowerCase) and silently discarded the value
selector captured on header click, making value-only columns unsortable
- a sort is now identified by its (field, value selector) pair instead of the
field name alone, both for the sorted header indicator and for toggling in
onHeaderClick; previously any field-less sortable column matched a field-less
sorter (undefined == undefined) and lit up together, and columns sharing a
field name hijacked each other's sort direction
- clicking a field-less column no longer toggles the sort of the field
currently stored in the sortField binding
- the sorter fix-up loop in prepareData now also matches columns by sortField
and covers line0
Sort key precedence is unchanged: sortValue > value > sortField > field.
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
1 participant
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 the field-independent parts of #1314 (repro in #1313, included as the first commit).
What's fixed
Sorting for columns identified by a
valueselector (tpl/expr/computable) instead of afield:prepareDatano longer fabricates a{ field: undefined, direction: undefined }sorter when the
sortField/sortDirectionbindings are declared but still empty — thispreviously matched any field-less sortable column (
undefined == undefined) and crashedrenderHeaderwithdirection.toLowerCase().sortField/sortDirectionbindings: the fabricatedsorter also discarded the
valueselector captured on header click, so such columns neversorted; clicking them additionally cleared the
sortFieldbinding and toggled the directionof the previously active sort.
the field name alone — previously all field-less sortable columns lit up with the sorted
icon together, and two columns sorting the same field through different selectors hijacked
each other's direction on click.
prepareDatanow also matches columns bysortFieldand coversline0.Also documents the
valueandsortValuecolumn properties in the docs column-configurationtable (precedence-neutral wording).
What's deliberately NOT changed: sort key precedence
The sort key is resolved the same way as before:
fieldonlyfieldvalueonlyvaluefield+valuevaluesortField(+field)sortFieldsortValue+ anythingsortValuesortField+valuevalue(!)i.e. effectively
sortValue > value > sortField > field. Note the last row: an explicitsortFieldis silently ignored when the column also has a displayvalue, which makessortFieldunusable exactly where it seems most needed (computed display, raw-data sort).This also contradicts the current docs (Grid page, Columns section):
which describes yet another rule (
fieldwinning overvalue) that the code has neverimplemented. The docs also don't mention
sortValueat all (partially addressed here by theneutral table entries).
Whether the intended precedence is the current one or e.g.
sortValue > sortField > value > fieldis an open question on the issue — a follow-up PR with the precedence change (a 3-line diff on
top of this one) and the full docs update is prepared and will be opened once that's decided.