Feature/table sort #162
Feature/table sort #162
Conversation
✅ Deploy Preview for cell-catalog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
interim17
requested review from
meganrm and
rugeli
and removed request for
meganrm
| const { lastColumn } = require("../../style/table.module.css"); | ||
|
|
||
| /* case insensitive string comparison */ | ||
| const compareStrings = (a = "", b = "") => |
There was a problem hiding this comment.
nit, but could call this caseInsenstiveStringCompare and then you don't need the comment
|
when I click the arrow a third time it does something weird |
|
should the down arrow for cell id be highlighted on load since that's what's being sorted on by default? (the prototype travis made isn't loading for me right now so I can't see the design) |
| const [sortedColumn, setSortedColumn] = useState<{ | ||
| key: string; | ||
| order: SortOrder; | ||
| }>({ key: "", order: null }); |
There was a problem hiding this comment.
isn't it sorted on id by default?
| return column; | ||
| } | ||
| const isSortedColumn = sortedColumn.key === column.key; | ||
| const sortIcon = |
There was a problem hiding this comment.
non-blocking: from my personal user experience, adding tooltips like Sort A-Z or Ascending/Descending to the sort icon would be very helpful.
|
Also noticing the clicking issues Megan mentioned:
(Just saw the latest commit for fixing the undefined third click, looks like it still persists though? Happy to help test more if needed!) |
I only added the value for the third click on the ID column initially, I just added it to the other array so that all columns have all three values, looks like that might fix it? |
rugeli
left a comment
rugeli
left a comment
There was a problem hiding this comment.
The fix looks good! Bugs I've seen are all cleared up!
| } | ||
| return { | ||
| ...interactiveColumn, | ||
| sorter: undefined, |
There was a problem hiding this comment.
why do you have to do this? shouldn't it already be undefined if it's not a sortable column?
| onCell: onCellInteraction, | ||
| }; | ||
| if (sortableTable && column.sorter) { | ||
| return applySortingProperties(interactiveColumn); |
There was a problem hiding this comment.
can't you just check if it has a sorter?
| expandable={isTablet ? mobileConfig : undefined} | ||
| columns={interactiveColumns} | ||
| dataSource={cellLines} | ||
| onChange={sortableTable ? onSortingChange : undefined} |
There was a problem hiding this comment.
I don't think you need this check
Problem
Closes #141 and #165
Solution
Add sorter functions to each column object in
getNormalTableColumnsImplement state, change handler, and necessary details for styling in
CellLineTable