Reusable table component and example #149
Conversation
robertardeleanu
requested review from
radubuciuceanu,
alexandru-calinoiu,
Utwo and
catileptic
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| } | ||
|
|
||
| function stableSort(array, comparator) { | ||
| const stabilizedThis = array.map((el, index) => [el, index]); |
There was a problem hiding this comment.
It is possible to chain these methods, such as (also added an alternative to the double return)
array
.map((el, index) => [el, index])
.sort((a, b) => {
const elementOrder = comparator(a[0], b[0]);
const decisiveOrder = elementOrder === 0 ? a[1] - b[1] : elementOrder;
return decisiveOrder;
})
.map((el) => el[0]);| } | ||
|
|
||
| function getComparator(order, orderBy) { | ||
| return order === "desc" |
There was a problem hiding this comment.
I see there is a code repetition, it could be shortened to:
| return order === "desc" | |
| const scalar = order === "desc" ? 1 : -1; | |
| return scalar * descendingComparator(a.original, b.original, orderBy); |
| }; | ||
|
|
||
| function descendingComparator(a, b, orderBy) { | ||
| if (b[orderBy] < a[orderBy]) { |
There was a problem hiding this comment.
This logic could be simplified to
| if (b[orderBy] < a[orderBy]) { | |
| return b[orderBy] - a[orderBy]; |
But further, the below function could be simplified to
const scalar = order === "desc" ? 1 : -1;
return scalar * (b.original[orderBy] - a.original[orderBy]);And this logic as they are pure functions, they could be entirely moved to a helper file with some test suite
| </TableCell> | ||
| ); | ||
|
|
||
| const singleSelectCell = ( |
There was a problem hiding this comment.
I am not sure if by assigning this component straight to the const React will execute (not necessary render) code that is inside of it, it is worth to check.
Also the code <TableCell padding="checkbox"> is repeated, it could be returned regardless and then composed with Checkbox / Radio
|
|
||
| const emptyRows = page > 0 ? Math.max(0, (1 + page) * rowsPerPage - processedData.length) : 0; | ||
|
|
||
| const renderTableHead = () => { |
There was a problem hiding this comment.
Many of these functions could be placed in a separate file, as it clogs this whole file
| }); | ||
| }, [data]); | ||
|
|
||
| const processedColumns = useMemo(() => columns.filter(({ show = true }) => show)); |
There was a problem hiding this comment.
This hook will always run because it has no dependencies, making memoizing not useful
|
|
||
| const handleRequestSort = (event, property) => { | ||
| const isAsc = orderBy === property && order === 'asc'; | ||
| setOrder(isAsc ? 'desc' : 'asc'); |
There was a problem hiding this comment.
Could there be an "enum" for asc/desc to be used across this component? Thinking of benefit of auto complete and prevent typos
const orderType = {
asc: 'asc',
desc: 'desc'
};|
|
||
|
|
||
| return Template ? ( | ||
| <TableCell> |
There was a problem hiding this comment.
TableCell could wrap around the actual alternatives
| <TableCell> | |
| <TableCell> | |
| {Template ? <Template row={row} /> : row.original[column.accessor]} | |
| </TableCell> |
|
|
||
| const isSelected = (id) => selected.findIndex((entry) => entry._id === id) !== -1; | ||
|
|
||
| const emptyRows = page > 0 ? Math.max(0, (1 + page) * rowsPerPage - processedData.length) : 0; |
There was a problem hiding this comment.
The Math.max is already going establish a lower limit to 0, no?
| const emptyRows = page > 0 ? Math.max(0, (1 + page) * rowsPerPage - processedData.length) : 0; | |
| const emptyRows = Math.max(0, (1 + page) * rowsPerPage - processedData.length); |
| @@ -0,0 +1,268 @@ | |||
| import React, { useState, useMemo } from 'react'; | |||
| import PropTypes from 'prop-types' | |||
| import _ from 'lodash'; | |||
There was a problem hiding this comment.
If possible we could import functions individually, so that it can be tree-shaked
| import _ from 'lodash'; | |
| import { noop } from 'lodash'; |
What does it fix?
Implemented a generic and configurable table component and example for it
Closes #135
How has it been tested?
Manually tested with the example itself