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

Jump to bottom

removes comm outages from signal monitor #418

Merged
merged 4 commits into from
Jun 21, 2024
Merged

removes comm outages from signal monitor #418

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c0b4e2
removes comm outages from signal monitor
tillyw May 23, 2024
70ae8fa
fixed ternary
tillyw May 24, 2024
70148a1
display search
tillyw May 24, 2024
ba60253
remove conditional
tillyw Jun 12, 2024
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
13 changes: 7 additions & 6 deletions components/MapList/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,8 @@ function MapList({
{/* note the use of d-none (display: none) to hide elements. this avoids
laborious re-renders */}
<div
className={`d-flex flex-column ${
(!layout.listSearch && "d-none") || ""
}`}
className={`d-flex flex-column ${(!layout.listSearch && "d-none") || ""
}`}
style={{ overflowY: "hidden" }}
>
<ListSearch
Expand All @@ -177,15 +176,17 @@ function MapList({
hasSelectedFeature={!!selectedFeature}
featureCounts={featureCounts}
/>
<div className="px-3" style={{ overflowY: "scroll" }}>
{searchedGeojson.features.length ? <div className="px-3" style={{ overflowY: "scroll" }}>
<List
geojson={searchedGeojson}
mapRef={mapRef}
setSelectedFeature={setSelectedFeature}
ListItemContent={ListItemContent}
/>
</div>
</div>
</div> :
<div className="p-3">Signal system operating normally.</div>
}
</div>
Copy link
Member

@johnclary johnclary May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think this approach is going to work, because this component is used by other dashboards, e.g. the cameras dashboard:
Screenshot 2024-05-29 at 10 59 37 AM

The other problem is that this message displays if you search the list when there is a signal outage:
Screenshot 2024-05-29 at 11 00 26 AM

I guess one more problem is that i think this "everything is ok" message will display if there is an HTTP error returned from the Socrata query—e.g. if there is a socrata outage. just looking very quickly at the code, i'm not sure we currently display an error message under any circumstances.

From a styling perspective, it would be nice if the message rendered inside a list item with the same padding as other list items. Maybe using a green alert banner or something besides just plain text?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i did not think to check if the component is used by other dashboards! thank you for pointing that out. and on your second and third point you're totally right i need to use a more robust way of checking why there might not be data returned from socrata. and i can definitely add styling to the message. @johnclary should we break these up into two issues (one to remove comm outages and one to display a message stating everything is working) or should i rework this?

Copy link
Member

Choose a reason for hiding this comment

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

I know you asked John, but my opinion is to rework this

Copy link
Member

Choose a reason for hiding this comment

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

i defer to Chia and Christina! 👍 🙏

{/* page info content */}
{layout.info && isSmallScreen && (
<div className="px-3">
Expand Down
14 changes: 1 addition & 13 deletions page-settings/signal-monitor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { FaExclamationTriangle, FaClock, FaPhone, FaTimes } from "react-icons/fa";
import { FaExclamationTriangle, FaClock, FaTimes } from "react-icons/fa";

const COLORS = {
red: "#c41213",
orange: "#f29900",
blue: "#377eb8",
black: "#000000",
};

Expand All @@ -26,15 +25,6 @@ const OPERATION_STATES = [
checked: true,
icon: FaClock,
},
{
key: "comm_outage",
value: "3",
label: "Comm. issue",
color: COLORS.blue,
featureProp: "operation_state",
checked: true,
icon: FaPhone,
},
{
key: "dark_signal",
value: "4",
Expand Down Expand Up @@ -62,8 +52,6 @@ export const LAYER_STYLES = {
OPERATION_STATES[0].color,
"3",
OPERATION_STATES[2].color,
"4",
OPERATION_STATES[3].color,
// fallback
"#ccc",
],
Expand Down