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

Make it so that we can use Web.DOM.Document createElement with the purescript-canvas library. Useful for double buffering. #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thanacles
Copy link

I would like to create a canvas that I could use for double buffering. I feel like this is a common use case for canvases and thus would be useful to others as well. So I add a createCanvas function to the library in this PR.

Issue: #75

@hdgarrood
Copy link
Collaborator

This is of course useful alongside these functions, but the purescript-web libraries are intended to be low level libraries that conform relatively closely to the relevant web APIs - and so this function should really live in web-dom, since it's part of the Document API. Specifically, see:

My worry with adding this here is that if we start adding functions because they're useful, it'll make more space for more opinionated decisions, which can increase the maintenance burden quite a bit.

@hdgarrood
Copy link
Collaborator

I suppose this library should arguably import HTMLCanvasElement instead of defining its own type, along the lines of what's discussed in #61.

@thanacles
Copy link
Author

@hdgarrood Makes sense. Do you think there is a way to switch this library to use HTMLCanvasElement and stay reverse compatible? Or do you think, we should just bite the bullet and make a breaking change?

@hdgarrood
Copy link
Collaborator

It should be possible to replace the CanvasElement declaration with

type CanvasElement = HTMLCanvasElement

which I think would make it mostly non-breaking, although you could potentially still have issues if you're trying to declare type class instances for both CanvasElement and HTMLCanvasElement.

@thanacles thanacles changed the title Add function for creating a canvas element. Useful for double buffering. Make it so that we can use Web.DOM.Document createElement with the purescript-canvas library. Useful for double buffering. Feb 1, 2021
@thanacles
Copy link
Author

How about something like this. There is more refactoring that can be done, mainly to do with reusing functions in the other libraries instead of duplicating. But, I feel like this change is disruptive enough, and refactoring should be delegated to the future. I want something like this change for a project I'm working on because I want to use double buffering.

@hdgarrood
Copy link
Collaborator

hdgarrood commented Feb 1, 2021

I would actually prefer to do this refactoring in one go (i.e. the entire task of #61 - splitting off the Context2D interface to a new web-context2d library) than spreading it out over multiple releases. I would recommend that you define coercions in your project as a stopgap, like this:

toHTMLCanvasElement :: CanvasElement -> HTMLCanvasElement
toHTMLCanvasElement = unsafeCoerce

or indeed just include the FFI wrapper for document.createElement("canvas") that you had in the first version of this PR locally in your project.

@thanacles
Copy link
Author

OK, will do. :(

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 this pull request may close these issues.

3 participants