Skip to content

Commit fc9301f

Browse files
LoxerisLoxeris
committed
fix: Resolve issues highlighted in PR review
1 parent e180b96 commit fc9301f

File tree

17 files changed

+152
-147
lines changed

17 files changed

+152
-147
lines changed

‎src/app/(dashboard)/jobmonitor/error.tsx‎

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎src/app/(dashboard)/jobmonitor/layout.tsx‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎src/app/(dashboard)/jobmonitor/page.tsx‎

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎src/app/(dashboard)/layout.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import Dashboard from "@/components/layout/Dashboard";
88
import ApplicationsProvider from "@/contexts/ApplicationsProvider";
99
import { OIDCSecure } from "@/components/layout/OIDCSecure";
1010

11-
export default function JobMonitorLayout({
11+
export default function DashboardLayout({
1212
children,
1313
}: {
1414
children: React.ReactNode;

‎src/app/(dashboard)/page.tsx‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,16 @@ export default function Page() {
1010
const appId = searchParams.get("appId");
1111
const [sections] = React.useContext(ApplicationsContext);
1212

13-
const appType = sections
14-
.find((section) => section.items.some((item) => item.id === appId))
15-
?.items.find((item) => item.id === appId)?.type;
13+
const appType = React.useMemo(() => {
14+
const section = sections.find((section) =>
15+
section.items.some((item) => item.id === appId),
16+
);
17+
return section?.items.find((item) => item.id === appId)?.type;
18+
}, [sections, appId]);
1619

17-
const Component = applicationList.find((app) => app.name === appType)
18-
?.component;
20+
const Component = React.useMemo(() => {
21+
return applicationList.find((app) => app.name === appType)?.component;
22+
}, [appType]);
1923

2024
return Component ? <Component /> : <UserDashboard />;
2125
}

‎src/components/applications/ApplicationList.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { Dashboard, FolderCopy, Monitor } from "@mui/icons-material";
22
import JobMonitor from "./JobMonitor";
33
import UserDashboard from "./UserDashboard";
4+
import ApplicationConfig from "@/types/ApplicationConfig";
45

5-
export const applicationList = [
6+
export const applicationList: ApplicationConfig[] = [
67
{ name: "Dashboard", component: UserDashboard, icon: Dashboard },
78
{
89
name: "Job Monitor",

‎src/components/layout/OIDCSecure.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export function OIDCSecure({ children }: OIDCProps) {
2323
if (!isAuthenticated) {
2424
router.push(
2525
"/auth?" +
26+
// URLSearchParams to ensure that auth redirects users to the URL they came from
2627
new URLSearchParams({ redirect: window.location.href }).toString(),
2728
);
2829
}

‎src/components/ui/ApplicationDialog.tsx‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import {
88
Button,
99
Grid,
1010
Icon,
11+
IconButton,
1112
} from "@mui/material";
13+
import { Close } from "@mui/icons-material";
1214
import { applicationList } from "../applications/ApplicationList";
1315

1416
/**
@@ -55,10 +57,26 @@ export default function AppDialog({
5557
fullWidth
5658
maxWidth="sm"
5759
>
58-
<DialogTitle id="application-dialog-label">New Application</DialogTitle>
60+
<DialogTitle id="application-dialog-label">
61+
Available applications
62+
</DialogTitle>
63+
<IconButton
64+
aria-label="close"
65+
onClick={() => setAppDialogOpen(false)}
66+
sx={{
67+
position: "absolute",
68+
right: 8,
69+
top: 8,
70+
color: (theme) => theme.palette.grey[500],
71+
}}
72+
>
73+
<Close />
74+
</IconButton>
5975
<DialogContent>
6076
<DialogContentText sx={{ mb: 2 }}>
61-
Choose the type of application you would like to create.
77+
Click on any application to open it in a new instance in the drawer.
78+
Multiple instances of the same application can be opened
79+
simultaneously.
6280
</DialogContentText>
6381
<Grid container spacing={2}>
6482
{applicationList.map((app) => (
@@ -78,14 +96,6 @@ export default function AppDialog({
7896
))}
7997
</Grid>
8098
</DialogContent>
81-
<DialogActions>
82-
<Button
83-
sx={{ color: "warning.main" }}
84-
onClick={() => setAppDialogOpen(false)}
85-
>
86-
Cancel
87-
</Button>
88-
</DialogActions>
8999
</Dialog>
90100
);
91101
}

‎src/components/ui/DashboardDrawer.tsx‎

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { usePathname } from "next/navigation";
21
import {
32
Box,
43
Button,
@@ -25,10 +24,11 @@ import React, {
2524
} from "react";
2625
import { monitorForElements } from "@atlaskit/pragmatic-drag-and-drop/element/adapter";
2726
import { extractClosestEdge } from "@atlaskit/pragmatic-drag-and-drop-hitbox/closest-edge";
27+
import Image from "next/image";
2828
import DrawerItemGroup from "./DrawerItemGroup";
29-
import { DiracLogo } from "./DiracLogo";
3029
import AppDialog from "./ApplicationDialog";
3130
import { ApplicationsContext } from "@/contexts/ApplicationsProvider";
31+
import { useMUITheme } from "@/hooks/theme";
3232

3333
interface DashboardDrawerProps {
3434
variant: "permanent" | "temporary";
@@ -45,15 +45,12 @@ interface DashboardDrawerProps {
4545
* @returns {JSX.Element} The rendered DashboardDrawer component.
4646
*/
4747
export default function DashboardDrawer(props: DashboardDrawerProps) {
48-
// Get the current URL
49-
const pathname = usePathname();
5048
// Determine the container for the Drawer based on whether the window object exists.
5149
const container =
5250
window !== undefined ? () => window.document.body : undefined;
5351
// Check if the drawer is in "temporary" mode.
5452
const isTemporary = props.variant === "temporary";
55-
56-
// Wether the modal for Application Creation is open
53+
// Whether the modal for Application Creation is open
5754
const [appDialogOpen, setAppDialogOpen] = useState(false);
5855

5956
const [contextMenu, setContextMenu] = React.useState<{
@@ -73,7 +70,10 @@ export default function DashboardDrawer(props: DashboardDrawerProps) {
7370
// Each section has an associated icon and path.
7471
const [userSections, setSections] = useContext(ApplicationsContext);
7572

73+
const theme = useMUITheme();
74+
7675
useEffect(() => {
76+
// Handle changes to sections when drag and drop occurs.
7777
return monitorForElements({
7878
onDrop({ source, location }) {
7979
const target = location.current.dropTargets[0];
@@ -84,6 +84,7 @@ export default function DashboardDrawer(props: DashboardDrawerProps) {
8484
const targetData = target.data;
8585

8686
if (location.current.dropTargets.length == 2) {
87+
// If the target is an item
8788
const groupTitle = targetData.title;
8889
const closestEdgeOfTarget = extractClosestEdge(targetData);
8990
const targetIndex = targetData.index as number;
@@ -105,6 +106,7 @@ export default function DashboardDrawer(props: DashboardDrawerProps) {
105106
destinationIndex,
106107
);
107108
} else {
109+
// If the target is a group
108110
const groupTitle = targetData.title;
109111
const sourceGroup = userSections.find(
110112
(group) => group.title == sourceData.title,
@@ -140,13 +142,13 @@ export default function DashboardDrawer(props: DashboardDrawerProps) {
140142
destinationIndex &&
141143
sourceIndex < destinationIndex
142144
) {
143-
destinationIndex -= 1;
145+
destinationIndex -= 1; // Corrects the index within the same group if needed
144146
}
145147
if (
146148
sourceGroup.title === destinationGroup.title &&
147149
(destinationIndex == null || sourceIndex === destinationIndex)
148150
) {
149-
return;
151+
return; // Nothing to do
150152
}
151153

152154
if (sourceGroup.title === destinationGroup.title) {
@@ -195,8 +197,7 @@ export default function DashboardDrawer(props: DashboardDrawerProps) {
195197
/**
196198
* Handles the creation of a new app in the dashboard drawer.
197199
*
198-
* @param appName - The name of the app.
199-
* @param path - The path of the app.
200+
* @param appType - The type of the app to be created.
200201
* @param icon - The icon component for the app.
201202
*/
202203
const handleAppCreation = (appType: string, icon: ComponentType) => {
@@ -361,8 +362,20 @@ export default function DashboardDrawer(props: DashboardDrawerProps) {
361362
style={{ display: "flex", flexDirection: "column", height: "100%" }}
362363
>
363364
{/* Display the logo in the toolbar section of the drawer. */}
364-
<Toolbar>
365-
<DiracLogo />
365+
<Toolbar
366+
sx={{
367+
position: "sticky",
368+
top: "0",
369+
zIndex: 1,
370+
backgroundColor: theme.palette.background.default,
371+
}}
372+
>
373+
<Image
374+
src="/DIRAC-logo.png"
375+
alt="DIRAC logo"
376+
width={150}
377+
height={45}
378+
/>
366379
</Toolbar>
367380
{/* Map over user sections and render them as list items in the drawer. */}
368381
<List>
@@ -380,8 +393,17 @@ export default function DashboardDrawer(props: DashboardDrawerProps) {
380393
</ListItem>
381394
))}
382395
</List>
383-
{/* Render a link to documentation, positioned at the bottom of the drawer. */}
384-
<List sx={{ mt: "auto" }}>
396+
397+
{/* Render a link to documentation and a button to add applications, positioned at the bottom of the drawer. */}
398+
<List
399+
sx={{
400+
mt: "auto",
401+
position: "sticky",
402+
bottom: "0",
403+
zIndex: 1,
404+
backgroundColor: theme.palette.background.default,
405+
}}
406+
>
385407
<ListItem key={"Add application"}>
386408
<ListItemButton onClick={() => setAppDialogOpen(true)}>
387409
<ListItemIcon>

‎src/components/ui/DiracLogo.tsx‎

Lines changed: 0 additions & 16 deletions
This file was deleted.

0 commit comments

Comments
 (0)