-
-
Notifications
You must be signed in to change notification settings - Fork 127
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
style: use requestAnimationFrame for auto pan #71
Changes from all commits
36eabc8
7eafdd8
e7ed26a
cbe2c55
89f629d
5a46483
4b69794
452411d
dbeafd8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ export function onMouseDown(event, ViewerDOM, tool, value, props, coords = null) | |
switch (tool) { | ||
case TOOL_ZOOM_OUT: | ||
let SVGPoint = getSVGPoint(value, x, y); | ||
nextValue = zoom(value, SVGPoint.x, SVGPoint.y, 1 / props.scaleFactor); | ||
nextValue = zoom(value, SVGPoint.x, SVGPoint.y, 1 / props.scaleFactor, props); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Originally I was passing props around for I forgot to remove these props arguments in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The point about this change is that I'm not sure that set |
||
break; | ||
|
||
case TOOL_ZOOM_IN: | ||
|
@@ -65,7 +65,7 @@ export function onMouseMove(event, ViewerDOM, tool, value, props, coords = null) | |
switch (tool) { | ||
case TOOL_ZOOM_IN: | ||
if (value.mode === MODE_ZOOMING) | ||
nextValue = forceExit ? stopZooming(value, x, y, props.scaleFactor) : updateZooming(value, x, y); | ||
nextValue = forceExit ? stopZooming(value, x, y, props.scaleFactor, props) : updateZooming(value, x, y); | ||
break; | ||
|
||
case TOOL_AUTO: | ||
|
@@ -97,12 +97,12 @@ export function onMouseUp(event, ViewerDOM, tool, value, props, coords = null) { | |
switch (tool) { | ||
case TOOL_ZOOM_OUT: | ||
if (value.mode === MODE_ZOOMING) | ||
nextValue = stopZooming(value, x, y, 1 / props.scaleFactor); | ||
nextValue = stopZooming(value, x, y, 1 / props.scaleFactor, props); | ||
break; | ||
|
||
case TOOL_ZOOM_IN: | ||
if (value.mode === MODE_ZOOMING) | ||
nextValue = stopZooming(value, x, y, props.scaleFactor); | ||
nextValue = stopZooming(value, x, y, props.scaleFactor, props); | ||
break; | ||
|
||
case TOOL_AUTO: | ||
|
@@ -138,7 +138,7 @@ export function onDoubleClick(event, ViewerDOM, tool, value, props, coords = nul | |
let modifierKeysReducer = (current, modifierKey) => current || event.getModifierState(modifierKey); | ||
let modifierKeyActive = props.modifierKeys.reduce(modifierKeysReducer, false); | ||
let scaleFactor = modifierKeyActive ? 1 / props.scaleFactor : props.scaleFactor; | ||
nextValue = zoom(value, SVGPoint.x, SVGPoint.y, scaleFactor); | ||
nextValue = zoom(value, SVGPoint.x, SVGPoint.y, scaleFactor, props); | ||
} | ||
break; | ||
|
||
|
@@ -166,7 +166,7 @@ export function onWheel(event, ViewerDOM, tool, value, props, coords = null) { | |
let scaleFactor = mapRange(delta, -1, 1, props.scaleFactorOnWheel, 1 / props.scaleFactorOnWheel); | ||
|
||
let SVGPoint = getSVGPoint(value, x, y); | ||
let nextValue = zoom(value, SVGPoint.x, SVGPoint.y, scaleFactor); | ||
let nextValue = zoom(value, SVGPoint.x, SVGPoint.y, scaleFactor, props); | ||
|
||
event.preventDefault(); | ||
return nextValue; | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -7,7 +7,7 @@ import eventFactory from './events/event-factory'; | |||||||||||||||||||
|
||||||||||||||||||||
//features | ||||||||||||||||||||
import {pan} from './features/pan'; | ||||||||||||||||||||
import {getDefaultValue, setViewerSize, setSVGSize, setPointOnViewerCenter, reset} from './features/common'; | ||||||||||||||||||||
import {getDefaultValue, setViewerSize, setSVGSize, setPointOnViewerCenter, reset, setZoomLevels} from './features/common'; | ||||||||||||||||||||
import { | ||||||||||||||||||||
onMouseDown, | ||||||||||||||||||||
onMouseMove, | ||||||||||||||||||||
|
@@ -54,6 +54,8 @@ export default class ReactSVGPanZoom extends React.Component { | |||||||||||||||||||
tool: tool ? tool : TOOL_NONE | ||||||||||||||||||||
}; | ||||||||||||||||||||
this.ViewerDOM = null; | ||||||||||||||||||||
|
||||||||||||||||||||
this.autoPanLoop = this.autoPanLoop.bind(this); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
componentWillReceiveProps(nextProps) { | ||||||||||||||||||||
|
@@ -72,6 +74,11 @@ export default class ReactSVGPanZoom extends React.Component { | |||||||||||||||||||
needUpdate = true; | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
if (value.scaleFactorMin !== nextProps.scaleFactorMin || value.scaleFactorMax !== nextProps.scaleFactorMax) { | ||||||||||||||||||||
nextValue = setZoomLevels(nextValue, nextProps.scaleFactorMin, nextProps.scaleFactorMax); | ||||||||||||||||||||
needUpdate = true; | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
if (needUpdate) { | ||||||||||||||||||||
this.setValue(nextValue); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
@@ -181,23 +188,30 @@ export default class ReactSVGPanZoom extends React.Component { | |||||||||||||||||||
onEventHandler(eventFactory(event, value, ViewerDOM)); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
autoPanLoop() { | ||||||||||||||||||||
let coords = {x: this.state.viewerX, y: this.state.viewerY}; | ||||||||||||||||||||
let nextValue = onInterval(null, this.ViewerDOM, this.getTool(), this.getValue(), this.props, coords); | ||||||||||||||||||||
|
||||||||||||||||||||
if (this.getValue() !== nextValue) { | ||||||||||||||||||||
this.setValue(nextValue); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
if(this.autoPanIsRunning) { | ||||||||||||||||||||
requestAnimationFrame(this.autoPanLoop); | ||||||||||||||||||||
} | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
|
||||||||||||||||||||
componentDidMount() { | ||||||||||||||||||||
let {props, state} = this; | ||||||||||||||||||||
if (props.onChangeValue) props.onChangeValue(state.value); | ||||||||||||||||||||
|
||||||||||||||||||||
this.autoPanTimer = setInterval(() => { | ||||||||||||||||||||
let coords = {x: this.state.viewerX, y: this.state.viewerY}; | ||||||||||||||||||||
let nextValue = onInterval(null, this.ViewerDOM, this.getTool(), this.getValue(), this.props, coords); | ||||||||||||||||||||
|
||||||||||||||||||||
if (this.getValue() !== nextValue) { | ||||||||||||||||||||
this.setValue(nextValue); | ||||||||||||||||||||
} | ||||||||||||||||||||
}, 200); | ||||||||||||||||||||
this.autoPanIsRunning = true; | ||||||||||||||||||||
requestAnimationFrame(this.autoPanLoop); | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This runs the auto pan loop when component mounts, whether or not the user is actually auto panning. We should refactor the code so that auto pan loop only runs when the user hovers on one or more of the auto pan regions. I have not thoroughly check the performance metrics, so we should consider optimising the code if the performance drop is too large. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see... to handle this we can use the react-svg-pan-zoom/src/viewer.js Lines 290 to 298 in 5ee9741
|
||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
componentWillUnmount() { | ||||||||||||||||||||
clearTimeout(this.autoPanTimer); | ||||||||||||||||||||
this.autoPanIsRunning = false; | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
render() { | ||||||||||||||||||||
|
@@ -465,6 +479,12 @@ ReactSVGPanZoom.propTypes = { | |||||||||||||||||||
//how much scale in or out on mouse wheel (requires detectWheel enabled) | ||||||||||||||||||||
scaleFactorOnWheel: PropTypes.number, | ||||||||||||||||||||
|
||||||||||||||||||||
// maximum amount of scale a user can zoom in to | ||||||||||||||||||||
scaleFactorMax: PropTypes.number, | ||||||||||||||||||||
|
||||||||||||||||||||
// minimum amount of a scale a user can zoom out of | ||||||||||||||||||||
scaleFactorMin: PropTypes.number, | ||||||||||||||||||||
|
||||||||||||||||||||
//current active tool (TOOL_NONE, TOOL_PAN, TOOL_ZOOM_IN, TOOL_ZOOM_OUT) | ||||||||||||||||||||
tool: PropTypes.oneOf([TOOL_AUTO, TOOL_NONE, TOOL_PAN, TOOL_ZOOM_IN, TOOL_ZOOM_OUT]), | ||||||||||||||||||||
|
||||||||||||||||||||
|
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 have changed this to
let
as I was modifying this variable at some point in development, but now there is no reason for this change. Although it's no big deal, I should be usingconst
.