search

dominikh / go-js-dom

MIT License
249 stars 42 forks source link

Use float for arcs #42

Closed Eun closed 7 years ago

dmitshur commented 7 years ago

Hi @Eun, thanks for wanting to contribute!

Can you please provide some rationale for this change?

According to https://www.w3schools.com/tags/canvas_arc.asp, arc parameters are:

image

Given sAngle, eAngle are angles specified in radians, using float64 makes sense to me.

Eun commented 7 years ago

Radian is should be a float because why shouldn't it be allowed to go for 90 degrees (1.570796 radians)?

Practical Example: To draw a full circle you should use Pi:

context.Arc(100, 100, 75, 0, math.Pi*2, false)

Anyways please use MDN as reference since it is more exact and not outdated: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/arc

dmitshur commented 7 years ago

The CanvasRenderingContext2D.Rotate method currently takes int parameter in dom package. It's documented to also use radians:

https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/rotate

We could fix that too, if you want to expand the scope of your PR @Eun.

dominikh commented 7 years ago

LGTM @shurcooL

Eun commented 7 years ago

@shurcooL @dominikh Updated for CanvasRenderingContext2D.rotate