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

Fix/fe/consistent initial state #1663

Merged
merged 14 commits into from
Jan 29, 2025
Merged

Fix/fe/consistent initial state #1663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a90ed64
fix monitorStatusHeader name, safe property access
ajhollid Jan 29, 2025
cadb9bb
provide default values for pagination
ajhollid Jan 29, 2025
65c909e
provide default props for Table
ajhollid Jan 29, 2025
6788b56
add ref to stop initial search
ajhollid Jan 29, 2025
ef28f8c
simplify UptimeDataTable
ajhollid Jan 29, 2025
12711e8
set initial state explicitly to for consistency
ajhollid Jan 29, 2025
a985178
Use MonitorCountHeader, initialze state to undefined'
ajhollid Jan 29, 2025
cfb5c7a
Update hooks to use undefined for initial state
ajhollid Jan 29, 2025
4c3b348
safe param access
ajhollid Jan 29, 2025
80a16ef
safe param access
ajhollid Jan 29, 2025
0feb205
remove null return
ajhollid Jan 29, 2025
c1fa39f
move fallback css to fallback css file
ajhollid Jan 29, 2025
d2d2e87
Add a network fallback component
ajhollid Jan 29, 2025
4559215
add network error state to hooks
ajhollid Jan 29, 2025
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
16 changes: 16 additions & 0 deletions Client/src/Components/Fallback/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@
font-size: var(--env-var-font-size-medium);
}

[class*="fallback__"] .background-pattern-svg {
position: absolute;
top: 0;
left: 50%;
transform: translate(-50%, -50%);
z-index: 0;

width: 100%;
max-width: 800px;
height: 100%;
max-height: 800px;

background-position: center;
background-size: cover;
background-repeat: no-repeat;
}
.fallback__status > .MuiStack-root {
margin-left: var(--env-var-spacing-2);
}
17 changes: 0 additions & 17 deletions Client/src/Components/Layouts/HomeLayout/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,3 @@
min-height: calc(100vh - var(--env-var-spacing-2) * 2);
flex: 1;
}

.home-layout > div:has(> [class*="fallback__"]) .background-pattern-svg {
position: absolute;
top: 0;
left: 50%;
transform: translate(-50%, -50%);
z-index: 0;

width: 100%;
max-width: 800px;
height: 100%;
max-height: 800px;

background-position: center;
background-size: cover;
background-repeat: no-repeat;
}
10 changes: 5 additions & 5 deletions Client/src/Components/MonitorStatusHeader/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import ConfigButton from "./ConfigButton";
import SkeletonLayout from "./skeleton";
import PropTypes from "prop-types";

const MonitorHeader = ({ shouldRender = true, isAdmin, monitor }) => {
const MonitorStatusHeader = ({ shouldRender = true, isAdmin, monitor }) => {
const theme = useTheme();
const { statusColor, statusMsg, determineState } = useUtils();
if (!shouldRender) {
Expand All @@ -21,7 +21,7 @@ const MonitorHeader = ({ shouldRender = true, isAdmin, monitor }) => {
justifyContent="space-between"
>
<Stack>
<Typography variant="h1">{monitor.name}</Typography>
<Typography variant="h1">{monitor?.name}</Typography>
<Stack
direction="row"
alignItems={"center"}
Expand All @@ -39,16 +39,16 @@ const MonitorHeader = ({ shouldRender = true, isAdmin, monitor }) => {
</Stack>
<ConfigButton
shouldRender={isAdmin}
monitorId={monitor._id}
monitorId={monitor?._id}
/>
</Stack>
);
};

MonitorHeader.propTypes = {
MonitorStatusHeader.propTypes = {
shouldRender: PropTypes.bool,
isAdmin: PropTypes.bool,
monitor: PropTypes.object,
};

export default MonitorHeader;
export default MonitorStatusHeader;
85 changes: 85 additions & 0 deletions Client/src/Components/NetworkErrorFallback/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { useTheme } from "@emotion/react";
import { Box, Stack, Typography } from "@mui/material";
import Skeleton from "../../assets/Images/create-placeholder.svg?react";
import SkeletonDark from "../../assets/Images/create-placeholder-dark.svg?react";
import Background from "../../assets/Images/background-grid.svg?react";
import { useSelector } from "react-redux";

/**
* Fallback component to display a fallback UI for network errors
*
* @returns {JSX.Element} The rendered fallback UI.
*/

const NetworkErrorFallback = () => {
const theme = useTheme();
const mode = useSelector((state) => state.ui.mode);

return (
<Box
className="page-speed"
position="relative"
border={1}
borderColor={theme.palette.primary.lowContrast}
borderRadius={theme.shape.borderRadius}
backgroundColor={theme.palette.primary.main}
overflow="hidden"
sx={{
borderStyle: "dashed",
}}
>
<Stack
alignItems="center"
gap={theme.spacing(20)}
sx={{
width: "fit-content",
margin: "auto",
marginTop: "100px",
}}
>
{mode === "light" ? (
<Skeleton style={{ zIndex: 1 }} />
) : (
<SkeletonDark style={{ zIndex: 1 }} />
)}
<Box
sx={{
"& svg g g:last-of-type path": {
stroke: theme.palette.primary.lowContrast,
},
position: "absolute",
top: "0",
left: "50%",
transform: "translate(-50%, -50%)",
width: "100%",
maxWidth: "800px",
height: "100%",
maxHeight: "800px",
backgroundPosition: "center",
backgroundSize: "cover",
backgroundRepeat: "no-repeat",
}}
>
<Background style={{ width: "100%" }} />
</Box>
<Stack
gap={theme.spacing(4)}
alignItems="center"
maxWidth={"300px"}
zIndex={1}
>
<Typography
variant="h1"
marginY={theme.spacing(4)}
color={theme.palette.primary.contrastTextTertiary}
>
Network error
</Typography>
<Typography>Please check your connection</Typography>
</Stack>
</Stack>
</Box>
);
};

export default NetworkErrorFallback;
10 changes: 5 additions & 5 deletions Client/src/Components/Table/TablePagination/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import SelectorVertical from "../../../assets/icons/selector-vertical.svg?react"

Pagination.propTypes = {
paginationLabel: PropTypes.string, // Label for the pagination.
itemCount: PropTypes.number.isRequired, // Total number of items for pagination.
page: PropTypes.number.isRequired, // Current page index.
itemCount: PropTypes.number, // Total number of items for pagination.
page: PropTypes.number, // Current page index.
rowsPerPage: PropTypes.number.isRequired, // Number of rows displayed per page.
handleChangePage: PropTypes.func.isRequired, // Function to handle page changes.
handleChangeRowsPerPage: PropTypes.func, // Function to handle changes in rows per page.
Expand All @@ -29,9 +29,9 @@ const ROWS_PER_PAGE_OPTIONS = [5, 10, 15, 25];
*/
function Pagination({
paginationLabel,
itemCount,
page,
rowsPerPage,
itemCount = 0,
page = 0,
rowsPerPage = 5,
handleChangePage,
handleChangeRowsPerPage,
}) {
Expand Down
13 changes: 10 additions & 3 deletions Client/src/Components/Table/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,14 @@ import { useTheme } from "@emotion/react";
* @returns {JSX.Element} The rendered table component.
*/

const DataTable = ({ headers, data, config = { emptyView: "No data" } }) => {
const DataTable = ({
headers = [],
data = [],
config = {
emptyView: "No data",
onRowClick: () => {},
},
}) => {
const theme = useTheme();
if ((headers?.length ?? 0) === 0) {
return "No data";
Expand Down Expand Up @@ -117,9 +124,9 @@ DataTable.propTypes = {
render: PropTypes.func.isRequired,
})
).isRequired,
data: PropTypes.array.isRequired,
data: PropTypes.array,
config: PropTypes.shape({
onRowClick: PropTypes.func.isRequired,
onRowClick: PropTypes.func,
rowSX: PropTypes.object,
emptyView: PropTypes.node,
}),
Expand Down
4 changes: 2 additions & 2 deletions Client/src/Pages/PageSpeed/Details/Components/Charts/PieChart.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,6 @@ const PieChart = ({ audits }) => {
const [highlightedItem, setHighLightedItem] = useState(null);
const [expand, setExpand] = useState(false);

if (typeof audits === "undefined") return null;

/**
* Retrieves color properties based on the performance value.
*
Expand Down Expand Up @@ -166,6 +164,8 @@ const PieChart = ({ audits }) => {
*/
let performance = 0;
const getPieData = (audits) => {
if (typeof audits === "undefined") return undefined;

let data = [];
let startAngle = 0;
const padding = 3; // padding between arcs
Expand Down
130 changes: 65 additions & 65 deletions Client/src/Pages/PageSpeed/Details/Components/Charts/PieChartLegend.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,84 +7,84 @@ import PropTypes from "prop-types";
const PieChartLegend = ({ audits }) => {
const theme = useTheme();

if (typeof audits === "undefined") return null;
return (
<LegendBox
icon={<SpeedometerIcon />}
header="Performance metrics"
sx={{ flex: 1 }}
>
{Object.keys(audits).map((key) => {
if (key === "_id") return;
{typeof audits !== "undefined" &&
Object.keys(audits).map((key) => {
if (key === "_id") return;

let audit = audits[key];
let score = audit.score * 100;
let bg =
score >= 90
? theme.palette.success.main
: score >= 50
? theme.palette.warning.main
: score >= 0
? theme.palette.error.main
: theme.palette.tertiary.main;
let audit = audits[key];
let score = audit.score * 100;
let bg =
score >= 90
? theme.palette.success.main
: score >= 50
? theme.palette.warning.main
: score >= 0
? theme.palette.error.main
: theme.palette.tertiary.main;

// Find the position where the number ends and the unit begins
const match = audit.displayValue.match(/(\d+\.?\d*)\s*([a-zA-Z]+)/);
let value;
let unit;
if (match) {
value = match[1];
match[2] === "s" ? (unit = "seconds") : (unit = match[2]);
} else {
value = audit.displayValue;
}
// Find the position where the number ends and the unit begins
const match = audit.displayValue.match(/(\d+\.?\d*)\s*([a-zA-Z]+)/);
let value;
let unit;
if (match) {
value = match[1];
match[2] === "s" ? (unit = "seconds") : (unit = match[2]);
} else {
value = audit.displayValue;
}
Comment on lines +31 to +40
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Yo dawg, let's make this string parsing more robust! 💪

The current string parsing logic could use some love:

  1. No error handling for malformed displayValue
  2. Assignment in ternary operator is causing linting errors
  3. Hardcoded unit conversion only handles 's' to 'seconds'

Here's a cleaner approach:

- const match = audit.displayValue.match(/(\d+\.?\d*)\s*([a-zA-Z]+)/);
- let value;
- let unit;
- if (match) {
-   value = match[1];
-   match[2] === "s" ? (unit = "seconds") : (unit = match[2]);
- } else {
-   value = audit.displayValue;
- }
+ const match = audit.displayValue.match(/(\d+\.?\d*)\s*([a-zA-Z]+)/);
+ const value = match?.[1] ?? audit.displayValue;
+ const rawUnit = match?.[2];
+ const unitMap = {
+   s: "seconds",
+   ms: "milliseconds",
+ };
+ const unit = rawUnit ? (unitMap[rawUnit] ?? rawUnit) : undefined;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Find the position where the number ends and the unit begins
const match = audit.displayValue.match(/(\d+\.?\d*)\s*([a-zA-Z]+)/);
let value;
let unit;
if (match) {
value = match[1];
match[2] === "s" ? (unit = "seconds") : (unit = match[2]);
} else {
value = audit.displayValue;
}
// Find the position where the number ends and the unit begins
const match = audit.displayValue.match(/(\d+\.?\d*)\s*([a-zA-Z]+)/);
const value = match?.[1] ?? audit.displayValue;
const rawUnit = match?.[2];
const unitMap = {
s: "seconds",
ms: "milliseconds",
};
const unit = rawUnit ? (unitMap[rawUnit] ?? rawUnit) : undefined;
🧰 Tools
🪛 Biome (1.9.4)

[error] 37-37: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

[error] 37-37: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


return (
<Stack
flex={1}
key={`${key}-box`}
justifyContent="space-between"
direction="row"
gap={theme.spacing(4)}
p={theme.spacing(3)}
border={1}
borderColor={theme.palette.primary.lowContrast}
borderRadius={4}
>
<Box>
<Typography
fontSize={12}
fontWeight={500}
lineHeight={1}
mb={1}
textTransform="uppercase"
>
{audit.title}
</Typography>
<Typography
component="span"
fontSize={14}
fontWeight={500}
color={theme.palette.primary.contrastText}
>
{value}
return (
<Stack
flex={1}
key={`${key}-box`}
justifyContent="space-between"
direction="row"
gap={theme.spacing(4)}
p={theme.spacing(3)}
border={1}
borderColor={theme.palette.primary.lowContrast}
borderRadius={4}
>
<Box>
<Typography
fontSize={12}
fontWeight={500}
lineHeight={1}
mb={1}
textTransform="uppercase"
>
{audit.title}
</Typography>
<Typography
component="span"
variant="body2"
ml={2}
fontSize={14}
fontWeight={500}
color={theme.palette.primary.contrastText}
>
{unit}
{value}
<Typography
component="span"
variant="body2"
ml={2}
>
{unit}
</Typography>
</Typography>
</Typography>
</Box>
<Box
width={4}
backgroundColor={bg}
borderRadius={4}
/>
</Stack>
);
})}
</Box>
<Box
width={4}
backgroundColor={bg}
borderRadius={4}
/>
</Stack>
);
})}
</LegendBox>
);
};
Expand Down
4 changes: 0 additions & 4 deletions Client/src/Pages/PageSpeed/Details/Components/PageSpeedAreaChart/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ const PageSpeedAreaChart = ({ shouldRender, monitor, metrics, handleMetrics }) =
return <SkeletonLayout />;
}

if (typeof monitor === "undefined") {
return null;
}

const data = monitor?.checks ? [...monitor.checks].reverse() : [];

return (
Expand Down
Loading