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.

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

RouteReportPage.js added table pagination #1133

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
24 changes: 22 additions & 2 deletions modern/src/reports/RouteReportPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { Fragment, useCallback, useState } from 'react';
import { useSelector } from 'react-redux';
import { useNavigate } from 'react-router-dom';
import {
IconButton, Table, TableBody, TableCell, TableHead, TableRow,
IconButton, Table, TableBody, TableCell, TableHead, TablePagination, TableRow,
} from '@mui/material';
import GpsFixedIcon from '@mui/icons-material/GpsFixed';
import LocationSearchingIcon from '@mui/icons-material/LocationSearching';
Expand Down Expand Up @@ -39,6 +39,9 @@ const RouteReportPage = () => {
const [loading, setLoading] = useState(false);
const [selectedItem, setSelectedItem] = useState(null);

const [page, setPage] = useState(0);
const [itemsPerPage, setItemsPerPage] = useState(50);

const onMapPointClick = useCallback((positionId) => {
setSelectedItem(items.find((it) => it.id === positionId));
}, [items, setSelectedItem]);
Expand Down Expand Up @@ -137,7 +140,7 @@ const RouteReportPage = () => {
</TableRow>
</TableHead>
<TableBody>
{!loading ? items.slice(0, 4000).map((item) => (
{!loading ? items.slice(page * itemsPerPage, page * itemsPerPage + itemsPerPage).map((item) => (
Copy link
Member

Choose a reason for hiding this comment

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

Have you tested that it shows 1000 if less than 1000?

Copy link

@LiGuru LiGuru Jun 14, 2023

Choose a reason for hiding this comment

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

What you think, is it tested?

{!loading ? items.slice(page * itemsPerPage, items.length > 1000 ? page * itemsPerPage + itemsPerPage : items.length).map((item, id) => (
😁

Copy link
Member

Choose a reason for hiding this comment

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

I think it's too complicated and not very readable. Also we don't need to do slice if the array fits.

Choose a reason for hiding this comment

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

Hey there,

I wanted to suggest adding virtualization, like react-window, to improve the table's performance. It can optimize rendering, especially for large datasets, by only rendering visible rows. This can enhance the user experience by reducing memory usage and providing smoother scrolling.

Consider exploring the integration of react-window for better efficiency. It's a popular library for efficient virtualization in React applications.

Thanks for considering this suggestion!

Best regards,
Mohammad Raufzahed

<TableRow key={item.id}>
<TableCell className={classes.columnAction} padding="none">
{selectedItem === item ? (
Expand All @@ -164,6 +167,23 @@ const RouteReportPage = () => {
)) : (<TableShimmer columns={columns.length + 2} startAction />)}
</TableBody>
</Table>
{items.length > 1000 && (
<TablePagination
className={classes.pagination}
rowsPerPageOptions={[50, 100, 200, 500]}
component="div"
count={items.length}
rowsPerPage={itemsPerPage}
page={page}
onPageChange={(event, page) => setPage(page)}
onRowsPerPageChange={
(event) => {
setItemsPerPage(Number(event.target.value));
setPage(0);
}
}
/>
)}
</div>
</div>
</PageLayout>
Expand Down
10 changes: 10 additions & 0 deletions modern/src/reports/common/useReportStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,14 @@ export default makeStyles((theme) => ({
flexGrow: 1,
overflow: 'hidden',
},
pagination: {
position: 'static',
bottom: 0,
'& .MuiTablePagination-spacer': {
display: 'none',
},
'& .MuiTablePagination-toolbar': {
justifyContent: 'center',
},
},
Copy link
Member

Choose a reason for hiding this comment

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

What is this for?

}));