RouteReportPage.js added table pagination by kriistiina · Pull Request #1133 · traccar/traccar-web

kriistiina · 2023-06-13T12:23:29Z

[fix] Browser slowdown after executing a Route report with large number of records;
The 4000 row preview slice limit is removed.

Forum Discussion
Library used: Material-UI table pagination

This is an initial version using the MUI example for table pagination.
Additional features/behavior may be added after discussion and approval.

tananaev · 2023-06-13T12:28:02Z

I think we discussed making it optional if there number exceeds some limit.

Can you also provide screenshots of what it looks like in desktop and mobile views.

kriistiina · 2023-06-13T13:36:51Z

Yes, with some customization, the pagination component can be shown or hidden on a condition.
With the changes I just added, the initial views of Route report look like this for desktop and mobile.

After execution of Route report with large number of records, the pagination component is shown and looks like this for desktop and mobile.

On a following report execution, the pagination component is shown or hidden, based on the number of records in the new report. Reports with less records than the first pagination step do not contain pagination and look like this for desktop and mobile.

tananaev

When I said exceeding some limit, I didn't really mean one page. I was thinking something like 4k limit that we had there or at least 1-2k.

modern/src/reports/RouteReportPage.js

tananaev · 2023-06-13T14:29:06Z

modern/src/reports/RouteReportPage.js

+  };
+
+  const handleChangeRowsPerPage = (event) => {
+    setRowsPerPage(+event.target.value);


Use proper explicit conversion and only if needed. I would also inline it.

[fix] Pagination styling moved to useStyle; [fix] rowsPerPageOptions updated elements; [fix] Removed comments; [fix] onPageChange inlined; [fix] rowsPerPage renamed to itemsPerPage; [fix] onRowsPerPageChange inlined;

tananaev · 2023-06-14T16:28:23Z

modern/src/reports/common/useReportStyles.js

+  pagination: {
+    position: 'static',
+    bottom: 0,
+    '& .MuiTablePagination-spacer': {
+      display: 'none',
+    },
+    '& .MuiTablePagination-toolbar': {
+      justifyContent: 'center',
+    },
+  },


What is this for?

tananaev · 2023-06-14T16:28:55Z

modern/src/reports/RouteReportPage.js

            </TableHead>
            <TableBody>
-              {!loading ? items.slice(0, 4000).map((item) => (
+              {!loading ? items.slice(page * itemsPerPage, page * itemsPerPage + itemsPerPage).map((item) => (


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

What you think, is it tested?

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

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

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

[fix] avoid .slice() when the array fits;

kriistiina · 2023-06-20T09:24:34Z

@tananaev
Sorry about the delay, I was on a business trip last few days. I fixed the issue regarding the visualization of all items when their count is less than 1000 and made the .slice conditional only if the items count exceeds 1000.

tananaev · 2023-06-20T13:05:00Z

What do you think about the comment of using the "react-window" or something similar? The same way we do on the main screen for devices.

[fix] infinite scroll in Table component with always visible ReportFilter and TableHead;

kriistiina · 2023-06-21T10:48:10Z

I used react-virtuoso to load all report items in a Table component, based on this example. This way, the behavior stays close to the one before with infinite scroll, but without the browser slowdown.

tananaev · 2023-06-21T13:16:01Z

modern/package.json

    "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"


Why do we need this here in dev dependencies?

tananaev · 2023-06-21T13:17:45Z

modern/src/reports/common/useReportStyles.js

    position: 'sticky',
    left: 0,
    display: 'flex',
+    height: '100%',


Please explain this.

tananaev · 2023-06-21T13:18:28Z

modern/src/reports/RouteReportPage.js

-          </Table>
        </div>
+        {!loading && (
+          <TableVirtuoso


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

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".

;)

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.

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?

Adding any new things we can discuss separately first.

[fix] Removed additional package react-virtuoso; [fix] Infinite scroll with always visible ReportFilter;

kriistiina · 2023-07-04T13:40:07Z

I managed to visualize the Route report by using the FixedSizeGrid component from react-window and removed the additional react-virtuoso.

tananaev · 2023-07-04T13:59:17Z

modern/src/reports/RouteReportPage.js

+                  columnCount={columns.length + 2}
+                  columnWidth={(columns.length + 2) * (width * (phone ? 0.26 : desktop ? 0.1 : 0.18)) >= width ?
+                    width * (phone ? 0.26 : desktop ? 0.1 : 0.18) : width / (columns.length + 2)}
+                  rowCount={items.length > 0 ? items.length : 5}
+                  rowHeight={52}


This does not look right at all. Why are there a million hardcoded numbers and some crazy calculations?

tananaev · 2023-07-04T13:59:55Z

modern/src/reports/RouteReportPage.js

+                  {({ columnIndex, rowIndex, style }) => {
+                    const item = items[rowIndex];
+                    const columnKey = columns[columnIndex - 2];
+                    return (


The content also doesn't look good. Very hard to understand what's going on.

RouteReportPage.js added table pagination

5ff21a5

[email protected] and others added 2 commits June 13, 2023 16:34

Add Show / Hide Functionality to RouteReportPage.js

803a63b

Merge branch 'traccar:master' into master

dbef170

tananaev reviewed Jun 13, 2023

View reviewed changes

[email protected] added 2 commits June 14, 2023 10:01

[fix] Pagination active after 1k records;

837046a

[fix] Pagination styling moved to useStyle; [fix] rowsPerPageOptions updated elements; [fix] Removed comments; [fix] onPageChange inlined; [fix] rowsPerPage renamed to itemsPerPage; [fix] onRowsPerPageChange inlined;

Merge remote-tracking branch 'origin/master' into master

6df31a9

kriistiina requested a review from tananaev June 14, 2023 15:44

tananaev reviewed Jun 14, 2023

View reviewed changes

[fix] show all items when less than 1000;

45d52b1

[fix] avoid .slice() when the array fits;

[fix] Route report using react-virtuoso with Table component;

16d15e3

[fix] infinite scroll in Table component with always visible ReportFilter and TableHead;

tananaev reviewed Jun 21, 2023

View reviewed changes

[fix] Route report using react-window FizedSizeGrid component;

549a3f7

[fix] Removed additional package react-virtuoso; [fix] Infinite scroll with always visible ReportFilter;

tananaev reviewed Jul 4, 2023

View reviewed changes

Uh oh!

Conversation

kriistiina commented Jun 13, 2023

Uh oh!

tananaev commented Jun 13, 2023

Uh oh!

kriistiina commented Jun 13, 2023

Uh oh!

tananaev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LiGuru Jun 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kriistiina commented Jun 20, 2023

Uh oh!

tananaev commented Jun 20, 2023

Uh oh!

kriistiina commented Jun 21, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LiGuru Jun 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kriistiina commented Jul 4, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LiGuru Jun 14, 2023 •

edited

Loading

LiGuru Jun 21, 2023 •

edited

Loading