Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(thumbnail highlight): Thumbnails of hydrated series are now highlighted #3594

Merged
merged 6 commits into from
Aug 16, 2023
Merged

feat(thumbnail highlight): Thumbnails of hydrated series are now highlighted #3594

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f5c4258
feat(thumbnail highlight): Thumbnails representing hydratable series …
jbocce Aug 10, 2023
31ac98d
Updating thumbnails when a segmentation is deleted.
jbocce Aug 11, 2023
404d0b5
Merge remote-tracking branch 'upstream/master' into feat/OHIF-3421-im…
jbocce Aug 15, 2023
91315a5
PR feedback.
jbocce Aug 15, 2023
84830f3
git add DisplaySet.ts
jbocce Aug 16, 2023
5bef6ce
Merge remote-tracking branch 'upstream/master' into feat/OHIF-3421-im…
jbocce Aug 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions extensions/cornerstone/src/services/SegmentationService/SegmentationService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1123,6 +1123,8 @@ class SegmentationService extends PubSubService {
);
}

this._setDisplaySetIsHydrated(segmentationId, true);

segmentation.hydrated = true;

if (!suppressEvents) {
Expand All @@ -1132,6 +1134,18 @@ class SegmentationService extends PubSubService {
}
};

private _setDisplaySetIsHydrated(
displaySetUID: string,
isHydrated: boolean
): void {
const {
DisplaySetService: displaySetService,
} = this.servicesManager.services;
const displaySet = displaySetService.getDisplaySetByUID(displaySetUID);
displaySet.isHydrated = isHydrated;
displaySetService.setDisplaySetMetadataInvalidated(displaySetUID);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if this is the best way, but at the very least it will start a discussion.

Copy link
Member

Choose a reason for hiding this comment

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

hmmm, I see why you need this, but at the same time we have another listener for this at the OHIFCornerstoneViewport which invalidates the data. Maybe we need another event to decouple this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about a new DisplaySetService event called DISPLAY_SET_UPDATED?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure really, it is kind of the same event we are invalidating a metadata, but at the same time we don't want to hit that other listener in the OHIFCornerstoneViewport, aaaaah
I hate having DISPLAY_SET_UPDATED and DISPLAY_SET_SERIES_METADATA_INVALIDATED
Let's think till we meet

}

private _highlightLabelmap(
segmentIndex: number,
alpha: number,
Expand Down Expand Up @@ -1303,6 +1317,8 @@ class SegmentationService extends PubSubService {
}
}

this._setDisplaySetIsHydrated(segmentationId, false);

this._broadcastEvent(this.EVENTS.SEGMENTATION_REMOVED, {
segmentationId,
});
Expand Down
21 changes: 21 additions & 0 deletions ...s/measurement-tracking/src/panels/PanelStudyBrowserTracking/PanelStudyBrowserTracking.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,29 @@ function PanelStudyBrowserTracking({
}
);

const SubscriptionDisplaySetMetaDataInvalidated = displaySetService.subscribe(
displaySetService.EVENTS.DISPLAY_SET_SERIES_METADATA_INVALIDATED,
Copy link
Member

Choose a reason for hiding this comment

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

we should add the same logic to the PanelStudyBrowswer too

() => {
const mappedDisplaySets = _mapDisplaySets(
displaySetService.getActiveDisplaySets(),
thumbnailImageSrcMap,
trackedSeries,
viewports,
viewportGridService,
dataSource,
displaySetService,
uiDialogService,
uiNotificationService
);

setDisplaySets(mappedDisplaySets);
}
);

return () => {
SubscriptionDisplaySetsAdded.unsubscribe();
SubscriptionDisplaySetsChanged.unsubscribe();
SubscriptionDisplaySetMetaDataInvalidated.unsubscribe();
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [
Expand Down Expand Up @@ -459,6 +479,7 @@ function _mapDisplaySets(
// .. Any other data to pass
},
isTracked: trackedSeriesInstanceUIDs.includes(ds.SeriesInstanceUID),
isHydrated: ds.isHydrated,
viewportIdentificator,
};

Expand Down
2 changes: 2 additions & 0 deletions platform/ui/src/components/ThumbnailList/ThumbnailList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const ThumbnailList = ({
imageSrc,
messages,
imageAltText,
isHydrated,
Copy link
Member

Choose a reason for hiding this comment

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

it is kind of weird that we have isHydrated even for regular imagees (e.g., CT, PT, etc.), can we rename it to something else? I don't know what but basically it is is this display set hydrated given that it is derived (rt, seg, sr)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about calling it isADerivedAndHydratedDisplaySet?

Copy link
Member

Choose a reason for hiding this comment

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

How about isHydratedForDerivedDisplaySet

}) => {
const isActive = activeDisplaySetInstanceUIDs.includes(
displaySetInstanceUID
Expand Down Expand Up @@ -104,6 +105,7 @@ const ThumbnailList = ({
onThumbnailDoubleClick(displaySetInstanceUID)
}
viewportIdentificator={viewportIdentificator}
isHydrated={isHydrated}
/>
);
default:
Expand Down
23 changes: 19 additions & 4 deletions platform/ui/src/components/ThumbnailNoImage/ThumbnailNoImage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const ThumbnailNoImage = ({
messages,
dragData,
isActive,
isHydrated,
}) => {
const [collectedProps, drag, dragPreview] = useDrag({
type: 'displayset',
Expand All @@ -31,8 +32,8 @@ const ThumbnailNoImage = ({
return (
<div
className={classnames(
'flex flex-row flex-1 cursor-pointer outline-none border-transparent hover:border-blue-300 focus:border-blue-300 rounded select-none',
isActive ? 'border-2 border-primary-light' : 'border'
'flex flex-row flex-1 cursor-pointer outline-none hover:border-blue-300 focus:border-blue-300 rounded select-none',
isActive ? 'border-2 border-primary-light' : 'border border-transparent'
)}
style={{
padding: isActive ? '11px' : '12px',
Expand All @@ -47,12 +48,25 @@ const ThumbnailNoImage = ({
<div ref={drag}>
<div className="flex flex-col flex-1">
<div className="flex flex-row items-center flex-1 mb-2">
<Icon name="list-bullets" className="w-12 text-secondary-light" />
<Icon
name="list-bullets"
className={classnames(
'w-12',
isHydrated ? 'text-primary-light' : 'text-secondary-light'
)}
/>
<Tooltip
position="bottom"
content={<Typography>{modalityTooltip}</Typography>}
>
<div className="px-3 text-lg text-white rounded-sm bg-primary-main">
<div
className={classnames(
'px-3 text-lg rounded-sm',
isHydrated
? 'text-black bg-primary-light'
: 'text-white bg-primary-main'
)}
>
{modality}
</div>
</Tooltip>
Expand Down Expand Up @@ -116,6 +130,7 @@ ThumbnailNoImage.propTypes = {
onDoubleClick: PropTypes.func.isRequired,
messages: PropTypes.object,
isActive: PropTypes.bool.isRequired,
isHydrated: PropTypes.bool,
};

export default ThumbnailNoImage;