Skip to content

Commit

Permalink
fix: Redirect jobs page better
Browse files Browse the repository at this point in the history
Whenever you went to the Jobs page from the sidebar, you could click
jobs again and it didn't redirect you to the copr-builds anymore.

This meant that the page would display no tabs selected and there would
be no content below the tabs, such as the Copr Builds table. To fix
this we had to fix the `useEffect` to include `matches` which has the
URL information. That way whenever the URL changes this will also change

To make it even nicer on the eyes, we will hard-import Copr Builds table
so that it will be visible if the url is just `/jobs` again and it
didn't redirect correctly. This also makes it so that the rendering
of the table won't flicker if you click Jobs in the sidebar over and
over. Note that the statuses for the table will still flicker, as it
loads the table again and the implementation as of this commit does not
render the statuses properly and unnecessarily renders it (AFAIK).
  • Loading branch information
Venefilyn authored and lachmanfrantisek committed Apr 8, 2024
1 parent f0a366a commit a88fb52
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions frontend/src/app/Jobs/Jobs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
Text,
TextContent,
} from "@patternfly/react-core";
import { useEffect } from "react";
import { useEffect, useMemo } from "react";
import {
NavLink,
Outlet,
Expand All @@ -21,6 +21,7 @@ import {
useNavigate,
} from "react-router-dom";
import { useTitle } from "../utils/useTitle";
import { CoprBuildsTable } from "./CoprBuildsTable";

const Jobs = () => {
useTitle("Jobs");
Expand All @@ -30,13 +31,18 @@ const Jobs = () => {
(match) => match.pathname === location.pathname,
);

const hasRouteSelected = useMemo(
() => currentMatch?.id != "jobs",
[currentMatch],
);

// if we're not inside a specific route, default to copr-builds and redirect
const navigate = useNavigate();
useEffect(() => {
if (matches[matches.length - 1].id === "jobs") {
navigate("/jobs/copr-builds");
}
}, [navigate]);
}, [matches, navigate]);

return (
<PageGroup>
Expand All @@ -49,7 +55,9 @@ const Jobs = () => {
<PageNavigation>
<Nav aria-label="Job types" variant="tertiary">
<NavList>
<NavItem isActive={currentMatch?.id === "copr-builds"}>
<NavItem
isActive={currentMatch?.id === "copr-builds" || !hasRouteSelected}
>
<NavLink to={"copr-builds"}>Copr Builds</NavLink>
</NavItem>
<NavItem isActive={currentMatch?.id === "koji-builds"}>
Expand Down Expand Up @@ -79,7 +87,7 @@ const Jobs = () => {
</Nav>
</PageNavigation>
<PageSection>
<Outlet />
{hasRouteSelected ? <Outlet /> : <CoprBuildsTable />}
</PageSection>
</PageGroup>
);
Expand Down

0 comments on commit a88fb52

Please sign in to comment.