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

Jump to bottom

Pass-through options from DOM.context2d to getContext #273

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Pass-through options from DOM.context2d to getContext #273

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3227ae6
The third argument to DOM.context2d can be an options object with {sc…
Fil Dec 20, 2021
c82eaa8
pass-through
Fil Dec 20, 2021
4e72641
cleaner
Fil Dec 20, 2021
76e1b11
Apply Fabian’s suggestion, handle null options
Fil Oct 5, 2022
e1b6ea9
options === null
Fil Oct 5, 2022
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
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ html`<canvas width=960 height=500>`

If you are using [2D Canvas](https://www.w3.org/TR/2dcontext/) (rather than [WebGL](https://webglfundamentals.org/)), you should use [DOM.context2d](#DOM_context2d) instead of DOM.canvas for automatic pixel density scaling.

<a href="#DOM_context2d" name="DOM_context2d">#</a> DOM.<b>context2d</b>(<i>width</i>, <i>height</i>[, <i>dpi</i>]) [<>](https://github.com/observablehq/stdlib/blob/main/src/dom/context2d.mjs "Source")
<a href="#DOM_context2d" name="DOM_context2d">#</a> DOM.<b>context2d</b>(<i>width</i>, <i>height</i>[, <i>options</i>]) [<>](https://github.com/observablehq/stdlib/blob/main/src/dom/context2d.mjs "Source")
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we list both signatures here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's there: "As a shorthand notation, options having only scale can be specified as a number." (could be phrased better?)

Copy link
Member

Choose a reason for hiding this comment

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

I feel the shorthand should be mentioned earlier and emphasized, since it's the format that you'll encounter throughout Observable and the notation that you'll commonly use to disable dpr scaling.


Returns a new canvas context with the specified *width* and *height* and the specified device pixel ratio *dpi*. If *dpi* is not specified, it defaults to [*window*.devicePixelRatio](https://developer.mozilla.org/docs/Web/API/Window/devicePixelRatio). To access the context’s canvas, use [*context*.canvas](https://developer.mozilla.org/docs/Web/API/CanvasRenderingContext2D/canvas). For example, to create a 960×500 canvas:
Returns a new canvas context with the specified *width* and *height* and the specified *options*. The scale property of the options object defines the device pixel ratio, and defaults to [*window*.devicePixelRatio](https://developer.mozilla.org/docs/Web/API/Window/devicePixelRatio). The remaining options are passed through as contextAttributes to [getContext](https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/getContext). As a shorthand notation, *options* having only scale can be specified as a number. To access the context’s canvas, use [*context*.canvas](https://developer.mozilla.org/docs/Web/API/CanvasRenderingContext2D/canvas). For example, to create a 960×500 canvas:

```js
{
Expand Down
16 changes: 9 additions & 7 deletions src/dom/context2d.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
export default function(width, height, dpi) {
if (dpi == null) dpi = devicePixelRatio;
var canvas = document.createElement("canvas");
canvas.width = width * dpi;
canvas.height = height * dpi;
export default function (width, height, options = {}) {
const {scale = devicePixelRatio, ...contextOptions} = !isNaN(options)
? {...(options != null && {scale: options})}
Copy link
Member

@mootari mootari Oct 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

There's no need for the destructuring here, and the null check should be explicit. If options is undefined, we already get {}.

Suggested change
? {...(options != null && {scale: options})}
? options === null ? {} : {scale: options}

: options;
Comment on lines +2 to +4
Copy link
Member

@mootari mootari Oct 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
const {scale = devicePixelRatio, ...contextOptions} = !isNaN(options)
? {...(options != null && {scale: options})}
: options;
const {scale = devicePixelRatio, ...contextOptions} = options && !isNaN(options)
? {scale: +options}
: options;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

0 is currently working as before (in the sense that it returns a canvas of size 0)—it would be simpler if we didn't support this "feature".

Copy link
Member

Choose a reason for hiding this comment

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

With the additional null check in the earlier comment we can remove options && here.

Copy link
Member

Choose a reason for hiding this comment

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

(btw, that parseFloat was the result of a brainfart - I meant to write +options)

const canvas = document.createElement("canvas");
canvas.width = width * scale;
canvas.height = height * scale;
canvas.style.width = width + "px";
var context = canvas.getContext("2d");
context.scale(dpi, dpi);
const context = canvas.getContext("2d", !options ? options : contextOptions);
mootari marked this conversation as resolved.
Show resolved Hide resolved
context.scale(scale, scale);
return context;
}