search

jakubfiala / atrament

A small JS library for beautiful drawing and handwriting on the HTML Canvas.
http://fiala.space/atrament/demo
MIT License
1.55k stars 116 forks source link

Fixed refill bug & mouseup handler #24

Closed jakubfiala closed 7 years ago

jakubfiala commented 7 years ago

fixes #22 and #23

re: #22 – I thought the quickest fix was to stop the recursion altogether if we're refilling with the same color, because what's the point.

re: #23 – I removed preventDefault from the mouseup listener, as it is listening on document which renders the whole UI unusable unless atrament is running in its own iframe or something. Hopefully this won't break anything, but I can't imagine why it should.

@rubenstolk hey, I know you've contributed a lot of good stuff to this library, so I was wondering if you could look at this PR when you have a minute, just to make sure I didn't do something terrible. I'm also aware of the eslint issue, which I'm going to solve as the next step – but wanted to do so separately so the diff isn't too big on this. If you're busy, let me know, but it would be cool to get a code review!

rubenstolk commented 7 years ago

Hey @jakubfiala I think it looks good in general, I've never really used the fill feature, so didn't run into any problems there.

One question, what's the purpose of the setTimeout?

jakubfiala commented 7 years ago

@rubenstolk cheers for having a look! good question about the setTimeout, I think it ensures things don't break when you click twice rapidly or something similar. Will dig into it a bit more though, it's not an elegant solution.