feat: group measurements by study #4617
Conversation
…ts-service-filtering
…ts-service-filtering
✅ Deploy Preview for ohif-platform-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
pedrokohler
changed the title
pedrokohler
changed the title
wayfarer3130
changed the base branch from
master
to
feat/measurements-service-filtering
…/Viewers into feat/measurements-service-filtering
| @@ -53,6 +53,7 @@ function PanelMeasurementTableTracking({ | |||
| extensionManager={extensionManager} | |||
| commandsManager={commandsManager} | |||
| measurementFilters={measurementFilters} | |||
| title="Measurements" | |||
There was a problem hiding this comment.
The grouping function should get passed in here to PanelMeasurements, and should be a grouping function that groups things into "Tracked Measurements" with a group header being the study header, and the body being the default PanelMeasurements sub-grouping.
There was a problem hiding this comment.
Probably the default grouping function for PanelMeasurements is fine, just passing in the top level selector which selects only tracked measurements, and that should ONLY use series instance UID so that when we add multiple studies to PanelMeasurement it just automatically creates multiple sub-groups.
There was a problem hiding this comment.
Filtering measurements to include only tracked ones is understandable—that makes sense.
However, I don’t understand why, or even what it means, to 'group' measurements by 'tracked measurements.' Could you clarify?
|
Could you add two additional breakdown to the PanelMeasurements, and change the configuration of PanelMeasurements to use that. The classes/functions we want are: groupStudyMeasurement(props, measurements)
Then split the existing primary items/additional items into a separate component that just shows the two items, along with the primary header. |
| @@ -28,7 +28,7 @@ function PanelMeasurementTableTracking({ | |||
| }); | |||
|
|
|||
| useEffect(() => { | |||
| let updatedMeasurementFilters = { ...measurementFilters }; | |||
| const updatedMeasurementFilters = { ...measurementFilters }; | |||
| if (trackedMeasurements.matches('tracking') && trackedStudy) { | |||
| updatedMeasurementFilters.measurementFilter = filterTracked(trackedStudy, trackedSeries); | |||
There was a problem hiding this comment.
Can you fix the measurementFilter to be just a single filter, and make it independent of the tracked study.
| variant="ghost" | ||
| className="pl-1.5" | ||
| onClick={() => { | ||
| commandsManager.runCommand('clearMeasurements', measurementFilters); |
There was a problem hiding this comment.
Ouch - this one is running clear measurements instead of saveCSV - can you fix that so it runs saveCSV (or whatever the command name is to save the CSV)
Also, can you pass in the nested measurement filter instead of the top level measurement filter for all 3 commands - that is the new filter I (just) asked to be created in the PanelMeasurement table - that way the command applies to the correct set of data.
| @@ -0,0 +1,13 @@ | |||
| export const groupByStudy = displaySetService => (groupedMeasurements, item) => { | |||
There was a problem hiding this comment.
Just add a docs to indicate that this groups measurements by study in order to allow display and saving by study.
Context
Building on top of: #4501
Changes & Results
These changes group the measurements of the all-measurements tab by study.
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment