Visual treatment for small numbers #532

nasaownsky · 2022-04-11T18:16:36Z

Description of the change

Hi, @hobuobi! Glad to work with you again.

I implemented some of the expected behavior orienting on mocks, and it looks like this:

What do you think?

Also, I seen in the mocks such styling for line rate charts, but I don't quite understand how to filter values in them. In above examples we has values, that we can simply compare with threshold level, but line rate charts contains something like 101 of 246 (31%). I assume that I should use upper bound for comparing with threshold? Anyway, if you could give me more information, I would appreciate that.

Type of change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Configuration change (adjusts configuration to achieve some end related to functionality, development, performance, or security)

Related issues

Closes #480

Checklists

Development

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

Manual testing against realistic data has been performed locally

Code review

These boxes should be checked by reviewers prior to merging:

This pull request has a descriptive title and information useful to a reviewer
This pull request has been moved out of a Draft state, has no "Work In Progress" label, and has assigned reviewers
Potential security implications or infrastructural changes have been considered, if relevant

coveralls · 2022-04-11T18:21:40Z

Pull Request Test Coverage Report for Build 2447408917

25 of 27 (92.59%) changed or added relevant lines in 7 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.01%) to 83.678%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
spotlight-client/src/charts/BubbleChart.tsx	4	5	80.0%
spotlight-client/src/charts/ProportionalBar.tsx	4	5	80.0%

Totals
Change from base Build 2430417634:	-0.01%
Covered Lines:	2084
Relevant Lines:	2355

💛 - Coveralls

hobuobi · 2022-04-12T18:43:22Z

Great to be working with you again too! :)
A few quick comments on what you've done so far:

Visual treatment generally looks wonderful!
This was clear to me after looking at your implementation, but it would probably make more sense to apply the visual treatment to the entire visualization. For example, if the total number of events, people, etc. is less than our threshold, we would apply the treatment to all circles, bars, etc. in that visualization.
For the line rate charts, we should also compare the total to the breakdown. For example, in the North Dakota "How many people end up back in prison?" chart, when breaking down the 2011 cohort by race, we should see the Other, Black, and Hispanic lines with the visual treatment because the total people they each represent is less than 100.

We should also update the copy that appears below each chart. Instead of saying:

Please always take note of the number of people associated with each proportion presented here; in cases where the counts are especially low, the proportion may not be statistically significant and therefore not indicative of long-term trends.

We should say:

Please always take note of the number of people associated with each proportion presented here; in cases where the counts are especially low, the proportion may not be statistically significant and therefore not indicative of long-term trends. Visualizations with fewer than 100 people will appear hashed, dotted, or with another alternative visual treatment.

terryttsai

Hey, nice to connect with you nasaownsky. I'm new to this codebase but I will be getting familiar with it as time goes on. So far I just have a couple comments:

terryttsai · 2022-04-13T00:14:56Z

spotlight-client/src/charts/useCreateHatchDefs.tsx

+export function useCreateHatchDefs(
+  data: CategoricalChartRecord[],
+  highlighted: ItemToHighlight | undefined,
+  type?: "rate"


What is type used for?

Hey! Nice to connect with you too!
This is redundant part, I changed the approach here. Now fixed

terryttsai · 2022-04-13T00:16:42Z

Is there some sort of legend or explanation as to what the diagonal lines mean?

terryttsai · 2022-04-13T00:17:27Z

While testing this branch, I noticed that a bar with diagonal lines don't have an animation transition from faded to not faded when the mouse is no longer hovering over it?

terryttsai · 2022-04-13T00:23:48Z

spotlight-client/src/charts/utils.ts

@@ -23,6 +23,11 @@ import { isItemToHighlight, ItemToHighlight } from "./types";

 const FADE_AMOUNT = 0.45;

+export function generateHatchFill(id: string): string {
+  const cleanId = id.replace(/[^\w\d]/g, "");
+  return `url(#${cleanId})`;


I'm new to this, what does url(#...) do?

It's pointing to defs with pattern by id

nasaownsky · 2022-04-15T14:59:09Z

@hobuobi So I made a lot of changes here, now it's working as expected I suppose.

@terryttsai Yeah, now there is an explanation in notes. As for the animations - I spent a lot of time trying to figure out why they aren't working, but so far has no success in it. The hatched bars do not animate when hovering over them. What do you think about that @hobuobi?

hobuobi

All looks good! Two things that we should follow-up on at a later date:

Reimplementing the faded transitions for the hashed treatment
Adding a visual legend for the hashed treatment, rather than the caption

terryttsai · 2022-05-09T20:58:49Z

spotlight-client/src/charts/utils.ts


 const FADE_AMOUNT = 0.45;

+export function isSmallData(data: CommonDataPoint[]): boolean {


Checking out your latest changes, I'm no longer seeing any special treatment for small numbers. Looks like the issue is this sumBy here, we're now summing up all the values in a set of data, rather than doing what we were doing before which is comparing a single point of data's value with d.value < STATISTIC_THRESHOLD.

I pulled up all latest changes and the hashed treatment still working. At least in demo mode. I merged changes, try it now, please.

I don't see the hashed treatment working for me, where should I expect to see it? Here's a video of what I'm seeing: https://www.loom.com/share/04848c562c3849f9a523d86d1888e991

I expect in us-nd/narratives/prison to see hashed treatment for numbers under 100

If I change the function isSmallData to

export function isSmallData(value: number): boolean { return value < STATISTIC_THRESHOLD; }

and pass in d.value whenever we call isSmallData then I see this:

which is what I expect.

Oh, this was in Humphrey comment from 12 Apr:

This was clear to me after looking at your implementation, but it would probably make more sense to apply the visual treatment to the entire visualization. For example, if the total number of events, people, etc. is less than our threshold, we would apply the treatment to all circles, bars, etc. in that visualization.

So, in order to see the treatment you should for example in us-nd/narratives/prison/2 select Bismark Transition Center filter instead of All Facilities.

Got it, thanks for clarifying! I missed that comment, I was relying on what was written in the PR description, could you update it? LGTM!

terryttsai

Had an issue trying to see hashed treatment for small numbers with your latest changes.

…into nasaownsky/480-small-numbers

nasaownsky · 2022-05-27T10:51:44Z

spotlight-client/src/VizControls/VizControls.tsx

+        {smallData && (
+          <TooltipTrigger maxWidth={232} contents={tenant?.smallDataDisclaimer}>
+            <Icon kind={IconSVG.Info} width={16} height={16} />
+          </TooltipTrigger>
+        )}


Hey, @macfarlandian! Is there any way to style the Tooltip when using TooltipTrigger from design-system?

not the tooltip itself, no, only the contents ... it is a known issue that unfortunately the design of tooltip contents in Spotlight does not really work with the tooltip from the design system. Hasn't been much of an issue in Spotlight thus far because there were no tooltips outside of the charts, but if we are adding that use case here, maybe it's better to add this feature to the design system than to reimplement the component. I know @terryttsai has looked into this recently so curious what he thinks

cc @terryttsai

I remember that the public dashboard's tooltip is designed differently than the one in design-system. Originally I had a story to move our tooltip to the one in design-system but ran into the issue in that I couldn't style it to match our design. Might make sure to modify the design-system's tooltip so that it can be styled properly. Either that or continue to use our own tooltip in this codebase...

@terryttsai @macfarlandian I think it would be nice if we can properly style the Tooltip from the design-system. Could you create an Issue for that?

done! Recidiviz/web-libraries#105

…into nasaownsky/480-small-numbers

nasaownsky added 2 commits April 11, 2022 20:44

Added visual treatment for small numbers

eeba611

Test fixes

eb294e8

nasaownsky requested review from colincadams and hobuobi April 11, 2022 18:16

lilidworkin requested review from lilidworkin and terryttsai April 12, 2022 16:50

terryttsai reviewed Apr 13, 2022

View reviewed changes

nasaownsky added 2 commits April 15, 2022 17:49

Fixes

8e1c2c3

Test fixes

54d9525

hobuobi approved these changes Apr 25, 2022

View reviewed changes

terryttsai reviewed May 9, 2022

View reviewed changes

terryttsai suggested changes May 9, 2022

View reviewed changes

nasaownsky added 2 commits May 24, 2022 21:22

Merge branch 'main' of https://github.com/Recidiviz/public-dashboard …

5341753

…into nasaownsky/480-small-numbers

Added visual legend instead of caption

879c76c

nasaownsky commented May 27, 2022

View reviewed changes

macfarlandian mentioned this pull request Jun 2, 2022

Enable tooltip restyling in TooltipTrigger Recidiviz/web-libraries#105

Open

Merge branch 'main' of https://github.com/Recidiviz/public-dashboard …

b5ee2e1

…into nasaownsky/480-small-numbers

terryttsai approved these changes Jun 10, 2022

View reviewed changes

terryttsai merged commit 61fdeb3 into main Jun 10, 2022

terryttsai deleted the nasaownsky/480-small-numbers branch June 10, 2022 21:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Visual treatment for small numbers #532

Visual treatment for small numbers #532

nasaownsky commented Apr 11, 2022

coveralls commented Apr 11, 2022 •

edited

hobuobi commented Apr 12, 2022

terryttsai left a comment

terryttsai Apr 13, 2022 •

edited

nasaownsky Apr 15, 2022

terryttsai commented Apr 13, 2022

terryttsai commented Apr 13, 2022

terryttsai Apr 13, 2022 •

edited

nasaownsky Apr 15, 2022

nasaownsky commented Apr 15, 2022

hobuobi left a comment

terryttsai May 9, 2022 •

edited

nasaownsky May 24, 2022 •

edited

terryttsai Jun 9, 2022

terryttsai Jun 9, 2022

terryttsai Jun 9, 2022

nasaownsky Jun 9, 2022

terryttsai Jun 10, 2022

terryttsai left a comment •

edited

nasaownsky May 27, 2022

macfarlandian May 27, 2022

nasaownsky Jun 1, 2022

terryttsai Jun 1, 2022

nasaownsky Jun 2, 2022

macfarlandian Jun 2, 2022


		const FADE_AMOUNT = 0.45;

		export function isSmallData(data: CommonDataPoint[]): boolean {

Visual treatment for small numbers #532

Visual treatment for small numbers #532

Conversation

nasaownsky commented Apr 11, 2022

Description of the change

Type of change

Related issues

Checklists

Development

Code review

coveralls commented Apr 11, 2022 • edited

Pull Request Test Coverage Report for Build 2447408917

💛 - Coveralls

hobuobi commented Apr 12, 2022

terryttsai left a comment

Choose a reason for hiding this comment

terryttsai Apr 13, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

terryttsai commented Apr 13, 2022

terryttsai commented Apr 13, 2022

terryttsai Apr 13, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nasaownsky commented Apr 15, 2022

hobuobi left a comment

Choose a reason for hiding this comment

terryttsai May 9, 2022 • edited

Choose a reason for hiding this comment

nasaownsky May 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

terryttsai left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Apr 11, 2022 •

edited

terryttsai Apr 13, 2022 •

edited

terryttsai Apr 13, 2022 •

edited

terryttsai May 9, 2022 •

edited

nasaownsky May 24, 2022 •

edited

terryttsai left a comment •

edited