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

Remove radiusY and rotation from arcTo() method #3371

Merged
merged 2 commits into from
Jan 23, 2018
Merged

Remove radiusY and rotation from arcTo() method #3371

merged 2 commits into from
Jan 23, 2018

Conversation

annevk
Copy link
Member

@annevk annevk commented Jan 17, 2018

They are not implemented.

Partially reverts e25d3f9.

Fixes #878.


/canvas.html ( diff )

They are not implemented.

Partially reverts e25d3f9.

Fixes #878.
@annevk annevk added removal/deprecation Removing or deprecating a feature topic: canvas labels Jan 17, 2018
@annevk
Copy link
Member Author

annevk commented Jan 17, 2018

@junov r?

Copy link
Member

@junov junov left a comment

Choose a reason for hiding this comment

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

lgtm with minor corrections

source Outdated
the point (<var>x0</var>, <var>y0</var>) to the start tangent point by a straight line, adding
the start tangent point to the subpath, and then connect the start tangent point to the end
tangent point by <var>The Arc</var>, adding the end tangent point to the subpath.</p></li>
ellipse that has radius <var>radius</var> on the major and minor axis, and that has one point
Copy link
Member

Choose a reason for hiding this comment

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

use "circle" instead of "ellipse", and don't mention major/minor axis.

source Outdated
tangent to the half-infinite line that crosses the point (<var>x0</var>, <var>y0</var>) and ends
at the point (<var>x1</var>, <var>y1</var>), and that has a different point tangent to the
half-infinite line that ends at the point (<var>x1</var>, <var>y1</var>) and crosses the point
(<var>x2</var>, <var>y2</var>). The points at which this ellipse touches these two lines are
Copy link
Member

Choose a reason for hiding this comment

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

ellipse -> circle

@annevk
Copy link
Member Author

annevk commented Jan 19, 2018

Thanks, I should have studied the original commit more carefully. I think I got all the places now.

@rakuco
Copy link
Member

rakuco commented Jan 26, 2018

I've made a related change to wpt's interfaces/html.idl in the PR above. From a quick grep, it doesn't look like any tests were using the radius{X,Y} overload, so I guess there's nothing else that needs to be updated there?

@annevk
Copy link
Member Author

annevk commented Jan 26, 2018

Thanks, I forgot about that. I suspect that's it.

annevk pushed a commit to web-platform-tests/wpt that referenced this pull request Jan 26, 2018
@foolip
Copy link
Member

foolip commented Jan 27, 2018

Context is that @rakuco and I are looking at a normative spec change without tests every week this quarter to try to understand what's not working well in practice.

In this case I think the answer is not to do a careful search for existing tests for unimplemented features, but to auto-sync IDL: web-platform-tests/wpt#7784. And tests for unimplemented features that remain behind will fail everywhere, and we'll have other ways of finding those soon: web-platform-tests/results-collection#83. @mdittmer FYI :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
removal/deprecation Removing or deprecating a feature topic: canvas
Development

Successfully merging this pull request may close these issues.

4 participants