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

refactor: Move sidebar items around #336

Merged
merged 1 commit into from
Oct 16, 2023
Merged

refactor: Move sidebar items around #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
118 changes: 86 additions & 32 deletions frontend/src/app/AppLayout/AppLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { NavLink, Outlet, matchRoutes, useLocation } from "react-router-dom";
import {
NavLink,
Outlet,
RouteObject,
matchRoutes,
useLocation,
} from "react-router-dom";
import {
Nav,
NavList,
Expand All @@ -19,10 +25,11 @@ import {
Toolbar,
ToolbarContent,
ToolbarItem,
NavExpandable,
} from "@patternfly/react-core";
import { routes } from "../routes";
import packitLogo from "../../static/logo.png";
import { useState, useEffect } from "react";
import React, { useState, useEffect } from "react";
import {
ExternalLinkAltIcon,
CodeBranchIcon,
Expand All @@ -32,6 +39,7 @@ import {
const AppLayout = () => {
const location = useLocation();
const [activeLocationLabel, setActiveLocationLabel] = useState("");
const [activeLocationCategory, setActiveLocationCategory] = useState("");
const currentRouteTree = matchRoutes(routes, location);
// Dynamically set page title and update currently active page in sidebar
useEffect(() => {
Expand All @@ -43,6 +51,7 @@ const AppLayout = () => {
const element = currentRouteTree[index];
if (element.route.handle?.label) {
setActiveLocationLabel(element.route.handle.label);
setActiveLocationCategory(element.route.handle.category || "");
}
}
}, [currentRouteTree]);
Expand Down Expand Up @@ -128,42 +137,87 @@ const AppLayout = () => {
</Masthead>
);

const NavigationRoute: React.FC<{ route: RouteObject; index: string }> = ({
route,
index,
}) => (
<NavItem
key={route.handle.label}
isActive={activeLocationLabel === route.handle.label}
>
<NavLink itemID={index.toString()} to={route.path ?? ""}>
{route.handle.label}
</NavLink>
</NavItem>
);
const addedCategories: string[] = [];
const Navigation = (
<Nav id="nav-primary-simple" theme="dark">
<NavList id="nav-list-simple">
{routes[0].children?.map(
(route, index) =>
route.handle?.label && (
<NavItem
key={route.handle.label}
{routes[0].children?.map((route, index) => {
if (route.handle?.category) {
if (addedCategories.includes(route.handle.category))
return;
const categoryRoutes = routes[0].children?.filter(
(r) => r.handle?.category === route.handle.category,
);
addedCategories.push(route.handle.category);
return (
<NavExpandable
key={route.handle.category}
title={route.handle.category}
groupId={`nav-expandable-group-${route.handle.category}`}
isActive={
activeLocationLabel === route.handle.label
activeLocationCategory ===
route.handle.category
}
isExpanded
>
<NavLink
itemID={index.toString()}
to={route.path ?? ""}
>
{route.handle.label}
</NavLink>
</NavItem>
),
)}
<NavItem key="status" id="status">
<a href="https://status.packit.dev">Status</a>
</NavItem>
<NavItem key="blog-posts" id="blog-posts">
<a href="https://packit.dev/posts">Blog posts</a>
</NavItem>
<NavItem key="faq-page" id="faq-page">
<a
target="_blank"
href="https://packit.dev/docs/faq"
rel="noreferrer"
>
FAQ
</a>
</NavItem>
{categoryRoutes?.map((r, i) => (
<NavigationRoute
route={r}
index={
r.path ||
index.toString() + i.toString()
}
/>
))}
</NavExpandable>
);
}
return (
route.handle?.label && (
<NavigationRoute
route={route}
index={index.toString()}
/>
)
);
})}
<NavExpandable
title="External links"
groupId="nav-expandable-group-external"
// isActive={
// activeGroup === "nav-expandable-group-1"
// }
isExpanded
>
<NavItem key="status" id="status">
<a href="https://status.packit.dev">Status</a>
</NavItem>
<NavItem key="blog-posts" id="blog-posts">
<a href="https://packit.dev/posts">Blog posts</a>
</NavItem>
<NavItem key="faq-page" id="faq-page">
<a
target="_blank"
href="https://packit.dev/docs/faq"
rel="noreferrer"
>
FAQ
</a>
</NavItem>
</NavExpandable>
</NavList>
</Nav>
);
Expand Down
18 changes: 11 additions & 7 deletions frontend/src/app/routes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,21 @@ const routes: RouteObject[] = [
label: "Home",
},
},
{
element: <Projects />,
path: "/projects",
handle: {
divider: true,
label: "Projects",
category: "Dashboards",
},
},
{
Component: Jobs,
id: "jobs",
path: "/jobs",
handle: {
category: "Dashboards",
label: "Jobs",
},
children: [
Expand Down Expand Up @@ -101,16 +111,10 @@ const routes: RouteObject[] = [
element: <Pipelines />,
path: "/pipelines",
handle: {
category: "Dashboards",
label: "Pipelines",
},
},
Copy link
Member

Choose a reason for hiding this comment

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

I would group projects, pipelines and jobs, but am not sure if Projects should be the name of the group.

But can't think of a better name. Maybe something with results, activity,... I don't know..;)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking of adding this in the future so we can select the project we want and see the jobs and pipeline just for that project, then if you select none it shows all jobs and all pipelines. Could somehow get a page for just projects somehow though

But we could also leave it for now and just have the external links categorized

248487948-44d41eda-8021-4a8a-8769-4bf84f12e7b2

Copy link
Member

Choose a reason for hiding this comment

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

OK, now I remember we've discussed this.

I quite like that approach (both having jobs and pipelines in a group now and the future state with the context selector). Maybe, I would just call this group differently.

A few ideas:

  • Service
  • Packit Service
  • Records
  • Results
  • Dashboard(s)
  • Workloads
  • Processing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like both Dashboards and Services

Copy link
Member

Choose a reason for hiding this comment

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

From these two, I slightly prefer Dashboards... Let's pick this and we can easily rename it if needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image

{
element: <Projects />,
path: "/projects",
handle: {
label: "Projects",
},
},
{
path: "/projects/:forge",
element: <Forge />,
Expand Down