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

Jump to bottom

RouteReportPage.js added table pagination #1133

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

RouteReportPage.js added table pagination #1133

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5ff21a5
RouteReportPage.js added table pagination
Jun 13, 2023
803a63b
Add Show / Hide Functionality to RouteReportPage.js
Jun 13, 2023
dbef170
Merge branch 'traccar:master' into master
kriistiina Jun 13, 2023
837046a
[fix] Pagination active after 1k records;
Jun 14, 2023
6df31a9
Merge remote-tracking branch 'origin/master' into master
Jun 14, 2023
45d52b1
[fix] show all items when less than 1000;
Jun 20, 2023
16d15e3
[fix] Route report using react-virtuoso with Table component;
Jun 21, 2023
549a3f7
[fix] Route report using react-window FizedSizeGrid component;
Jul 4, 2023
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
3 changes: 2 additions & 1 deletion modern/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"eslint-config-airbnb": "^19.0.4",
"eslint-plugin-import": "^2.27.5",
"eslint-plugin-react": "^7.32.2",
"eslint-plugin-react-hooks": "^4.6.0"
"eslint-plugin-react-hooks": "^4.6.0",
"react-virtuoso": "^4.3.10"
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this here in dev dependencies?

}
}
104 changes: 66 additions & 38 deletions modern/src/reports/RouteReportPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import {
} from '@mui/material';
import GpsFixedIcon from '@mui/icons-material/GpsFixed';
import LocationSearchingIcon from '@mui/icons-material/LocationSearching';
// eslint-disable-next-line import/no-extraneous-dependencies
import { TableVirtuoso } from 'react-virtuoso';
import TableContainer from '@mui/material/TableContainer';
import ReportFilter from './components/ReportFilter';
import { useTranslation } from '../common/components/LocalizationProvider';
import PageLayout from '../common/components/PageLayout';
Expand All @@ -19,11 +22,22 @@ import MapRoutePath from '../map/MapRoutePath';
import MapRoutePoints from '../map/MapRoutePoints';
import MapPositions from '../map/MapPositions';
import useReportStyles from './common/useReportStyles';
import TableShimmer from '../common/components/TableShimmer';
import MapCamera from '../map/MapCamera';
import MapGeofence from '../map/MapGeofence';
import scheduleReport from './common/scheduleReport';

const VirtuosoTableComponents = {
Scroller: React.forwardRef((props, ref) => (
<TableContainer {...props} ref={ref} />
)),
Table: (props) => (
<Table {...props} />
),
TableHead,
TableRow: ({ item: _item, ...props }) => <TableRow {...props} />,
TableBody: React.forwardRef((props, ref) => <TableBody {...props} ref={ref} />),
};

const RouteReportPage = () => {
const navigate = useNavigate();
const classes = useReportStyles();
Expand Down Expand Up @@ -95,6 +109,48 @@ const RouteReportPage = () => {
}
});

const fixedHeaderContent = () => (
<TableRow style={{ backgroundColor: 'white' }}>
<TableCell className={classes.columnAction} />
<TableCell>{t('sharedDevice')}</TableCell>
{columns.map((key) => (
<TableCell
key={key}
variant="head"
align={key.numeric || false ? 'right' : 'left'}
>
{positionAttributes[key]?.name || key}
</TableCell>
))}
</TableRow>
);

const rowContent = (_index, item) => (
<>
<TableCell className={classes.columnAction} padding="none">
{selectedItem === item ? (
<IconButton size="small" onClick={() => setSelectedItem(null)}>
<GpsFixedIcon fontSize="small" />
</IconButton>
) : (
<IconButton size="small" onClick={() => setSelectedItem(item)}>
<LocationSearchingIcon fontSize="small" />
</IconButton>
)}
</TableCell>
<TableCell>{devices[item.deviceId].name}</TableCell>
{columns.map((key) => (
<TableCell key={key}>
<PositionValue
position={item}
property={item.hasOwnProperty(key) ? key : null}
attribute={item.hasOwnProperty(key) ? null : key}
/>
</TableCell>
))}
</>
);

return (
<PageLayout menu={<ReportsMenu />} breadcrumbs={['reportTitle', 'reportRoute']}>
<div className={classes.container}>
Expand All @@ -116,7 +172,7 @@ const RouteReportPage = () => {
<MapCamera positions={items} />
</div>
)}
<div className={classes.containerMain}>
<div>
<div className={classes.header}>
<ReportFilter handleSubmit={handleSubmit} handleSchedule={handleSchedule} multiDevice>
<ColumnSelect
Expand All @@ -128,43 +184,15 @@ const RouteReportPage = () => {
/>
</ReportFilter>
</div>
<Table>
<TableHead>
<TableRow>
<TableCell className={classes.columnAction} />
<TableCell>{t('sharedDevice')}</TableCell>
{columns.map((key) => (<TableCell key={key}>{positionAttributes[key]?.name || key}</TableCell>))}
</TableRow>
</TableHead>
<TableBody>
{!loading ? items.slice(0, 4000).map((item) => (
<TableRow key={item.id}>
<TableCell className={classes.columnAction} padding="none">
{selectedItem === item ? (
<IconButton size="small" onClick={() => setSelectedItem(null)}>
<GpsFixedIcon fontSize="small" />
</IconButton>
) : (
<IconButton size="small" onClick={() => setSelectedItem(item)}>
<LocationSearchingIcon fontSize="small" />
</IconButton>
)}
</TableCell>
<TableCell>{devices[item.deviceId].name}</TableCell>
{columns.map((key) => (
<TableCell key={key}>
<PositionValue
position={item}
property={item.hasOwnProperty(key) ? key : null}
attribute={item.hasOwnProperty(key) ? null : key}
/>
</TableCell>
))}
</TableRow>
)) : (<TableShimmer columns={columns.length + 2} startAction />)}
</TableBody>
</Table>
</div>
{!loading && (
<TableVirtuoso
Copy link
Member

Choose a reason for hiding this comment

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

We already have react-window. Why are we adding another library that basically does the same thing?

Copy link

@LiGuru LiGuru Jun 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It was my decision, react-window is pretty small, but needs more work to fit complicated needs.

https://dev.to/sanamumtaz/react-virtualization-react-window-vs-react-virtuoso-8g

MUI Table provided a solution with virtuoso, which is straight forward approach, I think.

And of course you mentioned that there are options "react-window or something similar".

;)

Copy link
Member

Choose a reason for hiding this comment

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

Obviously I didn't mean that it's ok to just add more libraries that do the same thing. If you want to replace the virtualization library, you have to provide justification why the old one is bad and the new one is better. Then migrate all existing use cases to the new library.

Copy link

Choose a reason for hiding this comment

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

Ok, let's clarify.
Kristina should implement rendering with react-window.

Kristina should not use additional libraries without clarification by you.

What about adding filters and sorting?

Copy link
Member

Choose a reason for hiding this comment

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

Adding any new things we can discuss separately first.

data={items}
components={VirtuosoTableComponents}
fixedHeaderContent={fixedHeaderContent}
itemContent={rowContent}
/>
)}
</div>
</PageLayout>
);
Expand Down
1 change: 1 addition & 0 deletions modern/src/reports/common/useReportStyles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export default makeStyles((theme) => ({
position: 'sticky',
left: 0,
display: 'flex',
height: '100%',
Copy link
Member

Choose a reason for hiding this comment

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

Please explain this.

flexDirection: 'column',
alignItems: 'stretch',
},
Expand Down