From 5adca16b550467e2e3e7a523dd07c3b770e2da56 Mon Sep 17 00:00:00 2001
From: Caio Cabral <ocaiocabral@gmail.com>
Date: Fri, 18 Oct 2024 15:45:50 -0400
Subject: [PATCH] refactor: moving search loading state to onitorTable
 component

---
 .../Home/MonitorTable/Skeleton/index.jsx      |  6 +-
 .../Monitors/Home/MonitorTable/index.jsx      | 55 ++++++++++++----
 Client/src/Pages/Monitors/Home/index.jsx      | 62 +++++--------------
 3 files changed, 61 insertions(+), 62 deletions(-)

diff --git a/Client/src/Pages/Monitors/Home/MonitorTable/Skeleton/index.jsx b/Client/src/Pages/Monitors/Home/MonitorTable/Skeleton/index.jsx
index 186b89725..e30e0a255 100644
--- a/Client/src/Pages/Monitors/Home/MonitorTable/Skeleton/index.jsx
+++ b/Client/src/Pages/Monitors/Home/MonitorTable/Skeleton/index.jsx
@@ -1,10 +1,10 @@
-import { Skeleton, TableBody, TableCell, TableRow } from "@mui/material";
+import { Skeleton, TableCell, TableRow } from "@mui/material";
 const ROWS_NUMBER = 7;
 const ROWS_ARRAY = Array.from({ length: ROWS_NUMBER }, (_, i) => i);
 
 const TableBodySkeleton = () => {
 	return (
-		<TableBody>
+		<>
 			{ROWS_ARRAY.map((row) => (
 				<TableRow key={row}>
 					<TableCell>
@@ -24,7 +24,7 @@ const TableBodySkeleton = () => {
 					</TableCell>
 				</TableRow>
 			))}
-		</TableBody>
+		</>
 	);
 };
 
diff --git a/Client/src/Pages/Monitors/Home/MonitorTable/index.jsx b/Client/src/Pages/Monitors/Home/MonitorTable/index.jsx
index 4e8f5100b..b5e36f634 100644
--- a/Client/src/Pages/Monitors/Home/MonitorTable/index.jsx
+++ b/Client/src/Pages/Monitors/Home/MonitorTable/index.jsx
@@ -12,6 +12,7 @@ import {
 	Stack,
 	Typography,
 	Button,
+	CircularProgress,
 } from "@mui/material";
 import ArrowDownwardRoundedIcon from "@mui/icons-material/ArrowDownwardRounded";
 import ArrowUpwardRoundedIcon from "@mui/icons-material/ArrowUpwardRounded";
@@ -108,7 +109,7 @@ TablePaginationActions.propTypes = {
 	onPageChange: PropTypes.func.isRequired,
 };
 
-const MonitorTable = ({ isAdmin, filter, setLoading }) => {
+const MonitorTable = ({ isAdmin, filter, setLoading, isSearching }) => {
 	const theme = useTheme();
 	const navigate = useNavigate();
 	const dispatch = useDispatch();
@@ -221,7 +222,37 @@ const MonitorTable = ({ isAdmin, filter, setLoading }) => {
 	};
 
 	return (
-		<>
+		<Box position="relative">
+			{isSearching && (
+				<>
+					<Box
+						width="100%"
+						height="100%"
+						position="absolute"
+						sx={{
+							backgroundColor: theme.palette.background.main,
+							opacity: 0.8,
+							zIndex: 100,
+						}}
+					/>
+					<Box
+						height="100%"
+						position="absolute"
+						top="20%"
+						left="50%"
+						sx={{
+							transform: "translateX(-50%)",
+							zIndex: 101,
+						}}
+					>
+						<CircularProgress
+							sx={{
+								color: theme.palette.other.icon,
+							}}
+						/>
+					</Box>
+				</>
+			)}
 			<TableContainer component={Paper}>
 				<Table>
 					<TableHead>
@@ -271,9 +302,10 @@ const MonitorTable = ({ isAdmin, filter, setLoading }) => {
 							<TableCell>Actions</TableCell>
 						</TableRow>
 					</TableHead>
-					{monitors.length > 0 ? (
-						<TableBody>
-							{monitors.map((monitor) => {
+					<TableBody>
+						{/* TODO add empty state. Check if is searching, and empty => skeleton. Is empty, not searching => skeleton */}
+						{monitors.length > 0 ? (
+							monitors.map((monitor) => {
 								let uptimePercentage = "";
 								let percentageColor = theme.palette.percentage.uptimeExcellent;
 
@@ -343,11 +375,11 @@ const MonitorTable = ({ isAdmin, filter, setLoading }) => {
 										</TableCell>
 									</TableRow>
 								);
-							})}
-						</TableBody>
-					) : (
-						<TableBodySkeleton />
-					)}
+							})
+						) : (
+							<TableBodySkeleton />
+						)}
+					</TableBody>
 				</Table>
 			</TableContainer>
 			<Stack
@@ -420,7 +452,7 @@ const MonitorTable = ({ isAdmin, filter, setLoading }) => {
 					}}
 				/>
 			</Stack>
-		</>
+		</Box>
 	);
 };
 
@@ -428,6 +460,7 @@ MonitorTable.propTypes = {
 	isAdmin: PropTypes.bool,
 	filter: PropTypes.string,
 	setLoading: PropTypes.func,
+	isSearching: PropTypes.bool,
 };
 
 const MemoizedMonitorTable = memo(MonitorTable);
diff --git a/Client/src/Pages/Monitors/Home/index.jsx b/Client/src/Pages/Monitors/Home/index.jsx
index e52556bb9..c2413f6cc 100644
--- a/Client/src/Pages/Monitors/Home/index.jsx
+++ b/Client/src/Pages/Monitors/Home/index.jsx
@@ -4,7 +4,7 @@ import { useSelector, useDispatch } from "react-redux";
 import { getUptimeMonitorsByTeamId } from "../../../Features/UptimeMonitors/uptimeMonitorsSlice";
 import { useNavigate } from "react-router-dom";
 import { useTheme } from "@emotion/react";
-import { Box, Button, CircularProgress, Stack, Typography } from "@mui/material";
+import { Box, Button, Stack, Typography } from "@mui/material";
 import PropTypes from "prop-types";
 import SkeletonLayout from "./skeleton";
 import Fallback from "./fallback";
@@ -36,17 +36,15 @@ const Monitors = ({ isAdmin }) => {
 		dispatch(getUptimeMonitorsByTeamId(authState.authToken));
 	}, [authState.authToken, dispatch]);
 
-	//Why are we tying loading to monitors length?
-	const loading =
-		monitorState?.isLoading; /* && monitorState?.monitorsSummary?.monitors?.length === 0 */
+	const loading = monitorState?.isLoading;
 
-	const hasMonitors = monitorState?.monitorsSummary?.monitors?.length !== 0;
-	const noMonitors = monitorState?.monitorsSummary?.monitors?.length === 0;
+	const totalMonitors = monitorState?.monitorsSummary?.monitorCounts?.total;
+	const isTotalMonitorsUndefined = totalMonitors === undefined;
+	const hasMonitors = !isTotalMonitorsUndefined && totalMonitors !== 0;
+	const noMonitors = isTotalMonitorsUndefined || totalMonitors === 0;
 	const canAddMonitor = isAdmin && hasMonitors;
 	const needsAdmin = !isAdmin && noMonitors;
 
-	/* console.log({ loading });
-	console.log({ monitorState }); */
 	return (
 		<Stack
 			className="monitors"
@@ -131,8 +129,7 @@ const Monitors = ({ isAdmin }) => {
 										borderColor={theme.palette.border.light}
 										backgroundColor={theme.palette.background.accent}
 									>
-										{/* TODO maybe we dont need the conditional here, since we already check for monitors before */}
-										{monitorState?.monitorsSummary?.monitorCounts?.total || 0}
+										{totalMonitors}
 									</Box>
 									<Box
 										width="25%"
@@ -140,50 +137,19 @@ const Monitors = ({ isAdmin }) => {
 										ml="auto"
 									>
 										<Search
-											options={monitorState?.monitorsSummary?.monitors ?? []}
+											options={monitorState.monitorsSummary.monitors}
 											filteredBy="name"
 											inputValue={search}
 											handleInputChange={handleSearch}
 										/>
 									</Box>
 								</Stack>
-								<Box position="relative">
-									{isSearching && (
-										<>
-											<Box
-												width="100%"
-												height="100%"
-												position="absolute"
-												sx={{
-													backgroundColor: theme.palette.background.main,
-													opacity: 0.8,
-													zIndex: 100,
-												}}
-											/>
-											<Box
-												height="100%"
-												position="absolute"
-												top="20%"
-												left="50%"
-												sx={{
-													transform: "translateX(-50%)",
-													zIndex: 101,
-												}}
-											>
-												<CircularProgress
-													sx={{
-														color: theme.palette.other.icon,
-													}}
-												/>
-											</Box>
-										</>
-									)}
-									<MonitorTable
-										isAdmin={isAdmin}
-										filter={debouncedFilter}
-										setLoading={setIsSearching}
-									/>
-								</Box>
+								<MonitorTable
+									isAdmin={isAdmin}
+									filter={debouncedFilter}
+									setLoading={setIsSearching}
+									isSearching={isSearching}
+								/>
 							</Box>
 						</>
 					)}