-
Notifications
You must be signed in to change notification settings - Fork 251
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 #1626 #1630
Changes from all commits
d249efd
a011d83
a18b817
0234cab
be6d2f2
7499a2e
c35472f
e49f9a3
e81dff1
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); | ||||||||||||||||
Comment on lines
+17
to
+18
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. Mom's spaghetti alert! 🍝 Debug code detected! Remove the console.log statement before it ends up in production, fam! const theme = useTheme();
- console.log(url, title); 📝 Committable suggestion
Suggested change
|
||||||||||||||||
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> | ||||||||||||||||
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. Opportunity's knocking - we're missing the protocol fix! The PR's main objective is to fix HTTPS monitors showing as HTTP, but I don't see any changes addressing this. We should modify how the URL is displayed to properly indicate the protocol. - <span style={{ opacity: 0.6 }}>{url}</span>
+ <Typography
+ component="span"
+ sx={{ opacity: 0.6 }}
+ >
+ {url.replace(/^https?:\/\//, 'http(s)://')}
+ </Typography> 📝 Committable suggestion
Suggested change
|
||||||||||||||||
</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, | ||
}} | ||
/> | ||
<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, | ||
}; | ||
|
||
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, | ||
}; | ||
|
||
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. Eh, the PropTypes are looking a bit thin there, bud! The component's missing PropTypes validation for the Here's how to beef it up: StatusBoxes.propTypes = {
+ shouldRender: PropTypes.bool.isRequired,
- monitorsSummary: PropTypes.object.isRequired,
+ monitorsSummary: PropTypes.shape({
+ upMonitors: PropTypes.number,
+ downMonitors: PropTypes.number,
+ pausedMonitors: PropTypes.number
+ }).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, the PropTypes validation needs to be stronger, eh!
The
count
prop should be marked as required since it's essential for the component to function properly.Here's how to fix it, buddy:
📝 Committable suggestion