[BUG] VZV: Missing "Other" modes crashes on the map #1421

mddilley · 2024-04-01T20:37:44Z

Associated issues

This PR updates the crash map histogram to shows total fatalities and serious injuries instead of total crashes. This matches what we see in the dashboard By Travel Mode visualization, and also the figures shown in the popup that appears when you click on a crash location on the map.

Testing

URL to test:
https://deploy-preview-1421--atd-vzv-staging.netlify.app/viewer/map

Steps to test:

On the crash map, use filters: All, Other mode only, and the default date range
When you hover the time of day histogram bars, you should see the numbers shown in parenthesis in the tooltips adding up to 5. This is the total fatalities and serious injuries of the 3 crashes shown on the map.
Go to the production site and notice that the tooltips numbers total up to 3 crashes instead of the total of fatalities and serious injuries associated with those crashes.

Ship list

Check migrations for any conflicts with latest migrations in master branch
Confirm Hasura role permissions for necessary access
Code reviewed
Product manager approved

…y histogram

…ality and serious injury data

mddilley · 2024-04-01T20:51:38Z

atd-vzv/src/views/map/Map.js

@@ -101,6 +104,7 @@ const Map = () => {
      filters,
      dateRange,
      mapPolygon,
+      mapRequestFields,


I added a parameter to the helper that builds up the Socrata $select parameter so we can control what fields are returned when this function is reused. We were requesting unneeded geo fields here previously.

Thanks Mike. This is a great pattern that we'd do ourselves well adopting everywhere we can. That's a nice thing about graphql, you can't ask for *, you have to ask for columns by name.

mddilley · 2024-04-01T20:53:30Z

atd-vzv/src/views/nav/SideMapTimeOfDayChart.js

Most of this part is renaming variables and adding some comments to hopefully make it easier to read next time - open to changes here. The comment on the important change further down.

mddilley · 2024-04-01T20:56:31Z

atd-vzv/src/views/nav/SideMapTimeOfDayChart.js

+          const crashFatalities = parseInt(crash.death_cnt);
+          const seriousInjuries = parseInt(crash.sus_serious_injry_cnt);
+          const isCrashInTimeWindow =
+            crashHour >= timeWindow[0] && crashHour <= timeWindow[1];
+
+          if (isCrashInTimeWindow) {
+            accumulator[i] = accumulator[i] + crashFatalities + seriousInjuries;
+          }


This is the fix. We used to add up the total number of crash records per time of day window that returned using the map filters for severity, mode, and date range.

Now, we add up the number of fatalities and serious injuries for each crash record.

roseeichelmann

The code looks great to me and this works well, but i do find this change a little confusing. I think of the crash map with all the dots as showing data on crashes, not people, and when you filter to injuries, you are showing all the crashes with injuries. And the total number in the histogram should be how many crashes happened within that time frame, at least thats how i expect it to work. It seems weird to be going from crashes being shown on the map to number of people being shown in the histogram. If we do go with this change i think we need a tooltip to specify what is the true meaning of the numbers in the histogram ie. its the number of crashes or the number of people.

Also this whole thing is making me realize that the meaning of "All" both in the map and summary pages is kind of ambiguous. I could see users thinking All means show all crashes or all people despite level of injury, but the meaning is really "show all fatality or serious injury crashes". Just some food for thought!

For example, maybe having just two buttons on the map for Fatalities and Serious Injuries and you can toggle both or just one and do away with the All button would make it more apparent that we are never displaying all the crashes that happen in Austin. (obvi a completely diff issue and out of scope for this)

mddilley · 2024-04-02T20:24:20Z

@roseeichelmann thanks!, and I agree that it is worth thinking about ways to indicate exactly what is displayed in this viz. Getting oriented on what data was coming into the component and what is displayed wasn't super straightforward when i was working on this.

For the All button, I see exactly what you are saying. It does kind of imply "All Crashes with no filters" (neither) instead of "All crashes that have a fatality or serious injury" (both). I remembered that we changed this once before and found this PR that shows that we once had just the two buttons but we changed it because it was confusing. Not a lot of crumbs as to why it was considered confusing so I think it could use another look. That was a long time ago now!

Curious what @johnclary thinks too. Happy to keep iterating on this one!

chiaberry

Cosign on what Rose said -- the code looks great, I appreciate all the work you put in to cleaning up the code and refining the request.

I do think its a little confusing to see three dots on the map, but then 5 in the histogram. But also think it should match what is on the charts on the VZV, so it makes sense to see 5 in the histogram.

Text saying the histogram shows people and the map shows incidents would be helpful. I was also thinking if the dots were bigger if there were more than 1 person injured or killed, if the size of the dot corresponded to people.

frankhereford

I agree with the sentiment that the data being displayed in the histogram isn't clear. Additionally, the calendar icon in it is also misplaced or unclear in my opinion.

I'd suggest we can help reduce this confusion by striking that calendar icon and replacing it with a header for the histogram that says "Injuries."

However - for what this PR set out to do, it's 💯! Thank you Mike!

frankhereford · 2024-07-08T15:21:54Z

atd-vzv/src/views/map/Map.js

@@ -101,6 +104,7 @@ const Map = () => {
      filters,
      dateRange,
      mapPolygon,
+      mapRequestFields,


Thanks Mike. This is a great pattern that we'd do ourselves well adopting everywhere we can. That's a nice thing about graphql, you can't ask for *, you have to ask for columns by name.

mddilley added 4 commits April 1, 2024 12:06

Docs filters and requests info

6b4eb11

Refactor createMapDataUrl to change up the columns for the time of da…

7f5206e

…y histogram

Trim fields coming into Time of Day component; calc with incoming fat…

ab5f45a

…ality and serious injury data

Remove comments and update variable names

fca8469

mddilley added the WIP Work in progress label Apr 1, 2024

mddilley commented Apr 1, 2024

View reviewed changes

mddilley removed the WIP Work in progress label Apr 1, 2024

mddilley requested review from frankhereford, chiaberry, johnclary, tillyw and roseeichelmann April 1, 2024 21:01

roseeichelmann approved these changes Apr 2, 2024

View reviewed changes

chiaberry approved these changes Apr 29, 2024

View reviewed changes

frankhereford approved these changes Jul 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] VZV: Missing "Other" modes crashes on the map #1421

[BUG] VZV: Missing "Other" modes crashes on the map #1421

mddilley commented Apr 1, 2024 •

edited

Loading

mddilley Apr 1, 2024

frankhereford Jul 8, 2024

mddilley Apr 1, 2024 •

edited

Loading

mddilley Apr 1, 2024 •

edited

Loading

roseeichelmann left a comment •

edited

Loading

mddilley commented Apr 2, 2024

chiaberry left a comment

frankhereford left a comment

frankhereford Jul 8, 2024

[BUG] VZV: Missing "Other" modes crashes on the map #1421

Are you sure you want to change the base?

[BUG] VZV: Missing "Other" modes crashes on the map #1421

Conversation

mddilley commented Apr 1, 2024 • edited Loading

Associated issues

Testing

Ship list

mddilley Apr 1, 2024

Choose a reason for hiding this comment

frankhereford Jul 8, 2024

Choose a reason for hiding this comment

mddilley Apr 1, 2024 • edited Loading

Choose a reason for hiding this comment

mddilley Apr 1, 2024 • edited Loading

Choose a reason for hiding this comment

roseeichelmann left a comment • edited Loading

Choose a reason for hiding this comment

mddilley commented Apr 2, 2024

chiaberry left a comment

Choose a reason for hiding this comment

frankhereford left a comment

Choose a reason for hiding this comment

frankhereford Jul 8, 2024

Choose a reason for hiding this comment

mddilley commented Apr 1, 2024 •

edited

Loading

mddilley Apr 1, 2024 •

edited

Loading

mddilley Apr 1, 2024 •

edited

Loading

roseeichelmann left a comment •

edited

Loading