-
Notifications
You must be signed in to change notification settings - Fork 213
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.
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: HTTPS monitors show HTTP on the table #1642
Changes from all commits
a011d83
a18b817
0234cab
be6d2f2
f19ae02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import { Box } from "@mui/material"; | ||
import PropTypes from "prop-types"; | ||
import { useTheme } from "@emotion/react"; | ||
const CircularCount = ({ count }) => { | ||
const theme = useTheme(); | ||
return ( | ||
<Box | ||
component="span" | ||
color={theme.palette.tertiary.contrastText} | ||
border={2} | ||
borderColor={theme.palette.accent.main} | ||
backgroundColor={theme.palette.tertiary.main} | ||
sx={{ | ||
padding: ".25em .75em", | ||
borderRadius: "50%", | ||
fontSize: "12px", | ||
fontWeight: 500, | ||
}} | ||
> | ||
{count} | ||
</Box> | ||
); | ||
}; | ||
|
||
CircularCount.propTypes = { | ||
count: PropTypes.number, | ||
}; | ||
|
||
export default CircularCount; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import { Box, Typography } from "@mui/material"; | ||
import { Stack, Box, Typography } from "@mui/material"; | ||
import PropTypes from "prop-types"; | ||
import { useTheme } from "@emotion/react"; | ||
/** | ||
* Host component. | ||
* This subcomponent receives a params object and displays the host details. | ||
|
@@ -13,44 +14,43 @@ import PropTypes from "prop-types"; | |
* @returns {React.ElementType} Returns a div element with the host details. | ||
*/ | ||
const Host = ({ url, title, percentageColor, percentage }) => { | ||
const noTitle = title === undefined || title === url; | ||
const theme = useTheme(); | ||
console.log(url, title); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yo, we got a console.log in production code! 🚫 Remove the debug console.log statement before merging. - console.log(url, title); |
||
return ( | ||
<Box className="host"> | ||
<Box | ||
display="inline-block" | ||
<Stack> | ||
<Stack | ||
direction="row" | ||
position="relative" | ||
sx={{ | ||
fontWeight: 500, | ||
"&:before": { | ||
position: "absolute", | ||
content: `""`, | ||
width: "4px", | ||
height: "4px", | ||
borderRadius: "50%", | ||
backgroundColor: "gray", | ||
opacity: 0.8, | ||
right: "-10px", | ||
top: "42%", | ||
}, | ||
}} | ||
alignItems="center" | ||
gap={theme.spacing(4)} | ||
> | ||
{title} | ||
</Box> | ||
{percentageColor && percentage && ( | ||
<Typography | ||
component="span" | ||
sx={{ | ||
color: percentageColor, | ||
fontWeight: 500, | ||
/* TODO point font weight to theme */ | ||
ml: "15px", | ||
}} | ||
> | ||
{percentage}% | ||
</Typography> | ||
)} | ||
{!noTitle && <Box sx={{ opacity: 0.6 }}>{url}</Box>} | ||
</Box> | ||
{percentageColor && percentage && ( | ||
<> | ||
<span | ||
style={{ | ||
content: '""', | ||
width: "4px", | ||
height: "4px", | ||
borderRadius: "50%", | ||
backgroundColor: "gray", | ||
opacity: 0.8, | ||
}} | ||
/> | ||
<Typography | ||
component="span" | ||
sx={{ | ||
color: percentageColor, | ||
fontWeight: 500, | ||
}} | ||
> | ||
{percentage}% | ||
</Typography> | ||
</> | ||
)} | ||
</Stack> | ||
<span style={{ opacity: 0.6 }}>{url}</span> | ||
</Stack> | ||
); | ||
}; | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,46 @@ | ||||||||||||||
import { CircularProgress, Box } from "@mui/material"; | ||||||||||||||
import { useTheme } from "@emotion/react"; | ||||||||||||||
import PropTypes from "prop-types"; | ||||||||||||||
const LoadingSpinner = ({ shouldRender }) => { | ||||||||||||||
const theme = useTheme(); | ||||||||||||||
if (shouldRender === false) { | ||||||||||||||
return; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return ( | ||||||||||||||
<> | ||||||||||||||
<Box | ||||||||||||||
width="100%" | ||||||||||||||
height="100%" | ||||||||||||||
position="absolute" | ||||||||||||||
sx={{ | ||||||||||||||
backgroundColor: theme.palette.primary.main, | ||||||||||||||
opacity: 0.8, | ||||||||||||||
zIndex: 100, | ||||||||||||||
}} | ||||||||||||||
/> | ||||||||||||||
Comment on lines
+12
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Knees weak, arms heavy, but this overlay needs to be accessibility ready! 🎯 The overlay Box needs some accessibility love and could use some constants: +const OVERLAY_OPACITY = 0.8;
+const OVERLAY_Z_INDEX = 100;
<Box
width="100%"
height="100%"
position="absolute"
+ role="progressbar"
+ aria-label="Loading content"
sx={{
backgroundColor: theme.palette.primary.main,
- opacity: 0.8,
- zIndex: 100,
+ opacity: OVERLAY_OPACITY,
+ zIndex: OVERLAY_Z_INDEX,
}}
/>
|
||||||||||||||
<Box | ||||||||||||||
height="100%" | ||||||||||||||
position="absolute" | ||||||||||||||
top="50%" | ||||||||||||||
left="50%" | ||||||||||||||
sx={{ | ||||||||||||||
transform: "translateX(-50%)", | ||||||||||||||
zIndex: 101, | ||||||||||||||
}} | ||||||||||||||
> | ||||||||||||||
<CircularProgress | ||||||||||||||
sx={{ | ||||||||||||||
color: theme.palette.accent.main, | ||||||||||||||
}} | ||||||||||||||
/> | ||||||||||||||
</Box> | ||||||||||||||
</> | ||||||||||||||
); | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
LoadingSpinner.propTypes = { | ||||||||||||||
shouldRender: PropTypes.bool, | ||||||||||||||
}; | ||||||||||||||
Comment on lines
+42
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Lose yourself in the PropTypes, the moment, you better never let it go! 🎤 The prop validation needs to be more strict since LoadingSpinner.propTypes = {
- shouldRender: PropTypes.bool,
+ shouldRender: PropTypes.bool.isRequired,
}; 📝 Committable suggestion
Suggested change
|
||||||||||||||
|
||||||||||||||
export default LoadingSpinner; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,43 @@ | ||||||||||||||||||||||||||||||
import { useState } from "react"; | ||||||||||||||||||||||||||||||
import Search from "../../../../../Components/Inputs/Search"; | ||||||||||||||||||||||||||||||
import { Box } from "@mui/material"; | ||||||||||||||||||||||||||||||
import useDebounce from "../../Hooks/useDebounce"; | ||||||||||||||||||||||||||||||
import { useEffect } from "react"; | ||||||||||||||||||||||||||||||
import PropTypes from "prop-types"; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const SearchComponent = ({ monitors, onSearchChange, setIsSearching }) => { | ||||||||||||||||||||||||||||||
const [localSearch, setLocalSearch] = useState(""); | ||||||||||||||||||||||||||||||
const debouncedSearch = useDebounce(localSearch, 500); | ||||||||||||||||||||||||||||||
useEffect(() => { | ||||||||||||||||||||||||||||||
onSearchChange(debouncedSearch); | ||||||||||||||||||||||||||||||
setIsSearching(false); | ||||||||||||||||||||||||||||||
}, [debouncedSearch, onSearchChange, setIsSearching]); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
const handleSearch = (value) => { | ||||||||||||||||||||||||||||||
setLocalSearch(value); | ||||||||||||||||||||||||||||||
setIsSearching(true); | ||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||
<Box | ||||||||||||||||||||||||||||||
width="25%" | ||||||||||||||||||||||||||||||
minWidth={150} | ||||||||||||||||||||||||||||||
ml="auto" | ||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||
<Search | ||||||||||||||||||||||||||||||
options={monitors} | ||||||||||||||||||||||||||||||
filteredBy="name" | ||||||||||||||||||||||||||||||
inputValue={localSearch} | ||||||||||||||||||||||||||||||
handleInputChange={handleSearch} | ||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||
</Box> | ||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
SearchComponent.propTypes = { | ||||||||||||||||||||||||||||||
monitors: PropTypes.array, | ||||||||||||||||||||||||||||||
onSearchChange: PropTypes.func, | ||||||||||||||||||||||||||||||
setIsSearching: PropTypes.func, | ||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||
Comment on lines
+37
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Yo, these PropTypes need more sauce! 🍝 Let's make them more specific and mark required props: SearchComponent.propTypes = {
- monitors: PropTypes.array,
- onSearchChange: PropTypes.func,
- setIsSearching: PropTypes.func,
+ monitors: PropTypes.arrayOf(
+ PropTypes.shape({
+ name: PropTypes.string.isRequired,
+ })
+ ).isRequired,
+ onSearchChange: PropTypes.func.isRequired,
+ setIsSearching: PropTypes.func.isRequired,
}; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
export default SearchComponent; |
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,36 @@ | ||||||||||||||||
import PropTypes from "prop-types"; | ||||||||||||||||
import { Stack } from "@mui/material"; | ||||||||||||||||
import StatusBox from "./statusBox"; | ||||||||||||||||
import { useTheme } from "@emotion/react"; | ||||||||||||||||
import SkeletonLayout from "./skeleton"; | ||||||||||||||||
|
||||||||||||||||
const StatusBoxes = ({ shouldRender, monitorsSummary }) => { | ||||||||||||||||
const theme = useTheme(); | ||||||||||||||||
if (!shouldRender) return <SkeletonLayout shouldRender={shouldRender} />; | ||||||||||||||||
return ( | ||||||||||||||||
<Stack | ||||||||||||||||
gap={theme.spacing(8)} | ||||||||||||||||
direction="row" | ||||||||||||||||
justifyContent="space-between" | ||||||||||||||||
> | ||||||||||||||||
<StatusBox | ||||||||||||||||
title="up" | ||||||||||||||||
value={monitorsSummary?.upMonitors ?? 0} | ||||||||||||||||
/> | ||||||||||||||||
<StatusBox | ||||||||||||||||
title="down" | ||||||||||||||||
value={monitorsSummary?.downMonitors ?? 0} | ||||||||||||||||
/> | ||||||||||||||||
<StatusBox | ||||||||||||||||
title="paused" | ||||||||||||||||
value={monitorsSummary?.pausedMonitors ?? 0} | ||||||||||||||||
/> | ||||||||||||||||
</Stack> | ||||||||||||||||
); | ||||||||||||||||
}; | ||||||||||||||||
|
||||||||||||||||
StatusBoxes.propTypes = { | ||||||||||||||||
monitorsSummary: PropTypes.object.isRequired, | ||||||||||||||||
}; | ||||||||||||||||
Comment on lines
+32
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Yo, we're missing some PropTypes validation! 📝 The StatusBoxes.propTypes = {
monitorsSummary: PropTypes.object.isRequired,
+ shouldRender: PropTypes.bool.isRequired,
}; 📝 Committable suggestion
Suggested change
|
||||||||||||||||
|
||||||||||||||||
export default StatusBoxes; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import { Skeleton, Stack } from "@mui/material"; | ||
import { useTheme } from "@emotion/react"; | ||
|
||
const SkeletonLayout = () => { | ||
const theme = useTheme(); | ||
return ( | ||
<Stack | ||
gap={theme.spacing(12)} | ||
direction="row" | ||
justifyContent="space-between" | ||
> | ||
<Skeleton | ||
variant="rounded" | ||
width="100%" | ||
height={100} | ||
/> | ||
<Skeleton | ||
variant="rounded" | ||
width="100%" | ||
height={100} | ||
/> | ||
<Skeleton | ||
variant="rounded" | ||
width="100%" | ||
height={100} | ||
/> | ||
</Stack> | ||
); | ||
}; | ||
|
||
export default SkeletonLayout; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Yo dawg, I heard you like theme variables! 🎨
The hardcoded font size "12px" should be replaced with a theme typography value to maintain consistency and follow the PR's guideline about avoiding hardcoded values.