-
Notifications
You must be signed in to change notification settings - Fork 18
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
New: Add support for handling device pixel ratio #45
Conversation
1716092
to
e0b61fe
Compare
@@ -82,6 +132,17 @@ export class CanvasRenderer<N extends INodeBase, E extends IEdgeBase> extends Em | |||
this._render(graph); | |||
}, getThrottleMsFromFPS(this._settings.fps)); | |||
} | |||
|
|||
// Change DPR from automatic to manual handling or change DPR value manually | |||
if (!isNumber(previousDprValue) && isNumber(newDprValue) && newDprValue !== previousDprValue) { |
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.
Nitpicking, but is newDprValue !== previousDprValue
necessary here? It seems it's always true because newDprValue
is a number while previousDprValue
is not. On the other hand, !isNumber(previousDprValue)
can be removed to allow users to change the DPR value more than once, but it does not seem like a real use case.
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.
Yep you are right, newDrpValue !== previousDprValue
can be removed, but not the other check.
This is removing automatic observer for DPR. You can see on line 87 that it checks if settings doesn't have DPR, it will start automatic observer that checked for DPR from the browser.
So these lines 137 and 143 are for:
- 137: Stop the automatic observer - this can happen only if previousDPR was not a number, and the new value is defined
- 142: Start the observer again - this can happen only if previousDRP was a number, and the new value is null (this enables users to remove manual DPR handling).
Users can still change DPR manually which happens on line 127.
@@ -195,6 +256,23 @@ export class CanvasRenderer<N extends INodeBase, E extends IEdgeBase> extends Em | |||
} | |||
} | |||
|
|||
private _resize() { | |||
const dpr = this._settings.devicePixelRatio || window.devicePixelRatio; |
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.
Nitpicking, but it would be nice to add a check for window.devicePixelRatio
being null or undefined, just in case of older browsers that do not support this. I think using || 1
will do the job.
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.
Not nitpicking, but it's a valid bug. Great catch.
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 good to me now!
The following PR adds support for automatic and manual handling of
devicePixelRatio
. More information aboutdevicePixelRatio
can be found here: https://developer.mozilla.org/en-US/docs/Web/API/Window/devicePixelRatioThere are two modes: Automatic and Manual.
Automatic
devicePixelRatio
handler (default)The default behavior is the automatic handler of the
devicePixelRatio
value and changes (e.g. user drags a view from one display to another with a different DPR value). The automatic handler is turned on when the settings valuerender.devicePixelRatio
equals tonull
.Manual
devicePixelRatio
handlerIf you want to manually handle the
devicePixelRatio
values, just set the settings valuerender.devicePixelRatio
to a numeric DPR value. Orb will stop listening for DPR changes then.This fixes issue #19.