Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LF-4084: Update table/2 to include checkboxes and row selection behaviour #3149

Merged
merged 11 commits into from
Mar 7, 2024
Merged

LF-4084: Update table/2 to include checkboxes and row selection behaviour #3149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
852aa9d
LF-4084 add new color
SayakaOno Mar 1, 2024
8e02be7
LF-4084 add checkbox to table, add story
SayakaOno Mar 1, 2024
9cf1bdd
LF-4084 update V2 table to support sticky header
SayakaOno Mar 1, 2024
6d9649d
LF-4084 remove table padding causing an issue with sticky header
SayakaOno Mar 1, 2024
cd66ab0
LF-4084 remove checkbox hover effect, cleanup stories
SayakaOno Mar 1, 2024
aae6e39
LF-4084 update TableV2Props type
SayakaOno Mar 1, 2024
bc0038c
LF-4084 add missing TableHead propTypes
SayakaOno Mar 1, 2024
28c4759
Merge branch 'integration' into LF-4084/Update_TableV2_to_include_che…
SayakaOno Mar 5, 2024
ba375a5
LF-4084 update story
SayakaOno Mar 5, 2024
b7e0104
LF-4084 fix checkbox behaviour in table story
SayakaOno Mar 6, 2024
1260716
Merge branch 'integration' into LF-4084/Update_TableV2_to_include_che…
SayakaOno Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion packages/webapp/src/components/Table/Header/TableHead.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* GNU General Public License for more details, see <https://www.gnu.org/licenses/>.
*/
import PropTypes from 'prop-types';
import { Checkbox } from '@mui/material';
import TableCell from '@mui/material/TableCell';
import TableHead from '@mui/material/TableHead';
import TableRow from '@mui/material/TableRow';
Expand All @@ -22,14 +23,34 @@ import { ReactComponent as UnfoldCircle } from '../../../assets/images/unfold-ci
import { ReactComponent as ArrowDownCircle } from '../../../assets/images/arrow-down-circle.svg';
import styles from '../styles.module.scss';

export default function EnhancedTableHead({ columns, order, orderBy, onRequestSort, dense }) {
export default function EnhancedTableHead({
columns,
order,
orderBy,
onRequestSort,
dense,
shouldShowCheckbox,
onSelectAllClick,
numSelected,
rowCount,
}) {
const createSortHandler = (property) => (event) => {
onRequestSort(event, property);
};

return (
<TableHead className={styles.headerRow}>
<TableRow>
{shouldShowCheckbox && (
<TableCell padding="checkbox" className={styles.checkboxCell}>
<Checkbox
color="primary"
indeterminate={numSelected > 0 && numSelected < rowCount}
checked={rowCount > 0 && numSelected === rowCount}
onChange={onSelectAllClick}
/>
</TableCell>
)}
{columns.map(({ id, align, columnProps, label, sortable = true }) => {
if (!id) {
return null;
Expand Down Expand Up @@ -80,4 +101,8 @@ EnhancedTableHead.propTypes = {
order: PropTypes.oneOf(['asc', 'desc']).isRequired,
orderBy: PropTypes.string.isRequired,
dense: PropTypes.bool,
shouldShowCheckbox: PropTypes.bool,
onSelectAllClick: PropTypes.func,
numSelected: PropTypes.number,
rowCount: PropTypes.number,
};
70 changes: 56 additions & 14 deletions packages/webapp/src/components/Table/TableV2.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details, see <https://www.gnu.org/licenses/>.
*/
import { useState, useMemo, useEffect } from 'react';
import { useState, useMemo } from 'react';
import PropTypes from 'prop-types';
import { useTranslation } from 'react-i18next';
import { Checkbox } from '@mui/material';
import Box from '@mui/material/Box';
import Table from '@mui/material/Table';
import TableBody from '@mui/material/TableBody';
Expand Down Expand Up @@ -93,6 +94,11 @@ export default function TableV2(props) {
defaultOrderBy,
alternatingRowColor,
showHeader,
onCheck,
handleSelectAllClick,
Comment on lines +97 to +98
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if these two could be combined and simplified by passing a setSelectedIds function prop instead, and letting the table handle the different scenarios where the selected IDs need to change

Copy link
Collaborator Author

@SayakaOno SayakaOno Mar 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible, but I'm hesitant to do it.
A few of my concerns:

  • flexibility - what if we want to do something else when checkbox is clicked?
  • duplicated handleSelectAllClick - we will have a function for this in its parent component anyways because we have "clear selection" in the action menu

Are they not a big deal??

Update:
I figured that handleSelectAllClick should be like this:

const handleSelectAllClick = (e) => {
  if (e.target.checked) {
    setSelectedIds(data.map(({ id }) => id));
  } else {
    setSelectedIds([]);
  }
};

Considering we will have to write this in parent components, I am not sure which is better...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are good points, this was just a suggestion but I think either way is alright!

selectedIds,
stickyHeader,
maxHeight,
} = props;

const [order, setOrder] = useState('asc');
Expand All @@ -101,19 +107,28 @@ export default function TableV2(props) {
const [rowsPerPage, setRowsPerPage] = useState(minRows);

const fullColSpan = columns.reduce((total, column) => total + (column.id ? 1 : 0), 0);
const shouldShowCheckbox = onCheck && handleSelectAllClick && selectedIds;

const handleRequestSort = (event, property) => {
const isAsc = orderBy === property && order === 'asc';
setOrder(isAsc ? 'desc' : 'asc');
setOrderBy(property);
};

const handleRowClick = (event, index) => {
const handleRowClick = (event, row) => {
if (onRowClick) {
onRowClick(event, index);
onRowClick(event, row);
}
};

const handleCheckboxClick = (event, row) => {
if (onCheck) {
onCheck(event, row);
}
// prevent handleRowClick from being called
event.stopPropagation();
};

const handleChangePage = (event, newPage) => {
setPage(newPage);
};
Expand Down Expand Up @@ -142,10 +157,11 @@ export default function TableV2(props) {

return (
<Box sx={{ width: '100%' }}>
<TableContainer>
<TableContainer sx={{ maxHeight }}>
<Table
aria-labelledby="tableTitle"
className={clsx(styles.table, shouldFixTableLayout && styles.fixed)}
stickyHeader={stickyHeader && maxHeight ? true : false}
>
{showHeader && (
<EnhancedTableHead
Expand All @@ -154,21 +170,39 @@ export default function TableV2(props) {
orderBy={orderBy}
onRequestSort={handleRequestSort}
dense={dense}
shouldShowCheckbox={shouldShowCheckbox}
onSelectAllClick={handleSelectAllClick}
numSelected={selectedIds?.length}
rowCount={data.length}
/>
)}
<TableBody className={styles.tableBody}>
{visibleRows.map((row, index) => {
const isItemSelected = selectedIds?.includes(row.id);

return (
<TableRow
key={index}
key={row.id || index}
onClick={(event) => handleRowClick(event, row)}
isItemSelected={isItemSelected}
aria-checked={isItemSelected}
className={clsx(
styles.tableRow,
styles.itemRow,
onRowClick && styles.clickable,
alternatingRowColor ? styles.alternatingRowColor : styles.plainRowColor,
)}
>
{shouldShowCheckbox && (
<TableCell padding="checkbox" className={styles.checkboxCell}>
<Checkbox
color="primary"
onClick={(event) => handleCheckboxClick(event, row)}
checked={isItemSelected}
className={styles.checkbox}
/>
</TableCell>
)}
{columns.map(({ id, format, align, columnProps }) => {
if (!id) {
return null;
Expand Down Expand Up @@ -204,19 +238,22 @@ export default function TableV2(props) {
)}
{columns.some((column) => column.id && column.Footer) && (
<TableRow className={styles.footer}>
{columns.map(({ id, align, columnProps, Footer }) => {
{columns.map(({ id, align, columnProps, Footer }, index) => {
if (!id) {
return null;
}
return (
<TableCell
key={id}
align={align || 'left'}
className={clsx(styles.tableCell, dense && styles.dense)}
{...columnProps}
>
{Footer}
</TableCell>
<>
{!index && shouldShowCheckbox && <TableCell className={styles.tableCell} />}
<TableCell
key={id}
align={align || 'left'}
className={clsx(styles.tableCell, dense && styles.dense)}
{...columnProps}
>
{Footer}
</TableCell>
</>
);
})}
</TableRow>
Expand Down Expand Up @@ -271,6 +308,11 @@ TableV2.propTypes = {
defaultOrderBy: PropTypes.string,
alternatingRowColor: PropTypes.bool,
showHeader: PropTypes.bool,
onCheck: PropTypes.func,
handleSelectAllClick: PropTypes.func,
selectedIds: PropTypes.array,
stickyHeader: PropTypes.bool,
maxHeight: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
};

TableV2.defaultProps = {
Expand Down
57 changes: 50 additions & 7 deletions packages/webapp/src/components/Table/styles.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,57 @@
*/

.table {
padding: 0.5px;
border-collapse: separate;

&.fixed {
table-layout: fixed;
}
}

.headerRow {
border-top: 1px solid var(--Colors-Neutral-Neutral-50);
border-bottom: 1px solid var(--Colors-Neutral-Neutral-50);
margin-bottom: 16px;

&.clickable {
cursor: pointer;
}

th {
border-top: 1px solid var(--Colors-Neutral-Neutral-50);
border-bottom: 1px solid var(--Colors-Neutral-Neutral-50);
}
}

.tableRow {
&.clickable {
cursor: pointer;
}

&[aria-checked="true"] {
border: solid 1px var(--Colors-Accent-Accent-yellow-400);
background-color: var(--Colors-Accent-Accent-yellow-50);

& > td {
border-top: solid 1px var(--Colors-Accent-Accent-yellow-400);
border-bottom: solid 1px var(--Colors-Accent-Accent-yellow-400);

&:first-child {
border-radius: 4px 0 0 4px;
border: solid 1px var(--Colors-Accent-Accent-yellow-400);
border-right: none;
}
&:last-child {
border-radius: 0 4px 4px 0;
border: solid 1px var(--Colors-Accent-Accent-yellow-400);
border-left: none;
}
};
}

// override mui's style
td:first-child {
border-top: solid 1px transparent;
border-bottom: solid 1px var(--grey200);
}
}

.itemRow {
Expand Down Expand Up @@ -74,12 +106,9 @@
.tableCell {
padding: 0 12px;
height: 56px;
border-top: solid 1px transparent;
border-bottom: solid 1px var(--grey200);

&.tableHead {
border-bottom: none;
}

&.dense {
padding: 0 12px;
height: 40px;
Expand Down Expand Up @@ -172,3 +201,17 @@
line-height: 16px;
color: transparent;
}

// checkbox
.checkboxCell {
width: 32px;
text-align: center;

span {
padding: 0;

&:hover {
background-color: transparent;
}
}
}
5 changes: 5 additions & 0 deletions packages/webapp/src/components/Table/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,9 @@ export type TableV2Props = {
shouldFixTableLayout?: boolean;
defaultOrderBy?: string;
showHeader?: boolean;
onCheck?: (id: string | number) => void;
handleSelectAllClick?: () => void;
selectedIds?: (string | number)[];
stickyHeader?: boolean;
maxHeight?: number;
};
Loading
Loading