[DC-710]Add mocked visualizations and total cohort count #5114
Conversation
src/dataset-builder/CohortEditor.ts
Outdated
| setTimeout(() => { | ||
| cohortAgeData.series = generateAgeSeries(3, 90); | ||
| setCohortAges(cohortAgeData); | ||
| }, 2000); |
There was a problem hiding this comment.
I'm not sure this timeout is doing everything I want it to do. It does stop a double re-render (like without it, the graphs re-render when the async count method is called and again when it is returned), but I wanted to add a timeout so that it would return at a similar time as the async count call and it isn't doing that even if I increase the time. What am I missing here?
There was a problem hiding this comment.
So, I wouldn't expect you to need both the timeout and the useEffect subscribed to snapshotRequestParticipantCount. This effect should be called whenever snapshotRequestParticipantCount is updated. The trick might be that snapshotRequestParticipantCount is a useLoadedData hook, so you probably need to check the status of useLoadedData and only display/update when it is 'Ready'
src/dataset-builder/CohortEditor.ts
Outdated
| setTimeout(() => { | ||
| cohortAgeData.series = generateAgeSeries(3, 90); | ||
| setCohortAges(cohortAgeData); | ||
| }, 2000); |
There was a problem hiding this comment.
So, I wouldn't expect you to need both the timeout and the useEffect subscribed to snapshotRequestParticipantCount. This effect should be called whenever snapshotRequestParticipantCount is updated. The trick might be that snapshotRequestParticipantCount is a useLoadedData hook, so you probably need to check the status of useLoadedData and only display/update when it is 'Ready'
rjohanek
changed the title
rjohanek
changed the title
|
| { name: 'Pacific Islander', data: [] }, | ||
| ]; | ||
| // for each of the three gender identity totals, | ||
| const genderTotals = generateRandomNumbersThatAddUpTo(100, 3); |
There was a problem hiding this comment.
Just as a note - and its dummy data so not a big deal, but I think we would want it to add up to the number of participants, not to 100?
That said, I think trying to get the dummy data perfect is not worth the effort, and just letting it render charts seems fine to me.
There was a problem hiding this comment.
the graph is based on the overall percentage of participants not the number of participants. that's how it was in the mock
| showSeriesName: boolean; | ||
| } | ||
|
|
||
| export function generateCohortAgeData(series) { |
There was a problem hiding this comment.
This function doesn't seem like test data to me, since its the chart definition. I'd move this into the component.
| generateCohortDemographicData(defaultCohortDemographicSeries) | ||
| ); | ||
| const countStatus = snapshotRequestParticipantCount.status; | ||
| function chartOptions(cohortDemographics: CohortDemographics) { |
There was a problem hiding this comment.
The charts are meaty enough they may be worth an intermediate component, so that we can isolate the chart configuration. What are your thoughts on that?
There was a problem hiding this comment.
yeah I think that's a good idea
Jira Ticket: https://broadworkbench.atlassian.net/browse/DC-710
Summary of changes:
a. adds code to randomly generate numbers
b. adds graph data to state to trigger re-render
c. blocks rendering of graph on return of count endpoint
Why
demographic visualizations are helpful to researchers building a cohort
Testing strategy
Unit tests of helper functions
Test that count appears on page
Test that graphs appear on page
Visual Aids
Screen.Recording.2024-10-04.at.2.03.00.PM.mov