Skip to content

Commit eba6cfc

Browse files
authored
fix(ui): simplify UI orderable collections table by allowing only ascending order (#14433)
This PR removes the ability to toggle between ascending and descending order in orderable collections. We discussed this internally after some users found the ascending/descending toggle confusing, especially since orderable collections are not meant to represent alphabetical or numerical sorting but rather a manual order. If any issues arise from this change, we may later consider reintroducing the toggle with a clearer icon. Note: this change only affects the Admin UI. The API remains unchanged. Developers can still query the database using ascending or descending order on the `_order` field if needed.
1 parent 219fba2 commit eba6cfc

File tree

2 files changed

+23
-55
lines changed

2 files changed

+23
-55
lines changed

packages/ui/src/elements/SortHeader/index.tsx

Lines changed: 19 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
'use client'
22

3-
import React, { useEffect, useRef } from 'react'
3+
import React from 'react'
44

5-
import { SortDownIcon, SortUpIcon } from '../../icons/Sort/index.js'
5+
import { SortDownIcon } from '../../icons/Sort/index.js'
66
import { useListQuery } from '../../providers/ListQuery/index.js'
77
import './index.scss'
88
import { useTranslation } from '../../providers/Translation/index.js'
@@ -17,36 +17,23 @@ const baseClass = 'sort-header'
1717
function useSort() {
1818
const { handleSortChange, orderableFieldName, query } = useListQuery()
1919
const querySort = Array.isArray(query.sort) ? query.sort[0] : query.sort
20-
const sort = useRef<'asc' | 'desc'>(querySort === `-${orderableFieldName}` ? 'desc' : 'asc')
21-
const isActive = querySort === `-${orderableFieldName}` || querySort === orderableFieldName
22-
23-
// This is necessary if you initialize the page without sort url param
24-
// but your preferences are to sort by -_order.
25-
// Since sort isn't updated, the arrow would incorrectly point upward.
26-
useEffect(() => {
27-
if (!isActive) {
28-
return
29-
}
30-
sort.current = querySort === `-${orderableFieldName}` ? 'desc' : 'asc'
31-
}, [orderableFieldName, querySort, isActive])
20+
const isActive = querySort === orderableFieldName
3221

3322
const handleSortPress = () => {
34-
// If it's already sorted by the "_order" field, toggle between "asc" and "desc"
23+
// If it's already sorted by the "_order" field, do nothing
3524
if (isActive) {
36-
void handleSortChange(sort.current === 'asc' ? `-${orderableFieldName}` : orderableFieldName)
37-
sort.current = sort.current === 'asc' ? 'desc' : 'asc'
3825
return
3926
}
40-
// If NOT sorted by the "_order" field, sort by that field but do not toggle the current value of "asc" or "desc".
41-
void handleSortChange(sort.current === 'asc' ? orderableFieldName : `-${orderableFieldName}`)
27+
// If NOT sorted by the "_order" field, sort by that field.
28+
void handleSortChange(orderableFieldName)
4229
}
4330

44-
return { handleSortPress, isActive, sort }
31+
return { handleSortPress, isActive }
4532
}
4633

4734
export const SortHeader: React.FC<SortHeaderProps> = (props) => {
4835
const { appearance } = props
49-
const { handleSortPress, isActive, sort } = useSort()
36+
const { handleSortPress, isActive } = useSort()
5037
const { t } = useTranslation()
5138

5239
return (
@@ -56,31 +43,17 @@ export const SortHeader: React.FC<SortHeaderProps> = (props) => {
5643
.join(' ')}
5744
>
5845
<div className={`${baseClass}__buttons`}>
59-
{sort.current === 'desc' ? (
60-
<button
61-
aria-label={t('general:sortByLabelDirection', {
62-
direction: t('general:ascending'),
63-
label: 'Order',
64-
})}
65-
className={`${baseClass}__button ${baseClass}__${sort.current} ${isActive ? `${baseClass}--active` : ''}`}
66-
onClick={handleSortPress}
67-
type="button"
68-
>
69-
<SortDownIcon />
70-
</button>
71-
) : (
72-
<button
73-
aria-label={t('general:sortByLabelDirection', {
74-
direction: t('general:descending'),
75-
label: 'Order',
76-
})}
77-
className={`${baseClass}__button ${baseClass}__${sort.current} ${isActive ? `${baseClass}--active` : ''}`}
78-
onClick={handleSortPress}
79-
type="button"
80-
>
81-
<SortUpIcon />
82-
</button>
83-
)}
46+
<button
47+
aria-label={t('general:sortByLabelDirection', {
48+
direction: t('general:ascending'),
49+
label: 'Order',
50+
})}
51+
className={`${baseClass}__button ${isActive ? `${baseClass}--active` : ''}`}
52+
onClick={handleSortPress}
53+
type="button"
54+
>
55+
<SortDownIcon />
56+
</button>
8457
</div>
8558
</div>
8659
)

test/sort/e2e.spec.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,11 @@ describe('Sort functionality', () => {
7070
await moveRow(1, 4) // move to bottom
7171
await assertRows(0, 'A', 'C', 'D', 'B')
7272

73-
// SORT BY ORDER DESCENDING
73+
// Click the sort button again should not change the order
74+
// Note: In previous versions we allowed ascending and descending order.
7475
await page.locator('.sort-header button').nth(0).click()
75-
await page.waitForURL(/sort=-_order/, { timeout: 2000 })
76-
await assertRows(0, 'B', 'D', 'C', 'A')
77-
await moveRow(1, 3) // move to middle
78-
await assertRows(0, 'D', 'C', 'B', 'A')
79-
await moveRow(3, 1) // move to top
80-
await assertRows(0, 'B', 'D', 'C', 'A')
81-
await moveRow(1, 4) // move to bottom
82-
await assertRows(0, 'D', 'C', 'A', 'B')
76+
await page.waitForURL(/sort=_order/, { timeout: 2000 })
77+
await assertRows(0, 'A', 'C', 'D', 'B')
8378

8479
// SORT BY TITLE
8580
await page.getByLabel('Sort by Title Ascending').click()

0 commit comments

Comments
 (0)