Add click-to-sort column headers to all entity list table views - #6934
Add click-to-sort column headers to all entity list table views#6934slick-daddy wants to merge 14 commits into
Conversation
WithoutPants
left a comment
There was a problem hiding this comment.
Please include screenshots when making UI changes. Users shouldn't need to try it locally to find out what has changed.
For reference, here's how it looks on the scenes page, sorting by title ascending:
I think it would look more consistent if we used the same icons as the sort selector and that the icon should be placed at the right edge:
I would extract the Icon element from ListFilter.tsx and create an exported React component in the same file called SortByIcon which accepts a SortDirectionEnum. SortBySelect should replace its Icon with the new component.
I was able to style it like above by putting contents of the column header th element into a div and styling it with display: flex; align-items: center; justify-content: space-between.
The other thing I noticed is that you've used string instead of SortDirectionEnum for the sortBy parameter. Please change these to use the enum.
While you are in there, please add padding-bottom: 0 to the .table-list .table thead th css rule. The bottom padding is unnecessary and would be a good incidental fix.
| const playScene = usePlayScene(); | ||
|
|
||
| const playSelected = useCallback(() => { | ||
| // populate queue and go to first scene |
There was a problem hiding this comment.
Please revert these removals. There's no reason to remove them for this change.
| const reverseSortMap = useMemo(() => { | ||
| const rev: Record<string, string> = {}; | ||
| Object.entries(sceneColumnSortMap).forEach(([k, v]) => { | ||
| rev[v] = k; | ||
| }); | ||
| return rev; | ||
| }, []); |
There was a problem hiding this comment.
This can be moved out of the function component. I would probably add a utility function for creating the reverse map (probably in src/utils/data/ts), and the reverse sort map can be declared below the sort map above.
Do this for the other list components as well.
| )); | ||
| }, [visibleColumns]); | ||
| return visibleColumns.map((column) => { | ||
| const isSortable = column.sortable !== false && onSort; |
There was a problem hiding this comment.
Do sortable columns outnumber non-sortable columns? If not, then perhaps we should consider defaulting to sortable = false.
There was a problem hiding this comment.
Answering my own question, it appears that there's more sortable than not. I don't love defaulting to false with a name like sortable, but I don't have an alternative.
3d495a0 to
c192113
Compare
|
There's a bunch of unrelated changes to the server code. |
…t, CSS padding fix
- Add sortable: false to performer_count column in StudioListTable (backend does not support sorting by performer_count) - Add weight_kg: weight mapping to performerColumnSortMap (backend expects 'weight', not 'weight_kg')
- Revert unnecessary comment removals in SceneList.tsx - Create getActiveSortColumn utility in src/utils/data.ts - Use utility in all 5 entity List.tsx files, removing duplicated useMemo+reverseSortMap pattern
- Add 'favorite' to performerSortOptions, studioSortOptions, tagSortOptions - Remove sortable:false from favourite columns in PerformerListTable and TagListTable - Add favourite column to StudioListTable with FavoriteCell - Add favourite/favorite mapping to all three entity ColumnSortMaps
SQLite sorts NULLs first for both ASC and DESC. Add IS NULL ordering clause to push NULL values to the end in all sort directions. - getSort: add IS NULL to default, name, title, and filesize cases - performer: sortByOCounter, sortByLastPlayedAt, sortByLastOAt, sortByLatestScene - group: sortByOCounter - studio: sortByLatestScene - scene: last_played_at, last_o_at
- Revert IS NULL addition in getSort (caused unintended side effects) - Add targeted birthdate case in getPerformerSort that flips direction (birthdate is inverse to age: lower date = older, so ASC exchanges with DESC) and adds IS NULL for nulls-last behavior
9c8df38 to
bffc293
Compare
|
|
There's still changes to server files in this PR - specifically the |
These IS NULL ORDER BY changes were server-side SQL behavior improvements that don't belong in the click-to-sort column headers PR. The sort options arrays (favorite, birthdate handling) are kept since they directly enable the clickable column feature.
89287e8 to
bffc293
Compare
| sortQuery += qb.sortByLastOAt(direction) | ||
| case "latest_scene": | ||
| sortQuery += qb.sortByLatestScene(direction) | ||
| case "birthdate": |
There was a problem hiding this comment.
Didn't drop favorite sort option and birthdate case because they are needed for Favourite/Age column clicks.
There was a problem hiding this comment.
You didn't include these changes in the original description.
WithoutPants
left a comment
There was a problem hiding this comment.
Please ensure that the description in your PRs matches the changes you've made. You made undocumented changes in this PR, specifically changes to the server sorting behaviour, changing the related studios column presentation, and the favourite column to the studio list table. Having to pore through the changes to discover undocumented changes is significantly time-consuming.
| )); | ||
| }, [visibleColumns]); | ||
| return visibleColumns.map((column) => { | ||
| const isSortable = column.sortable !== false && onSort; |
There was a problem hiding this comment.
Answering my own question, it appears that there's more sortable than not. I don't love defaulting to false with a name like sortable, but I don't have an alternative.
| sortQuery += qb.sortByLastOAt(direction) | ||
| case "latest_scene": | ||
| sortQuery += qb.sortByLatestScene(direction) | ||
| case "birthdate": |
There was a problem hiding this comment.
You didn't include these changes in the original description.
…studio.go and StudioListTable.tsx
| withoutContext | ||
| /> | ||
| {showAllCounts && !!studio.o_counter_all ? ( | ||
| {showAllCounts && studio.o_counter_all ? ( |
There was a problem hiding this comment.
This was an incidental fix from pnpm run lint:js:fix. It flagged unnecessary double-negation (!!) on o_counter fields which are already numbers, not booleans.
This PR shouldn't be merged before #6933 gets merged.We are good now.The only thing I didn't like about this PR is reverse mapping approach. Maybe we should edit onClick instead. I am open to feedbacks.
Summary
Makes column headers clickable in list table views for Scenes, Galleries,
Performers, Studios, and Tags. The sort infrastructure is generic so all
entity tables benefit without per-entity duplication.
Changes
Changes After Review
Server: favorite sort option for Performer, Studio, Tag sort options
Server: birthdate sort case for age column support
StudioListTable: Favourite column with FavoriteCellStudioListTable: RelatedCell uses sub_studios pluralized labelGeneric sort support (
ListTable.tsx)sortableflag toIColumn— columns without a backendsort field (image, composite, etc.) are marked
sortable: false.onSort,sortBy,sortDirectionprops. Active sort columnrenders ▲ (ASC) or ▼ (DEC) indicator.
onSort(column.value)— new column starts ASC,same column toggles direction.
Per-entity wiring (
*List.tsx+*ListTable.tsx)Each entity follows the same pattern:
*ListTable.tsx— acceptsonSort/sortBy/sortDirectionpropsand threads them through to
ListTable. Non-sortable columns markedsortable: false.*List.tsx—Filtered*Listdefines a module-levelcolumnSortMaptranslating column values to backend sort fields.onSortuseCallback maps the column value via this map before callingfilter.setSortBy(). The inner*Listcomponent derivesactiveSortColumnby reverse-mappingfilter.sortByback to thecolumn value so
ListTablematches correctly for arrow display.Column mappings (value → backend field)
scene_count→scenes_count,image_count→images_count,gallery_count→galleries_countscene_code→codeimages→images_countheight_cm→height,penis_length_cm→penis_length,scene_count→scenes_count,gallery_count→galleries_count,image_count→images_count,age→birthdatescene_count→scenes_count,gallery_count→galleries_count,image_count→images_count,group_count→groups_count,performer_count→performers_count,studio_count→studios_countBug fixes
filter.sortBycontained the backendfield name (e.g.
"scenes_count") butListTablecompared againstcolumn.value(e.g."scene_count"). Fixed by reverse-mapping tocolumn value before passing to the table.
filter.sortBycould beundefined, whichcan't be used as an object index. Fixed with
?? ""fallback.Studio child count label
StudioListTableRelatedCellnow shows "N Studios" (pluralized viaintl.formatMessage) instead of a bare number.Files changed (11 files)
ui/.../List/ListTable.tsxui/.../Studios/StudioList.tsxui/.../Studios/StudioListTable.tsxui/.../Scenes/SceneList.tsxui/.../Scenes/SceneListTable.tsxui/.../Galleries/GalleryList.tsxui/.../Galleries/GalleryListTable.tsxui/.../Performers/PerformerList.tsxui/.../Performers/PerformerListTable.tsxui/.../Tags/TagList.tsxui/.../Tags/TagListTable.tsx