ReactJS paginated component with React Router

Question

I've recently started journey with ReactJS and to begin with I wanted to try something a bit more complex - paginated list of elements with back functionality enabled. Below are two main components that I'd like to get feedback about. Especially PagedListings.

import './App.css';
import PagedListings from './components/PagedListings';
import ScrollToTop from './components/ScrollToTop';
import React from "react";
import { Route } from 'react-router';
import { BrowserRouter as Router } from 'react-router-dom';

function App() {
  return (
    <Router>
    <ScrollToTop>
       <Route path={["/", "/:page"]} component={PagedListings}/>
    </ScrollToTop>
    </Router>
  );
}

export default App;

And the actual component executing planned task

import React, { useMemo } from "react";
import ListingsContainer from "./ListingsContainer";
import Pagination from "@material-ui/lab/Pagination";
import { useHistory, useLocation } from "react-router-dom";

const LISTINGS = ["Rec1", "Rec2", "Rec3",
  "Rec4", "Rec5", "Rec6", "Rec7", "Rec8",
  "Rec9", "Rec10",];
const DEFAULT_PAGE = 1;
const MAX_PAGE_COUNT = 10;

function PagedListings(props) {
  const history = useHistory();
  const location = useLocation();
  const urlParams = useMemo(() => {
    return new URLSearchParams(location.search);
  }, [location.search]);

  const handleChange = (
    event: React.MouseEvent<HTMLButtonElement> | null,
    newPage: number
  ) => {
    let oneBasedNewPage = newPage;

    urlParams.set("page", oneBasedNewPage.toString());
    updateURL();
  };

  const updateURL = () => {
    history.push({
      pathname: location.pathname,
      search: `?${urlParams}`,
    });
  };

  function getPageNumber() {
    let newPage = parseInt(urlParams.get("page")) || DEFAULT_PAGE;
    return newPage <= MAX_PAGE_COUNT ? newPage : MAX_PAGE_COUNT;
  }

  return (
    <>
      <ListingsContainer listings={LISTINGS.slice(0, getPageNumber())} />
      <Pagination
        count={MAX_PAGE_COUNT}
        defaultPage={DEFAULT_PAGE}
        page={getPageNumber()}
        onChange={handleChange}
      />
    </>
  );
}

export default PagedListings;

CertainPerformance · Accepted Answer · 2020-12-21 01:57:42Z

Your code looks quite reasonable. There are only a few small things I notice:

Indent the JSX in App a bit more: ScrollToTop is a child of Router, so it should be indented further, not on the same level as Router. (It doesn't really make a difference here, but it's good to be consistent, and could be an improvement if that section ever got more complicated)
Avoid let, prefer const - ESLint rule here.
Or, in the case of oneBasedNewPage, consider defining the argument with that name instead of creating a new variable - and maybe call it oneIndexed, not oneBased:
```
const handleChange = (
  event: React.MouseEvent<HTMLButtonElement> | null,
  oneIndexedNewPage: number
) => {
```

URLSearchParams does not differentiate between numbers and strings.

urlParams.set("page", oneBasedNewPage.toString());

can be

urlParams.set("page", oneIndexedNewPage);

if you wish.

Consider if using Math.min might be clearer than the conditional operator:

function getPageNumber() {
  const newPage = parseInt(urlParams.get("page")) || DEFAULT_PAGE;
  return Math.min(newPage, MAX_PAGE_COUNT);
}

Thank you for your review and all of the suggestions. I haven't been aware of the fact to use const instead of let in place you suggested. — sebap123
– sebap123, Commented Dec 22, 2020 at 9:41

Stack Exchange Network

ReactJS paginated component with React Router

1 Answer 1

You must log in to answer this question.

Hot Network Questions

ReactJS paginated component with React Router

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions