add sort by name to assigned page #459
Conversation
Co-authored-by: Vincent Shuali <[email protected]>
nickvisut
force-pushed
the
feature/add-sort-to-assigned
branch
from
32b82ce to
55e8c39
Compare
|
rebased on main to get around linting issue |
src/pages/benchmarks/index.tsx
Outdated
| const [sortProperty, setSortProperty] = useState<SortProperty>("first_name"); | ||
| const [sortDirection, setSortDirection] = useState<SortDirection>("asc"); | ||
|
|
||
| const { data: tasksData, isLoading, error } = trpc.para.getMyTasks.useQuery(); |
There was a problem hiding this comment.
Is "error" within data going to be accessed, or is this for troubleshooting of some sort?
src/pages/benchmarks/index.tsx
Outdated
| if (!tasksData) return []; | ||
| return [...tasksData].sort((a, b) => { | ||
| if (a[sortProperty] < b[sortProperty]) | ||
| return sortDirection === "asc" ? -1 : 1; |
There was a problem hiding this comment.
I had a bit of trouble following why you want to return 1 or -1. Could you please explain? Thanks!
There was a problem hiding this comment.
JavaScript decides whether two items in a sort are in the correct order relative to another depending on whether the sorting callback you give it returns +1 or -1.
Yes, until we add admin functionality to impersonate users, we have to go into the database and flip our role to "para" |
src/pages/benchmarks/index.tsx
Outdated
| const getSortedTasks = () => { | ||
| if (!tasksData) return []; | ||
|
|
||
| return [...tasksData].sort((a, b) => { |
There was a problem hiding this comment.
sorting a copy of the array bypasses reactivity for tasksData, is this intentional?
src/pages/benchmarks/index.tsx
Outdated
| } | ||
| }; | ||
|
|
||
| const displayedTasks = getSortedTasks(); |
There was a problem hiding this comment.
maybe just sort and display tasksData? no need for displayedTasks?
This PR adds sorting functionality on the front end for the Assigned page. It does so by adding a new button (simple stand-in for current placeholder) that toggles between name ascending and descending sort. It partly addresses #424.
Note that each interaction currently forces a rerender, so we will want to revisit in future to improve perf (through either
useEffect()and/or server-side sorting). This is good enough for POC, however.Also note that we will likely need to replace the button with a sorting dropdown to incorporate other sort properties.