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.

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

path.arc(…anticlockwise) must be true or 1, not truthy #21

Closed
Fil opened this issue May 13, 2019 · 1 comment
Closed

path.arc(…anticlockwise) must be true or 1, not truthy #21

Fil opened this issue May 13, 2019 · 1 comment

Comments

@Fil
Copy link
Member

Fil commented May 13, 2019

We have cw = 1 ^ ccw

  • 0, "0"(edited), undefined, false => draws a clockwise arc
  • 1, "1", true => draws an anticlockwise arc

So far so good, but trying with anticlockwise = "anticlockwise" or anticlockwise = 3 (or most other truthy values) will give a very weird path with an inverted curvature, because it will have both ccw truthy, and cw = 1.

I don't think the code should be fixed as this is probably the fastest approach, but maybe the documentation?

Currently we say

If anticlockwise is true, the arc is drawn in the anticlockwise direction; otherwise, it is drawn in the clockwise direction

"otherwise" could be changed to "if false or undefined", or "if false"…

Another possibility:

If anticlockwise, a boolean, is true, the arc is drawn in the anticlockwise direction; otherwise, it is drawn in the clockwise direction.

(I got bitten by using a checkbox’s value, which gave me anticlockwise="On".)

See interactive test at https://observablehq.com/d/d8c04cb491a6c444

@mbostock
Copy link
Member

That’s a bug.

@Fil Fil changed the title document path.arc(…anticlockwise) must be true or 1, not true-ish path.arc(…anticlockwise) must be true or 1, not true-ish May 13, 2019
@Fil Fil changed the title path.arc(…anticlockwise) must be true or 1, not true-ish path.arc(…anticlockwise) must be true or 1, not truthy Jun 7, 2019
@mbostock mbostock mentioned this issue Jul 30, 2019
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants