Skip to content

Commit 00287bd

Browse files
committed
fix: clear multi-sort when clicking column without shift key
- Fixed sorting logic in RowSorting.ts to properly clear existing multi-sort when clicking without shift key - Added comprehensive tests to verify the fix works correctly - Ensures single column click replaces all existing sorts - Maintains existing behavior for shift+click multi-sort Fixes #6070
1 parent 9c62cf2 commit 00287bd

File tree

2 files changed

+212
-4
lines changed

2 files changed

+212
-4
lines changed

packages/table-core/src/features/RowSorting.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -388,12 +388,13 @@ export const RowSorting: TableFeature = {
388388
sortAction = 'add'
389389
}
390390
} else {
391-
// Normal mode
392-
if (old?.length && existingIndex !== old.length - 1) {
393-
sortAction = 'replace'
394-
} else if (existingSorting) {
391+
// Normal mode - always replace when not in multi-sort mode
392+
// This ensures that clicking without shift key clears existing multi-sort
393+
if (existingSorting && old?.length === 1) {
394+
// Only one column sorted, so we can toggle
395395
sortAction = 'toggle'
396396
} else {
397+
// Multiple columns sorted or no existing sort, replace all
397398
sortAction = 'replace'
398399
}
399400
}
Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { createTable, getCoreRowModel, getSortedRowModel } from '../src'
3+
import type { Person } from './makeTestData'
4+
import { makeData } from './makeTestData'
5+
6+
const defaultData = makeData(3)
7+
8+
const defaultColumns = [
9+
{
10+
accessorKey: 'firstName' as keyof Person,
11+
id: 'firstName',
12+
},
13+
{
14+
accessorKey: 'lastName' as keyof Person,
15+
id: 'lastName',
16+
},
17+
{
18+
accessorKey: 'age' as keyof Person,
19+
id: 'age',
20+
},
21+
{
22+
accessorKey: 'visits' as keyof Person,
23+
id: 'visits',
24+
},
25+
]
26+
27+
describe('RowSorting', () => {
28+
it('should clear multi-sort when clicking column without shift key', () => {
29+
let sorting = [
30+
{ id: 'firstName', desc: false },
31+
{ id: 'lastName', desc: true },
32+
]
33+
34+
const table = createTable<Person>({
35+
data: defaultData,
36+
columns: defaultColumns,
37+
getCoreRowModel: getCoreRowModel(),
38+
getSortedRowModel: getSortedRowModel(),
39+
onStateChange() {},
40+
renderFallbackValue: '',
41+
state: {
42+
sorting,
43+
},
44+
onSortingChange: updater => {
45+
sorting = typeof updater === 'function' ? updater(sorting) : updater
46+
},
47+
})
48+
49+
expect(sorting).toHaveLength(2)
50+
expect(sorting[0]).toEqual({ id: 'firstName', desc: false })
51+
expect(sorting[1]).toEqual({ id: 'lastName', desc: true })
52+
53+
const ageColumn = table.getColumn('age')
54+
ageColumn?.toggleSorting(false, false)
55+
56+
expect(sorting).toHaveLength(1)
57+
expect(sorting[0]).toEqual({ id: 'age', desc: false })
58+
})
59+
60+
it('should maintain multi-sort when clicking column with shift key', () => {
61+
let sorting = [
62+
{ id: 'firstName', desc: false },
63+
{ id: 'lastName', desc: true },
64+
]
65+
66+
const table = createTable<Person>({
67+
data: defaultData,
68+
columns: defaultColumns,
69+
getCoreRowModel: getCoreRowModel(),
70+
getSortedRowModel: getSortedRowModel(),
71+
onStateChange() {},
72+
renderFallbackValue: '',
73+
state: {
74+
sorting,
75+
},
76+
onSortingChange: updater => {
77+
sorting = typeof updater === 'function' ? updater(sorting) : updater
78+
},
79+
})
80+
81+
const ageColumn = table.getColumn('age')
82+
ageColumn?.toggleSorting(false, true)
83+
84+
expect(sorting).toHaveLength(3)
85+
expect(sorting[0]).toEqual({ id: 'firstName', desc: false })
86+
expect(sorting[1]).toEqual({ id: 'lastName', desc: true })
87+
expect(sorting[2]).toEqual({ id: 'age', desc: false })
88+
})
89+
90+
it('should toggle sort direction when clicking same column without shift key in single sort mode', () => {
91+
let sorting = [{ id: 'firstName', desc: false }]
92+
93+
const table = createTable<Person>({
94+
data: defaultData,
95+
columns: defaultColumns,
96+
getCoreRowModel: getCoreRowModel(),
97+
getSortedRowModel: getSortedRowModel(),
98+
onStateChange() {},
99+
renderFallbackValue: '',
100+
state: {
101+
sorting,
102+
},
103+
onSortingChange: updater => {
104+
sorting = typeof updater === 'function' ? updater(sorting) : updater
105+
},
106+
})
107+
108+
const firstNameColumn = table.getColumn('firstName')
109+
firstNameColumn?.toggleSorting(undefined, false)
110+
111+
expect(sorting).toHaveLength(1)
112+
expect(sorting[0]).toEqual({ id: 'firstName', desc: true })
113+
})
114+
115+
it('should replace multi-sort when clicking different column without shift key', () => {
116+
let sorting = [
117+
{ id: 'firstName', desc: false },
118+
{ id: 'lastName', desc: true },
119+
{ id: 'age', desc: false },
120+
]
121+
122+
const table = createTable<Person>({
123+
data: defaultData,
124+
columns: defaultColumns,
125+
getCoreRowModel: getCoreRowModel(),
126+
getSortedRowModel: getSortedRowModel(),
127+
onStateChange() {},
128+
renderFallbackValue: '',
129+
state: {
130+
sorting,
131+
},
132+
onSortingChange: updater => {
133+
sorting = typeof updater === 'function' ? updater(sorting) : updater
134+
},
135+
})
136+
137+
const visitsColumn = table.getColumn('visits')
138+
visitsColumn?.toggleSorting(false, false)
139+
140+
expect(sorting).toHaveLength(1)
141+
expect(sorting[0]).toEqual({ id: 'visits', desc: false })
142+
})
143+
144+
it('should work with getToggleSortingHandler', () => {
145+
let sorting = [
146+
{ id: 'firstName', desc: false },
147+
{ id: 'lastName', desc: true },
148+
]
149+
150+
const table = createTable<Person>({
151+
data: defaultData,
152+
columns: defaultColumns,
153+
getCoreRowModel: getCoreRowModel(),
154+
getSortedRowModel: getSortedRowModel(),
155+
onStateChange() {},
156+
renderFallbackValue: '',
157+
state: {
158+
sorting,
159+
},
160+
onSortingChange: updater => {
161+
sorting = typeof updater === 'function' ? updater(sorting) : updater
162+
},
163+
})
164+
165+
const ageColumn = table.getColumn('age')
166+
const handler = ageColumn?.getToggleSortingHandler()
167+
168+
const mockEvent = { shiftKey: false }
169+
handler?.(mockEvent)
170+
171+
expect(sorting).toHaveLength(1)
172+
expect(sorting[0]).toEqual({ id: 'age', desc: true })
173+
})
174+
175+
it('should work with getToggleSortingHandler with shift key', () => {
176+
let sorting = [
177+
{ id: 'firstName', desc: false },
178+
{ id: 'lastName', desc: true },
179+
]
180+
181+
const table = createTable<Person>({
182+
data: defaultData,
183+
columns: defaultColumns,
184+
getCoreRowModel: getCoreRowModel(),
185+
getSortedRowModel: getSortedRowModel(),
186+
onStateChange() {},
187+
renderFallbackValue: '',
188+
state: {
189+
sorting,
190+
},
191+
onSortingChange: updater => {
192+
sorting = typeof updater === 'function' ? updater(sorting) : updater
193+
},
194+
})
195+
196+
const ageColumn = table.getColumn('age')
197+
const handler = ageColumn?.getToggleSortingHandler()
198+
199+
const mockEvent = { shiftKey: true }
200+
handler?.(mockEvent)
201+
202+
expect(sorting).toHaveLength(3)
203+
expect(sorting[0]).toEqual({ id: 'firstName', desc: false })
204+
expect(sorting[1]).toEqual({ id: 'lastName', desc: true })
205+
expect(sorting[2]).toEqual({ id: 'age', desc: true })
206+
})
207+
})

0 commit comments

Comments
 (0)