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

Path2D as Module? #132

Closed
3xau1o opened this issue Jul 5, 2024 · 3 comments · Fixed by #133
Closed

Path2D as Module? #132

3xau1o opened this issue Jul 5, 2024 · 3 comments · Fixed by #133

Comments

@3xau1o
Copy link

3xau1o commented Jul 5, 2024

Example of path2D as currently

let path: path2d = newPath2D("M24.85,10.126c2.018-4.783,6.628-8.125,11.99-8.125c7.223")
ctx->beginPath
ctx->strokePath2D(path)
ctx->fillPath2D(path)

After working with react rescript bindings would expect Path2D to be it's own module like

let path: path2d = Path2D.make("M24.85,10.126c2.018-4.783,6.628-8.125,11.99-8.125c7.223")
ctx->beginPath
ctx-> Path2D.stroke(path)
ctx-> Path2D.fill(path)

That would be a breaking change, however Path2D bindings seems currently on an early stage

Would a PR with those changes be accepted?

If so, would be possible to add all remaining Path2D method bindings to that module

Path2D.addPath()
Path2D.closePath()
Path2D.moveTo()
Path2D.lineTo()
Path2D.bezierCurveTo()
Path2D.quadraticCurveTo()
Path2D.arc()
Path2D.arcTo()
Path2D.ellipse()
Path2D.rect()
Path2D.roundRect()

@3xau1o 3xau1o changed the title Path2D Module? Path2D as Module? Jul 5, 2024
@TheSpyder
Copy link
Owner

Because it was contributed that way in #45 🤷‍♂️

Yes, I'd accept a PR to add a new module. I am a couple of years overdue for publishing a 1.0 release, that seems like a good breaking change to have

@3xau1o 3xau1o closed this as completed Jul 5, 2024
@3xau1o 3xau1o mentioned this issue Jul 5, 2024
7 tasks
@TheSpyder
Copy link
Owner

you didn't need to close this - your PR would automatically close it when merged.

@3xau1o 3xau1o reopened this Jul 11, 2024
@3xau1o
Copy link
Author

3xau1o commented Jul 11, 2024

may add more canvas tests

@TheSpyder TheSpyder linked a pull request Jul 11, 2024 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants