MarvinKlein1508 / SignaturePad

A simple to use blazor component to draw custom signatures.
MIT License
72 stars 12 forks source link

Added support for Backgroud Image #23

Closed dblood2 closed 8 months ago

dblood2 commented 8 months ago

Some of this may be too specific to my own project and not for users globally. Please feel free to edit my additions to make them better if necessary.

MarvinKlein1508 commented 8 months ago

Hi @dblood2

I've took a look on your PR and changed it a bit. Here are a few notes for you to understand what and why I changed it.

First of I've tried your implementation and I ended up in an infite refresh loop. So I took a look at your code and saw this:

        if (img.width > 0) {
            this._element.width = img.width;
            this._element.height = img.height;
        }
        else {
            if (image != "") {
                location.reload();
            }
        }

You should never, I repeat never force a reload in Blazor. This results in the server generating a new circuit for the user and the entire state of the app will be thrown away. In addtion the old circuit is still in memory due to the framework itself which keeps it for like 15 minutes.

Next I removed the two parameters for CanvasWidth & CanvasHeight and replaced them with blazors native feature to apply all extra parameters. This also makes it possible now to set apply all style through CSS, so I got rid of the hardcoded style="width:100%;" from my version.

I also got rid of adjusting the canvas size in the setBackgroundImage function. The size should only be set once. It's generally not a good idea to resize the canvas on the fly since this will override the last state of the canvas.

Since the background image should be loaded automatically everytime the image is being set, I got rid of all other calls to setBackgroundImage. Now I had it working on desktop.

So I checked my phone and it wasn't working there. The reason for this is that the window.devicePixelRatio is different depending on your resolution. So we need to draw the image based on the resolution of the device. So I've changed your drawImage implementation to this:

ctx.drawImage(img, 0, 0,
    img.width * window.devicePixelRatio,
    img.height * window.devicePixelRatio,
    0, 0,
    img.width * window.devicePixelRatio,
    img.height * window.devicePixelRatio
);

The last step I needed to do was applying the clientHeigth in the constructor. Then it also worked on my phone.

this._element.width = this._element.clientWidth;
this._element.height = this._element.clientHeight;

Could you checkout if this changes work for you as well?

I've tested them on a 1920x1080 monitor and on my iPhone 15 Pro.

dblood2 commented 8 months ago

I read through your comments, and I appreciate the detail you provided. I've pulled your changes and here are my thoughts:

The odd thing is that the reset doesn't happen on every call to setImage even though setBackgroundImage IS called. Again, I don't know if there's something afoul in my environment, or if you're seeing the same thing. I've recorded this and attached it to make sure you could see what I'm seeing.

https://github.com/MarvinKlein1508/SignaturePad/assets/16887777/e492b321-e031-4ac3-981c-e60c29c488f9

MarvinKlein1508 commented 8 months ago

@dblood2 hmm I'm not able to replicate this. Could you clear your browser cache and try again?

MarvinKlein1508 commented 8 months ago

Nevermind. I found it. Could you check again?

dblood2 commented 8 months ago

Seems to be working perfectly. Thanks again for the help.

MarvinKlein1508 commented 8 months ago

I'm going to merge this next monday and all missing demos and update github pages