Add new chart options to PA - #2297
Merged
Merged
Conversation
maggie-li-yd
force-pushed
the
maggie-li-yd/overlapping-histogram-2
branch
from
April 23, 2026 10:03
c2f1ab4 to
c39996d
Compare
maggie-li-yd
force-pushed
the
maggie-li-yd/overlapping-histogram-2
branch
from
April 29, 2026 02:03
f068c28 to
8a96b3c
Compare
maggie-li-yd
force-pushed
the
maggie-li-yd/overlapping-histogram-2
branch
from
April 29, 2026 09:16
8a96b3c to
03fffab
Compare
maggie-li-yd
force-pushed
the
maggie-li-yd/overlapping-histogram-2
branch
from
May 11, 2026 01:29
03fffab to
3a7fe45
Compare
maggie-li-yd
force-pushed
the
maggie-li-yd/overlapping-histogram-2
branch
from
May 11, 2026 01:29
3a7fe45 to
1141171
Compare
maggie-li-yd
marked this pull request as ready for review
May 11, 2026 09:06
maggie-li-yd
requested review from
LSriragavan,
brandantck,
jerome-ng,
khairul-syazwan and
p-hoffmann
as code owners
May 11, 2026 09:06
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds new stacked bar chart display modes (overlapping/overlay variants and kernel density plot), including a “distribution curve” overlay option, and wires the mode state through Vuex + bookmarks and the PA config system.
Changes:
- Introduces bar display mode + overlay state in the chart Vuex module and persists/restores it via bookmarks.
- Adds KDE computation + Plotly layout/trace transforms to support distribution curves and kernel density plot rendering.
- Updates the chart controller UI/layout and portal config to expose feature flags for the new modes.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/ui/apps/vue-mri-ui-lib/src/utils/Constants.ts | Plotly layout tweaks (axis styling). |
| plugins/ui/apps/vue-mri-ui-lib/src/styles/chartController.scss | Refactors axis panel layout to flex groupings for new controls. |
| plugins/ui/apps/vue-mri-ui-lib/src/styles/binning.scss | Adjusts binning button alignment/layout. |
| plugins/ui/apps/vue-mri-ui-lib/src/styles/axisMenuButton.scss | Adds disabled styling for axis menu buttons. |
| plugins/ui/apps/vue-mri-ui-lib/src/store/mutation-types.ts | Adds mutations for bar display mode/overlay/binsize restore. |
| plugins/ui/apps/vue-mri-ui-lib/src/store/modules/chartUtils.ts | Minor bar trace marker styling tweak. |
| plugins/ui/apps/vue-mri-ui-lib/src/store/modules/chart.ts | Adds bar mode/overlay state + mode-switch side effects (axis disable/binsize). |
| plugins/ui/apps/vue-mri-ui-lib/src/store/modules/bookmark.ts | Persists/restores bar mode; normalizes disabled modes to avoid “dirty” bookmarks. |
| plugins/ui/apps/vue-mri-ui-lib/src/lib/models/AxisModel.ts | Adds disabled flag to axis model defaults. |
| plugins/ui/apps/vue-mri-ui-lib/src/lib/i18n.ts | Adds i18n strings for new UI labels (“Chart type”, “Overlay”, etc.). |
| plugins/ui/apps/vue-mri-ui-lib/src/components/XAxisColorButton.vue | Moves positioning responsibility to the controller layout (removes absolute CSS). |
| plugins/ui/apps/vue-mri-ui-lib/src/components/StackBarModes/StackedMode.vue | Adds “stack” mode metadata + layout application function. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/StackBarModes/PartialOverlaySolidMode.vue | Adds overlapping bar mode + optional distribution overlay. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/StackBarModes/OverlayMode.vue | Adds overlapping histogram mode + optional distribution overlay. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/StackBarModes/DistributionCurvesMode.vue | Adds kernel density plot mode + trace replacement logic. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/StackBarModes/modes.ts | Central registry of modes + apply functions. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/StackBarChart.vue | Applies selected mode during Plotly.react; updates legend/y-axis title behavior. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/SortMenuButton.vue | Makes positioning conditional on layout props (works with new flex layout). |
| plugins/ui/apps/vue-mri-ui-lib/src/components/helpers/computeDistributionKDE.ts | New KDE computation + distribution overlay trace/layout mutation helper. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/DropDownMenuItem.vue | Allows custom menu item rendering via slot. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/DropDownMenu.vue | Threads selection + exposes slot for custom item content. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/CohortEntryExitButton.vue | Removes absolute positioning styles (delegates to new layout). |
| plugins/ui/apps/vue-mri-ui-lib/src/components/CohortEntryExit.vue | Wraps entry/exit buttons in layout subgroup container. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/ChartTypeAxisButton.vue | New chart-type dropdown control for stacked bar modes + overlay toggle. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/ChartToolbar.vue | Minor template cleanup (self-closing component tag). |
| plugins/ui/apps/vue-mri-ui-lib/src/components/ChartController.vue | Reorganizes axis controls into groups and adds chart-type control. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/BookmarkItems.vue | Displays stacked bar mode label in bookmark list tooltip line. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/AxisMenuButton.vue | Adds disabled support + makes positioning conditional on layout props. |
| plugins/ui/apps/vue-mri-ui-lib/src/components/tests/StackBarChart.test.ts | Extends unit tests to cover mode application and y-axis title behavior. |
| plugins/ui/apps/portal/src/plugins/mri/PatientAnalyticsConfig/ui5/views/ChartsTab.view.xml | Adds UI5 config checkboxes for enabling the new stacked modes. |
| plugins/ui/apps/portal/src/plugins/mri/PatientAnalyticsConfig/ui5/i18n/text.properties | Adds labels/tooltips for new stacked mode enablement options. |
| plugins/ui/apps/portal/src/plugins/mri/PatientAnalyticsConfig/ui5/i18n/text_en_GB.properties | Same as above for en_GB. |
| plugins/functions/mri-pg-config/src/db/seeds/03_Config.ts | Updates seeded stacked chart options + changes conflict handling strategy. |
| plugins/functions/mri-pg-config/src/db/configs/fhirConfigDuckdb.ts | Adds new stacked chart option defaults (disabled) for FHIR DuckDB config. |
| plugins/functions/mri-pa-config/src/config/formatter.ts | Includes new stacked chart options in formatted config output. |
| plugins/functions/mri-pa-config/src/config/configDefinition.ts | Extends config definition/validation to include new options. |
| plugins/functions/mri-pa-config/cfg/pa/configDefaultValues.ts | Adds default values for new stacked chart options. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 67 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
plugins/ui/apps/vue-mri-ui-lib/src/components/helpers/computeDistributionKDE.ts:1
- yVals
is derived viamap(Number)without filtering non-finite values. If any element converts toNaN,totalWeightbecomesNaN, thetotalWeight === 0guard won’t trigger, and downstream math will propagateNaNdensities. Consider sanitizing weights (e.g., treat non-finite as 0, or return null for that trace) before computingtotalWeight`, mean, variance, and densities.
plugins/ui/apps/vue-mri-ui-lib/src/components/helpers/computeDistributionKDE.ts:1 computeKDE(and the overlay path viaappendDistributionOverlay) introduces non-trivial math and several edge cases (e.g., non-finite weights, empty/1-bin inputs, andxPositionsmismatch). There are unit tests for StackBarChart modes, but there don’t appear to be focused tests covering KDE output stability and edge handling. Adding dedicated tests forcomputeKDE/appendDistributionOverlaywould help prevent regressions in density scaling, NaN handling, and grid sizing behavior.
| this.clearSelectionState() | ||
| return | ||
| } | ||
|
|
Comment on lines
+180
to
191
| if (axisModel && axisModel.props && axisModel.props.filterCardId && axisModel.props.key) { | ||
| if (axisModel.props.binsize !== null && axisModel.props.binsize !== undefined) { | ||
| return `${axisModel.props.binsize}` | ||
| } | ||
| const defaultBinningVal = this.getMriFrontendConfig | ||
| .getAttributeByPath(axisModel.props.attributeId) | ||
| .getDefaultBinSize() | ||
| if (axisModel.props.binsize !== null && axisModel.props.binsize !== undefined) { | ||
| this.cachedBinValue = `${axisModel.props.binsize}` | ||
| return `${axisModel.props.binsize}` | ||
| } else if (defaultBinningVal) { | ||
| this.cachedBinValue = `${defaultBinningVal}` | ||
| if (defaultBinningVal) { | ||
| return `${defaultBinningVal}` | ||
| } else { | ||
| return '' | ||
| } | ||
| } | ||
| return '' |
Comment on lines
+458
to
462
| const current = this.axisModel?.props?.binsize | ||
| if (arg !== current) { | ||
| this.setAxisValue({ | ||
| id: this.dimensionIndex, | ||
| props: { binsize: arg }, |
Comment on lines
+1
to
+15
| <template> | ||
| <div class="stackbar-mode-option" :class="{ active }"> | ||
| <div class="stackbar-mode-option__row" @click="$emit('select')"> | ||
| <span class="stackbar-mode-option__label">Overlapping</span> | ||
| </div> | ||
| <label class="stackbar-mode-option__sub" @click.stop> | ||
| <input | ||
| type="checkbox" | ||
| :checked="showDistributionOverlay" | ||
| @change="$emit('update:showDistributionOverlay', ($event.target as HTMLInputElement).checked)" | ||
| /> | ||
| Distribution Curve | ||
| </label> | ||
| </div> | ||
| </template> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes:
Demo:
Screen.Recording.2026-05-14.at.11.23.35.AM.mov
Merge Checklist
Please cross check this list if additions / modifications needs to be done on top of your core changes and tick them off. Reviewer can as well glance through and help the developer if something is missed out.
developbranch)