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

attempt to fix: video/canvas option checks #13

Closed
wants to merge 1 commit into from

Conversation

nodgear
Copy link

@nodgear nodgear commented Nov 30, 2022

Pull request following from #12

@ThaUnknown
Copy link
Owner

ThaUnknown commented Dec 1, 2022

I don't like this, a lot, the general idea is that if the user specifies their own canvas they position it themselves, so the parent shouldn't even be created in that case, what you want to do is spawn a canvas and have it as a reference once its appended, which is... useless as you can just simply do

const instance = new JASSUB(opts)
const canvas = instance._canvas

and you want to do

const canvas = document.createElement('canvas')
const instance = new JASSUB({ ...opts, canvas })

I don't really agree with this

I also want the user to be able specify both the video and the canvas at once, which would end up resizing the canvas on video mutation but not re-positioning it

I never got around to implementing this after I re-wrote this library from scratch, and the whole opts.canvas as of now is just an artifact/placeholder

@ThaUnknown
Copy link
Owner

46fabb9

@ThaUnknown ThaUnknown closed this Dec 2, 2022
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.

None yet

2 participants