Recidiviz / justice-counts-components

A set of React components powering a web app for exploring centralized, standardized metrics on the performance of justice systems across the United States
GNU General Public License v3.0
1 stars 0 forks source link

Fix minor issues in tooltip #57

Closed colincadams closed 3 years ago

colincadams commented 3 years ago

Description of the change

For #55 there were a few underlying issues:

  1. The dateReported is constructed with new Date("2020-11-30") which uses no timezone offset when constructing it. Then we printed it using the local timezone which would be "2020-11-29".
  2. We use month indices throughout (matching what appear to be javascript best practices) which means that 0 is a valid month. We were using an implicit boolean check on month instead of comparing it to null, which meant the % change part of the hint was omitted, even though it was the important part of the hint in this case.
  3. Data: Maine had a few recent monthly admissions points, and then prior to that some annual admissions data. All of them were recorded as monthly, so the annual data slipped through the calculations and was treated as monthly causing the apparent 95+% decrease shown here (and is the reason for the comparison of more than a year back to the prior january).

While I was trouncing around to try to figure out what was happening I figured I would just take a stab at fixing the first two to get more familiar with what is happening in JS land. If this feels reasonable, great, if not feel free to abandon this PR and take a different approach.

I added a test for the (2), but didn't see an easy way to mock the timezone to test (1). If there is one let me know! I also fixed the data entry error for (3) separately.

Type of change

Related issues

Closes #55

Checklists

Development

These boxes should be checked by the submitter prior to merging:

Code review

These boxes should be checked by reviewers prior to merging:

jessex commented 3 years ago

@colincadams for testing with static times or timezones, have you looked at the timekeeper package? We use it in some of our other frontend projects via import tk from "timekeeper";

Wmaileq commented 3 years ago

I've noticed tests failing in generateHint.js and decided to unify the date format to do not use the Date js class at all, as we use only UTC dates and should not depend on timezones.