Skip to content

Conversation

skarya22
Copy link
Contributor

@skarya22 skarya22 commented Jul 30, 2025

Brief summary of changes

  • Project and Visit filters were broken, they now appear
  • Added a Candidate Date Registered filter
  • Made filters hidden when reloading with fresh data. It was confusing before on datasets with more data than raisinbread as it would show old data until the new stuff was loaded. This was not shown in raisinbread because it loaded so fast.
  • Add ethnicity data to raisinbread as well as correct DoBs compared to their Date_registered
  • Add subtitles for Panel views in the dashboard for total counts
  • Added value to Pie chart tooltip from EEGNET
  • Added Age Distribution chart from EEGNET
  • Reorganized charts to be in better sections, can compare to live v27 on demo.loris.ca
image image image

Testing instructions (if applicable)

  1. Test every filter and confirm they show the desired behaviour
  2. Add a new candidate and confirm that the date filter works
  3. Create an account with less project/ site permissions but with Access Profile permission
  4. See that the numbers are less in the subtitles and charts do not contain information for non user projects or sites

Link(s) to related issue(s)

@skarya22 skarya22 added the Project: CCNA Issue or PR related to the CCNA project label Jul 30, 2025
@github-actions github-actions bot added Language: PHP PR or issue that update PHP code Language: Javascript PR or issue that update Javascript code Module: statistics PR or issue related to statistics module and removed Project: CCNA Issue or PR related to the CCNA project labels Jul 30, 2025
@github-actions github-actions bot added Language: SQL PR or issue that update SQL code RaisinBread PR or issue introducing/requiring improvements to the Raidinbread dataset labels Jul 31, 2025
@skarya22 skarya22 changed the title [statistics] Cleanup filters and add Registration Date [statistics] dashboard filters + subtitles Jul 31, 2025
@skarya22 skarya22 changed the title [statistics] dashboard filters + subtitles [statistics] dashboard filter improvements + subtitles Jul 31, 2025
@skarya22 skarya22 added the Blocking PR should be prioritized because it is blocking the progress of another task label Aug 7, 2025
Copy link
Contributor

@jeffersoncasimir jeffersoncasimir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "Date Registered" filter appears broken.

When selecting a date, the query returns 500, with the following server-side error:
Column not found: 1054 Unknown column 'c.Date_registered' in 'where clause'

I have confirmed that my candidate table indeed contains this column.

Also, when selecting a first filter, a seemingly inconsequential console error, that calls itself a warning, appears:

Warning: Cannot update a component (`StudyProgression`) while rendering a different component (`QueryChartForm`). To locate the bad setState() call inside `QueryChartForm`, follow the stack trace as described in https://reactjs.org/link/setstate-in-render Error Component Stack
    at QueryChartForm (queryChartForm.js:13:57)

Seems to otherwise work as expected.

@skarya22
Copy link
Contributor Author

skarya22 commented Aug 14, 2025

@jeffersoncasimir apologies for commit spam! I added the age distribution chart to this PR, and also cleaned up the module. There was a lot of dead frontend code. I think this will require a re-review unfortunately. It is currently failing tests but it is unrelated, so ready for review :)

I updated the PR summary, but here are the new ones:

  • Added Age Distribution chart from EEGNET
  • Reorganized charts to be in better sections, can compare to live v27 on demo.loris.ca
  • Changed the panel subtitle to not look like a button

Copy link
Contributor

@jeffersoncasimir jeffersoncasimir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Features otherwise work nicely.

When hovering on a timepoint with no data, the custom tooltip says undefined instead of showing 0 for all categories.

Screenshot:
Screenshot 2025-08-18 at 10 08 40 AM

These features have highlighted a problem with the RB data. Maybe this is for a new issue, or maybe something can be done to avoid the scenario where negative ages are shown:

Screenshot 2025-08-18 at 10 05 42 AM

Also, please make sure all spline charts have this: spline: {interpolation: {type: 'monotone'}},. It makes it so the splines don't overshoot the number they are targetting

@skarya22
Copy link
Contributor Author

Features otherwise work nicely.

When hovering on a timepoint with no data, the custom tooltip says undefined instead of showing 0 for all categories.

Screenshot: Screenshot 2025-08-18 at 10 08 40 AM

These features have highlighted a problem with the RB data. Maybe this is for a new issue, or maybe something can be done to avoid the scenario where negative ages are shown:

Screenshot 2025-08-18 at 10 05 42 AM Also, please make sure all spline charts have this: `spline: {interpolation: {type: 'monotone'}},`. It makes it so the splines don't overshoot the number they are targetting

Hi Jefferson,

I'll look into the first one --- but the second is resolved by this pr if you refresh rb

@skarya22 skarya22 self-assigned this Aug 18, 2025
@skarya22 skarya22 added the State: Needs work PR awaiting additional work by the author to proceed label Aug 18, 2025
@christinerogers
Copy link
Contributor

Great work @skarya22 and @jeffersoncasimir
Just at a glance -- Is it feasible to include the % plot display (toggle) on the Age at recruitment spline chart?
That was the most valuable display for this data, is what we heard from researchers from EEGNet.
They found it more useful to see what % of the cohort was (e.g.) 21 years old, not necessarily the exact number. Let's include both if it's not a big challenge.

@christinerogers
Copy link
Contributor

christinerogers commented Aug 27, 2025

@skarya22 could you mention in your PR description that this will be followed by Jefferson's PR with more EEGNet dashboard features -- e.g Examples missing from this one : my last comment plus

  • the project total in the title bar is smaller and less readable than the data in the plots. Alan wanted this to be a headline, please take a stab at making this more visible - e.g the white background did the trick on EEGNet, though its aesthetic was too clunky otherwise.
  • Size (GB/TB) of the data collection was also a key headline / feature. Could you address in the PR description why/when this will / won't be included in the visible plots? @skarya22 thanks

@skarya22
Copy link
Contributor Author

skarya22 commented Sep 3, 2025

@skarya22 could you mention in your PR description that this will be followed by Jefferson's PR with more EEGNet dashboard features -- e.g Examples missing from this one : my last comment plus

  • the project total in the title bar is smaller and less readable than the data in the plots. Alan wanted this to be a headline, please take a stab at making this more visible - e.g the white background did the trick on EEGNet, though its aesthetic was too clunky otherwise.
  • Size (GB/TB) of the data collection was also a key headline / feature. Could you address in the PR description why/when this will / won't be included in the visible plots? @skarya22 thanks

@christinerogers There are other issues for the other plots such as the size (GB/TB) that you mentioned which is assigned to @jeffersoncasimir #9944. Didn't think it was a good idea to include it all in one PR as this PR is already quite large.

I am honestly not too sure how to make the subtitle better, as we had discussed the white background would make it look like a button which is not very good in terms of UX design...

@christinerogers
Copy link
Contributor

thanks Saagar-- that issue this pr shoulda been crosslinked to relate them, thanks for doing it just now

@skarya22
Copy link
Contributor Author

skarya22 commented Sep 3, 2025

@christinerogers I added percentage to the tooltip instead of as a toggle as the actual chart remains unchanged when toggled since percentage and count would be visible the same chart (except the y axis).

Let me know if this is okay. I also made the subtitle the same size as the title, if that helps with the clarity
image

I hadn't linked this PR and the issue since the issue is for a different plot.

@skarya22 skarya22 removed the State: Needs work PR awaiting additional work by the author to proceed label Sep 3, 2025
@skarya22 skarya22 removed their assignment Sep 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language: Javascript PR or issue that update Javascript code Language: PHP PR or issue that update PHP code Language: SQL PR or issue that update SQL code Module: statistics PR or issue related to statistics module RaisinBread PR or issue introducing/requiring improvements to the Raidinbread dataset
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[dashboard] Add Age distribution chart
3 participants