Remove communication outages from signal monitor #419
Conversation
utils/helpers.js
Outdated
| @@ -81,6 +81,17 @@ export const useFeatureCounts = ({ geojson, filters }) => | |||
| // eslint-disable-next-line | |||
| }, [geojson]); | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
above is just linting, this is the hook i added
chiaberry
requested review from
johnclary,
Charlie-Henry,
tillyw and
roseeichelmann
|
Another feedback that John gave on the previous PR, was wondering if there was an error with the socrata query, if this "all clear" message would still appear. I tested by mangling the url for the query, and instead of this error appearing, the MapList component fails because the data munging throws errors before the page even renders. |
components/MapList/index.js
Outdated
| @@ -72,6 +75,8 @@ function MapList({ | |||
|
|
|||
| const featureCounts = useFeatureCounts({ geojson, filters }); | |||
|
|
|||
| const totalFeatureCount = useTotalFeatureCount(featureCounts); | |||
There was a problem hiding this comment.
this hook looks great, but in this case i think we might be able to just check if geojson?.features.length > 0, since geojson contains the full feature array before any filters are applied?
There was a problem hiding this comment.
It does contain the full features array which means that it also returns all of the signals with communication issues since those are still in the dataset. For example the length of total feature count right now is 1 but geojson features length is 31.
There was a problem hiding this comment.
ah, i see. should be filtered in the socrata API call if we don't want them?
| @@ -89,13 +94,13 @@ function MapList({ | |||
| maxWidth: MAX_SMALL_SCREEN_WIDTH_PIXELS, | |||
| }); | |||
|
|
|||
| // hids overflow on the document <body> while this component is mounted #noScrollBar | |||
| // hides overflow on the document <body> while this component is mounted #noScrollBar | |||
|
@chiaberry this old dark signals link can be removed |
There was a problem hiding this comment.
this is just linting now
| @@ -62,6 +62,10 @@ export const SIGNAL_STATUS_QUERY = { | |||
| key: "limit", | |||
| value: "99999999", | |||
| }, | |||
| { | |||
| key: "where", | |||
| value: "operation_text!='Communication issue'" | |||
There was a problem hiding this comment.
other option is "operation_state!='3'", but I find this easier to remember
| @@ -62,6 +62,10 @@ export const SIGNAL_STATUS_QUERY = { | |||
| key: "limit", | |||
| value: "99999999", | |||
| }, | |||
| { | |||
| key: "where", | |||
| value: "operation_text!='Communication issue'" | |||
|
Christina let me know this was good to merge! |
Tilly is on vacation so I took over wrapping up this issue cityofaustin/atd-data-tech#1585
https://deploy-preview-419--austin-transportation.netlify.app/signal-monitor?
I reworked Tilly's original code a little by adding a hook to check how many results were returned, and displaying a message based on that instead of the length of the filtered geojson.
Also liked John's suggesting of giving the message some styling, I used the success Alert component
Test by visiting the preview link and unchecking all the check boxes. Confirm the message does not appear.
Locally you can get the success message to appear by changing the conditional to be if there are more than 0 features. Otherwise wait until the signal system is operating normally to see the message :)