search

joakin / elm-canvas

A canvas drawing library for Elm
https://package.elm-lang.org/packages/joakin/elm-canvas/latest/
BSD 3-Clause "New" or "Revised" License
163 stars 30 forks source link

implemented observedAttributes for width and height (closes #7) #8

Closed dkodaj closed 5 years ago

dkodaj commented 5 years ago

In action: https://ellie-app.com/4PQ6ymKsHZQa1

dkodaj commented 5 years ago

Ok, will try!

dkodaj commented 5 years ago

I've committed the changes above and modified toHtml to set the element's width and height directly. Seems to work locally.

Calling this.setCanvasDimensions() instead of setCanvasDimensions() breaks it for some reason.

joakin commented 5 years ago

I don't think it works without the this. in the caller. I'm getting this when I run it locally:

[Error] ReferenceError: Can't find variable: setCanvasDimensions
    attributeChangedCallback (elm-canvas.js:45)
    setAttribute
    _VirtualDom_applyAttrs (ChangeOfDimensions.js:2811)
    _VirtualDom_applyFacts (ChangeOfDimensions.js:2776)
    _VirtualDom_render (ChangeOfDimensions.js:2748)
    _VirtualDom_render (ChangeOfDimensions.js:2752)
    _VirtualDom_applyPatchRedraw (ChangeOfDimensions.js:3725)
    _VirtualDom_applyPatchesHelp (ChangeOfDimensions.js:3638)
    (anonymous function) (ChangeOfDimensions.js:3883)
    _Browser_makeAnimator (ChangeOfDimensions.js:3939)
    _Platform_initialize (ChangeOfDimensions.js:1863)
    Global Code (change-of-dimensions.html:11)
[Error] ReferenceError: Can't find variable: setCanvasDimensions
    (anonymous function) (elm-canvas.js:37)
[Error] ReferenceError: Can't find variable: setCanvasDimensions
    attributeChangedCallback (elm-canvas.js:45)
    setAttribute
    _VirtualDom_applyAttrs (ChangeOfDimensions.js:2811)
    _VirtualDom_applyFacts (ChangeOfDimensions.js:2776)
    _VirtualDom_applyPatch (ChangeOfDimensions.js:3655)
    _VirtualDom_applyPatchesHelp (ChangeOfDimensions.js:3638)
    (anonymous function) (ChangeOfDimensions.js:3883)
    updateIfNeeded (ChangeOfDimensions.js:3947)

I'll have a look to see if I can fix it locally.

dkodaj commented 5 years ago

You're right, apologies. The reason it worked here (Firefox 65.0.1) had nothing to do with changes in the Canvas module or even with the observedAttributes stuff. Inded, the following small hack in connectedCallback() seemed enough:

https://ellie-app.com/4QdmfRQHMpga1

That looks more like a bug than a solution though.

joakin commented 5 years ago

I've fixed some stuff locally and merged your contribution. Thank you!

If you are interested, this are the things and fixes I did to your work: https://github.com/joakin/elm-canvas/commit/c628a917df834ae9373b52d3fd9ce8ce8654a6b6 The commit message has some info too.

I'll add your example and make some releases with the fixes.

dkodaj commented 5 years ago

Thanks!

On Tue, Feb 26, 2019 at 12:00 PM Joaquin notifications@github.com wrote:

I've fixed some stuff locally and merged your contribution. Thank you!

If you are interested, this are the things and fixes I did to your work: c628a91 https://github.com/joakin/elm-canvas/commit/c628a917df834ae9373b52d3fd9ce8ce8654a6b6 The commit message has some info too.

I'll add your example and make some releases with the fixes.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/joakin/elm-canvas/pull/8#issuecomment-467395039, or mute the thread https://github.com/notifications/unsubscribe-auth/AaZm_2Y_Y3jcKtmQfb3K-qDs7gyPeYmfks5vRRPQgaJpZM4bP29e .