feat: Start using group filtering to define measurements table layout #4501
Conversation
✅ 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. |
Viewers
|
||||||||||||||||||||||||||||
| Project |
Viewers
|
| Branch Review |
feat/measurements-service-filtering
|
| Run status |
|
| Run duration | 02m 15s |
| Commit |
|
| Committer | Bill Wallace |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
2
|
Skipped
|
0
|
Passing
|
44
|
| View all changes introduced in this branch ↗︎ | |
wayfarer3130
changed the title
extensions/measurement-tracking/src/panels/PanelMeasurementTableTracking.tsx
Show resolved
Hide resolved
| <MeasurementTable.Body /> | ||
| </MeasurementTable> | ||
| )} | ||
| {untrackedFindings.length > 0 && ( |
There was a problem hiding this comment.
I think this might upset some people because we're imposing an untracked view on them. I was hoping people could choose what to see based on the filters they set through the customization service. Here, we simply loop over those filters and create the view, regardless of the number.
<MeasurementTable.Header>
</MeasurementTable.Header>
There was a problem hiding this comment.
Actually, that is what I wanted to have, I just wasn't quite there yet. Let me develop this a bit mroe and will pass it by you.
There was a problem hiding this comment.
Seems like the demo is opting in for untracked as well
in viewer.ohif.org we don't show temporary (untracked) can you make sure we don't break it? or at least if we are doing , we should separate them out since we can't create DICOM SR of the other orientations
| @@ -169,7 +169,7 @@ export class CommandsManager { | |||
| * the commandOptions specified in the base. | |||
| */ | |||
| public run( | |||
| toRun: Command | Commands | Command[] | string | undefined, | |||
| toRun: Command | Commands | (Command | string)[] | string | undefined, | |||
There was a problem hiding this comment.
could we please update the docsring above and give example of what we can handle
something like
run ('updateMeasurement')
run (['updateMeasurement', 'displayWhatever'])
// other scnearios
| @@ -81,7 +81,7 @@ const getPanelModule = ({ commandsManager, servicesManager, extensionManager }: | |||
| name: 'panelMeasurement', | |||
| iconName: 'tab-linear', | |||
| iconLabel: 'Measure', | |||
| label: 'Measurement', | |||
| label: 'All Measurements', | |||
There was a problem hiding this comment.
I think changing this might disrupt other people's tests if they're selecting based on the label. Can we leave this one unchanged?
| public getMeasurements(filter?: MeasurementFilter) { | ||
| if (filter) { | ||
| return [...this.measurements.values()].filter(measurement => filter.call(this, measurement)); | ||
| } | ||
| return [...this.measurements.values()]; | ||
| } |
There was a problem hiding this comment.
how about this
public getMeasurements(filter?: MeasurementFilter) {
return [...this.measurements.values()].filter(measurement => filter?.call(this, measurement) ?? true);
}
| @@ -582,11 +588,15 @@ class MeasurementService extends PubSubService { | |||
| }); | |||
| } | |||
|
|
|||
| clearMeasurements() { | |||
| public clearMeasurements(filter?: MeasurementFilter) { | |||
There was a problem hiding this comment.
doc please, what is filter, and how it can be used
| * the study and series. | ||
| */ | ||
| export function filterTracked(trackedStudy: string, trackedSeries) { | ||
| return measurement => { |
There was a problem hiding this comment.
does it matter tracked? can't it be filterByStudyAndSeries and still does the job?
| * Filters for additional findings, that is, measurements with | ||
| * a value of type point, and having a referenced image | ||
| */ | ||
| export function filterAdditionalFinding(measurementService) { |
|
|
||
| const { formatDate } = utils; | ||
|
|
||
| export function StudySummaryFromMetadata({ studyInstanceUID }) { |
There was a problem hiding this comment.
Can we use dcmjs style namings? so StudyInstanceUID for the prop? below you are actually doing const { StudyDate, StudyDescription } = instanceMeta;
Context
The measurements service already has a few groups of measurement types, it would be nice to have more groups of types and allow them to be listed in various ways in the measurements panel.
This PR is a start of how the grouping might work, which includes some changes in the core grouping code. However, it doesn't include any enhanced features yet.
Changes & Results
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment