-
Notifications
You must be signed in to change notification settings - Fork 326
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.
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(scrollEvent): added out of bounds scroll #476
feat(scrollEvent): added out of bounds scroll #476
Conversation
✅ Deploy Preview for cornerstone-3d-docs canceled.
|
volumeId: volumeId, | ||
viewport: viewport, | ||
delta, | ||
desiredFrameIndex, | ||
maxFrameIndex, | ||
currentImageId: viewport.getCurrentImageId(), | ||
currentFrameIndex: desiredFrameIndex - delta, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add these to the EventTypes too?
* @param spacingInNormalDirection | ||
* @returns { int, int } | ||
*/ | ||
function _getDeltaFrameIndex(delta, sliceRange, spacingInNormalDirection) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add the exports you want from the getVolumeViewportScrollInfo
so that we don't recomute these again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see my comments
7366080
to
c8d944c
Compare
@sedghi this is ready for your review |
@@ -59,4 +61,32 @@ export function scrollVolume( | |||
position: newPosition, | |||
}); | |||
viewport.render(); | |||
|
|||
const { numScrollSteps, currentStepIndex } = | |||
csUtils.getVolumeViewportScrollInfo(viewport, volumeId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I"m a little bit confused, isn't it the fact that this function relies on the sliceRangeInfo
that is calculated above? So can we refactor its logic so that we don't do sliceRangeInfo
getter again?
function getVolumeViewportScrollInfo(
viewport: IVolumeViewport,
volumeId: string
) {
const { sliceRange, spacingInNormalDirection } = getVolumeSliceRangeInfo(
viewport,
volumeId
);
const { min, max, current } = sliceRange;
// Now we can see how many steps there are in this direction
const numScrollSteps = Math.round((max - min) / spacingInNormalDirection);
// Find out current frameIndex
const fraction = (current - min) / (max - min);
const floatingStepNumber = fraction * numScrollSteps;
const currentStepIndex = Math.round(floatingStepNumber);
return { numScrollSteps, currentStepIndex };
}
to
function getVolumeViewportScrollInfoFromSliceRange(
sliceRange,
spacingInNormalDirection
) {
const { min, max, current } = sliceRange;
// Now we can see how many steps there are in this direction
const numScrollSteps = Math.round((max - min) / spacingInNormalDirection);
// Find out current frameIndex
const fraction = (current - min) / (max - min);
const floatingStepNumber = fraction * numScrollSteps;
const currentStepIndex = Math.round(floatingStepNumber);
return { numScrollSteps, currentStepIndex };
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sedghi let me know what you think of the change I made. I just export the sliceRangeInfo from getVolumeViewportScrollInfo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API check failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah I need to build updateapi
c8d944c
to
7010643
Compare
7010643
to
024590b
Compare
This adds an event for when user stack scroll goes out of bounds