-
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: remove unnecessary event firing for annotations #123
Conversation
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.
Minor comments to change but mostly LGTM.
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.
Looks like the camera stuff is almost there, but looks like it will break on rotation.
// half of the height of the viewport in the world unit (mm), we can use that | ||
// to compute the scale factor which is the ratio of the viewport height in pixels | ||
// to the current rendered image height. | ||
const { rowPixelSpacing } = image; |
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.
Ooooh, what if the viewport is rotated? I don't think it correct.
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.
it seems to work fine
|
||
viewport.scale += diff; // parallelScale; //viewport.scale < 0.1 ? 0.1 : viewport.scale; | ||
if (scale) { | ||
const { rowPixelSpacing } = image; |
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.
And here.
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.
it seems to work fine
|
||
import _getHash from './_getHash'; | ||
|
||
import _setAttributesIfNecessary from './_setAttributesIfNecessary'; | ||
import _setNewAttributesIfValid from './_setNewAttributesIfValid'; | ||
|
||
function drawCircle( | ||
svgDrawingHelper: any, | ||
svgDrawingHelper: SVGDrawingHelper, |
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 thought we settled on svgDraingHelpers
?
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.
LGTM
Fix:
Feat:
suppressEvents
to StackViewportsetProperties