-
Notifications
You must be signed in to change notification settings - Fork 16
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.
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
Table Widget: core functionality #154
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
a5c697a
Table widget from old PR
bbecquet b38c51c
Fix typo in story
bbecquet 4c66f34
Code simplifications
bbecquet b86d810
Style changes to match design
bbecquet 8e5af12
Remove stories for table sub-components
bbecquet 76e98da
Reorganize and simplify
bbecquet 3f6024d
TableWidget with feature pagination/sort
bbecquet 09cc6b2
Adapt storybook to external data pagination/sorting
bbecquet 25ddc4c
Fix 'columns' prop typing
bbecquet dabd5be
Remove console.log
bbecquet 25d16b4
Fix after rebase
bbecquet be78e75
Simplify component
bbecquet e6d942e
Add test for TableWidgetUI
bbecquet f15fa8d
Fix 0-based MUI vs. 1-based API
bbecquet 510896d
Naming consistency
bbecquet f601c6f
Reset the page to 1 when the viewport changes
bbecquet e27e1c0
Update changelog
bbecquet 77b5ec2
Also reset page on filter change
bbecquet 7a63e93
Also reset page on data source change
bbecquet 0b128b9
Add a test for TableModel
bbecquet File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
import React from 'react'; | ||
import { render, fireEvent, screen } from '@testing-library/react'; | ||
import TableWidgetUI from '../../src/widgets/TableWidgetUI/TableWidgetUI'; | ||
import { getMaterialUIContext } from './testUtils'; | ||
import { columns, rows } from '../../src/widgets/TableWidgetUI/mockData'; | ||
|
||
describe('TableWidgetUI', () => { | ||
const Widget = (props) => | ||
getMaterialUIContext(<TableWidgetUI columns={columns} rows={rows} {...props} />); | ||
|
||
test('renders with default props', () => { | ||
render(<Widget />); | ||
expect(screen.queryByText('State / Province')).toBeInTheDocument(); | ||
}); | ||
|
||
test('renders only specified columns', () => { | ||
const keptFields = ['address', 'city']; | ||
const filteredColumns = columns.filter((col) => keptFields.includes(col.field)); | ||
render(<Widget columns={filteredColumns} />); | ||
columns.forEach((col) => { | ||
if (keptFields.includes(col.field)) { | ||
expect(screen.queryByText(col.headerName)).toBeInTheDocument(); | ||
} else { | ||
expect(screen.queryByText(col.headerName)).not.toBeInTheDocument(); | ||
} | ||
}); | ||
}); | ||
|
||
test('calls callback when clicking on a row', () => { | ||
const mockOnRowClick = jest.fn(); | ||
|
||
render(<Widget onRowClick={mockOnRowClick} />); | ||
|
||
const row = rows[1]; | ||
fireEvent.click(screen.getByText(row.address)); | ||
expect(mockOnRowClick).toHaveBeenCalledWith(row); | ||
}); | ||
|
||
describe('sorting', () => { | ||
test('clicking on a column name triggers a sort on this field', () => { | ||
const mockOnSetSortBy = jest.fn(); | ||
const mockOnSetSortDirection = jest.fn(); | ||
|
||
render( | ||
<Widget | ||
sorting | ||
onSetSortBy={mockOnSetSortBy} | ||
onSetSortDirection={mockOnSetSortDirection} | ||
/> | ||
); | ||
|
||
fireEvent.click(screen.getByText('Address')); | ||
expect(mockOnSetSortBy).toHaveBeenCalledWith('address'); | ||
}); | ||
|
||
test('clicking a second time reverse the sort direction', () => { | ||
const mockOnSetSortBy = jest.fn(); | ||
const mockOnSetSortDirection = jest.fn(); | ||
|
||
const props = { | ||
sorting: true, | ||
onSetSortBy: mockOnSetSortBy, | ||
onSetSortDirection: mockOnSetSortDirection | ||
}; | ||
|
||
const { rerender } = render(<Widget {...props} />); | ||
|
||
fireEvent.click(screen.getByText('Address')); | ||
expect(mockOnSetSortBy).toHaveBeenCalledWith('address'); | ||
expect(mockOnSetSortDirection).toHaveBeenCalledWith('asc'); | ||
|
||
// the component is fully controlled so we need to simulate the change of sort props | ||
rerender(<Widget {...props} sortBy='address' sortDirection='asc' />); | ||
fireEvent.click(screen.getByText('Address')); | ||
expect(mockOnSetSortDirection).toHaveBeenCalledWith('desc'); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
209 changes: 209 additions & 0 deletions
209
packages/react-ui/src/widgets/TableWidgetUI/TableWidgetUI.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,209 @@ | ||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import { | ||
Table, | ||
TableBody, | ||
TableCell, | ||
TableContainer, | ||
TableHead, | ||
TableRow, | ||
makeStyles, | ||
TableSortLabel, | ||
TablePagination | ||
} from '@material-ui/core'; | ||
|
||
const useStyles = makeStyles((theme) => ({ | ||
tableHead: { | ||
backgroundColor: theme.palette.common.white, | ||
'& .MuiTableCell-head': { | ||
border: 'none' | ||
}, | ||
'& .MuiTableCell-head, & .MuiTableCell-head span': { | ||
...theme.typography.caption, | ||
color: theme.palette.text.secondary | ||
} | ||
}, | ||
tableRow: { | ||
maxHeight: theme.spacing(6.5), | ||
transition: 'background-color 0.25s ease', | ||
'&.MuiTableRow-hover:hover': { | ||
cursor: 'pointer', | ||
backgroundColor: theme.palette.background.default | ||
} | ||
}, | ||
tableCell: { | ||
overflow: 'hidden', | ||
'& p': { | ||
maxWidth: '100%', | ||
whiteSpace: 'nowrap', | ||
overflow: 'hidden', | ||
textOverflow: 'ellipsis' | ||
} | ||
}, | ||
pagination: { | ||
'& .MuiTablePagination-caption': { | ||
...theme.typography.caption | ||
}, | ||
'& .MuiTablePagination-caption:first-of-type': { | ||
color: theme.palette.text.secondary | ||
}, | ||
'& .MuiTablePagination-input': { | ||
minHeight: theme.spacing(4.5), | ||
border: `2px solid ${theme.palette.divider}`, | ||
borderRadius: theme.spacing(0.5) | ||
} | ||
} | ||
})); | ||
|
||
function TableWidgetUI({ | ||
columns, | ||
rows, | ||
sorting, | ||
sortBy, | ||
sortDirection, | ||
onSetSortBy, | ||
onSetSortDirection, | ||
pagination, | ||
totalCount, | ||
page, | ||
onSetPage, | ||
rowsPerPage, | ||
rowsPerPageOptions, | ||
onSetRowsPerPage, | ||
onRowClick | ||
}) { | ||
const classes = useStyles(); | ||
|
||
const handleSort = (sortField) => { | ||
const isAsc = sortBy === sortField && sortDirection === 'asc'; | ||
onSetSortDirection(isAsc ? 'desc' : 'asc'); | ||
onSetSortBy(sortField); | ||
}; | ||
|
||
const handleChangePage = (_event, newPage) => { | ||
onSetPage(newPage + 1); | ||
}; | ||
|
||
const handleChangeRowsPerPage = (event) => { | ||
onSetRowsPerPage(parseInt(event.target.value, 10)); | ||
onSetPage(1); | ||
}; | ||
|
||
return ( | ||
<> | ||
<TableContainer> | ||
<Table> | ||
<TableHeaderComponent | ||
columns={columns} | ||
sorting={sorting} | ||
sortBy={sortBy} | ||
sortDirection={sortDirection} | ||
onSort={handleSort} | ||
/> | ||
<TableBodyComponent columns={columns} rows={rows} onRowClick={onRowClick} /> | ||
</Table> | ||
</TableContainer> | ||
{pagination && ( | ||
<TablePagination | ||
className={classes.pagination} | ||
rowsPerPageOptions={rowsPerPageOptions} | ||
component='div' | ||
count={totalCount} | ||
rowsPerPage={rowsPerPage} | ||
page={page - 1} | ||
onPageChange={handleChangePage} | ||
onRowsPerPageChange={handleChangeRowsPerPage} | ||
/> | ||
)} | ||
</> | ||
); | ||
} | ||
|
||
function TableHeaderComponent({ columns, sorting, sortBy, sortDirection, onSort }) { | ||
const classes = useStyles(); | ||
|
||
return ( | ||
<TableHead className={classes.tableHead}> | ||
<TableRow> | ||
{columns.map(({ field, headerName, align }) => ( | ||
<TableCell key={field} align={align || 'left'}> | ||
{sorting ? ( | ||
<TableSortLabel | ||
active={sortBy === field} | ||
direction={sortBy === field ? sortDirection : 'asc'} | ||
onClick={() => onSort(field)} | ||
> | ||
{headerName} | ||
</TableSortLabel> | ||
) : ( | ||
headerName | ||
)} | ||
</TableCell> | ||
))} | ||
</TableRow> | ||
</TableHead> | ||
); | ||
} | ||
|
||
function TableBodyComponent({ columns, rows, onRowClick }) { | ||
const classes = useStyles(); | ||
|
||
return ( | ||
<TableBody> | ||
{rows.map((row, i) => { | ||
const rowKey = row.cartodb_id || row.id || i; | ||
|
||
return ( | ||
<TableRow | ||
key={rowKey} | ||
className={classes.tableRow} | ||
hover={!!onRowClick} | ||
onClick={() => onRowClick && onRowClick(row)} | ||
> | ||
{columns.map( | ||
({ field, headerName, align, component }) => | ||
headerName && ( | ||
<TableCell | ||
key={`${rowKey}_${field}`} | ||
scope='row' | ||
align={align || 'left'} | ||
className={classes.tableCell} | ||
> | ||
{component ? component(row[field]) : row[field]} | ||
</TableCell> | ||
) | ||
)} | ||
</TableRow> | ||
); | ||
})} | ||
</TableBody> | ||
); | ||
} | ||
|
||
TableWidgetUI.defaultProps = { | ||
sorting: false, | ||
sortDirection: 'asc', | ||
pagination: false, | ||
rowsPerPage: 10, | ||
rowsPerPageOptions: [5, 10, 25] | ||
}; | ||
|
||
TableWidgetUI.propTypes = { | ||
columns: PropTypes.array.isRequired, | ||
rows: PropTypes.array.isRequired, | ||
sorting: PropTypes.bool, | ||
sortBy: PropTypes.string, | ||
sortDirection: PropTypes.string, | ||
onSetSortBy: PropTypes.func, | ||
onSetSortDirection: PropTypes.func, | ||
pagination: PropTypes.bool, | ||
totalCount: PropTypes.number, | ||
page: PropTypes.number, | ||
onSetPage: PropTypes.func, | ||
rowsPerPage: PropTypes.number, | ||
rowsPerPageOptions: PropTypes.array, | ||
onSetRowsPerPage: PropTypes.func, | ||
onRowClick: PropTypes.func | ||
}; | ||
|
||
export default TableWidgetUI; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to make this adjustment because
TablePagination
from MUI is zero-based whereas our pagination API is 1-based (see https://github.com/CartoDB/carto-react/blob/master/packages/react-workers/src/workers/features.worker.js#L166)Or maybe we could change our own implementation to start at page 0 too? Would it be possible @Clebal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bbecquet totally agree, it's a mistake that page is 1-based. Sorry for my late response, I didn't saw this. Can you open a PR with that change?