removes comm outages from signal monitor #418
Conversation
tillyw
requested review from
Charlie-Henry,
chiaberry,
ChristinaTremel,
amenity,
johnclary,
mddilley,
roseeichelmann and
frankhereford
|
sorry I wasn't sure which devs to tag for review so I tagged everybody but I think just one or two dev reviews would be ok! |
|
I don't have a great workaround for testing other than maybe us setting up a test socrata dataset? Here's the dataset it uses currently: https://data.austintexas.gov/Transportation-and-Mobility/Traffic-Signals-Status/5zpr-dehc/about_data |
|
Here's the link of the deploy preview: https://deploy-preview-418--austin-transportation.netlify.app/signal-monitor I cant decide if I think it would be better to include the search and the filters even if all the signals are operating normally, just to see the options dark, scheduled flash and unscheduled flash.
For example showing this with 0 next to the unscheduled flash as well, with the "all is operating normally" message. What do you think @ChristinaTremel ? |
thanks for adding the deploy preview link! i added it to the description. i wrestled with the same question @chiaberry! i ended up hiding the search in these cases because it felt odd to me to have a search bar that wouldn't yield anything. i agree though it would be nice to have an idea of what this dashboard is tracking even if there aren't any current instances of it. happy to rework if y'all have a preference! |
|
I personally like the idea of including the search and filters with (0) next to them and also have the message! Because if it's blank, I think it'd be nice for the user to see what would show on the map! |
|
I happened to check now and I don't see any signals on flash or dark but I also don't see the "system operating normally" text like in your screenshot @tillyw 
|
@Charlie-Henry thanks for catching that! pushing a fix now |
|
@Charlie-Henry the message should be displaying now! |
thanks @ChristinaTremel ! i've updated the deploy preview. please let me know if it looks good to you! |
|
@tillyw it looks perfect! 🥹 |
There was a problem hiding this comment.
the code changes to remove the settings for the comm outage look good 🙏 the other enhancement might need to go back into scoping?
@ChristinaTremel just to confirm: we do want to keep the comm outage signals in the data portal dataset, or should those be removed as well?
components/MapList/index.js
Outdated
| </div> : | ||
| <div className="p-3">Signal system operating normally.</div> | ||
| } | ||
| </div> |
There was a problem hiding this comment.
I don't think this approach is going to work, because this component is used by other dashboards, e.g. the cameras dashboard:
The other problem is that this message displays if you search the list when there is a signal outage:
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I know you asked John, but my opinion is to rework this
There was a problem hiding this comment.
i defer to Chia and Christina! 👍 🙏
i agree 💯 |
Completely missed this a couple weeks ago! I asked Allyson and she would prefer to keep the comm outage signals in the Traffic Signals Status socrata dataset. |
There was a problem hiding this comment.
Nothing specific, but I wanted to make sure this doesn't go in by accident since it's being covered now in PR #419. Thanks!!
fixes cityofaustin/atd-data-tech#1585
deploy preview for testing
This pr removes signals that have communication outages from the signal monitor. If all signals are operating properly visitors will see this screen (please ignore that fact that there are a couple of data points on the map):
One suggestion I would make is maybe we should consider renaming the signal monitor? since it only displays signals that are not functioning properly maybe we should call it 'Signal Outage Monitor' or something like that (I'm not sure what the best term would be). That way it will be less confusing to come across it in cases when everything is functioning properly and see a blank map (which would be a little confusing to me).
As far as testing, I'm not sure how best to do that. @Charlie-Henry is it possible to temporarily alter or clear the data being fed into the dashboard so our signal folks can test? Devs can test by setting
searchedGeojsontonullbut that's not the most elegant workaround!