ruhley / angular-color-picker

Vanilla AngularJS Color Picker Directive with no requirement on jQuery
http://ruhley.github.io/angular-color-picker/
MIT License
165 stars 78 forks source link

mousedown event may interfere with other events #43

Closed cahva closed 8 years ago

cahva commented 8 years ago

From version 0.7.0 color-picker broke my app. All other inputs were disabled on the page and color-picker could not be opened by clicking the actual input, only the swatch.

This change constrains the mousedown event to listen only the colorpicker element, not the whole document. And mousemove does not need to listen to the whole $document.

ruhley commented 8 years ago

What browser are you using. It works fine for me in Chrome, FF, IE11 and Edge. I will see if I can get a hold of older IE versions.

With my solution when you move the mouse outside the color picker it still updates, which makes it much easier to select in the corners. So I would like to keep it if possible.

cahva commented 8 years ago

I used Chrome but I dont think its browser related. Odd behavior could be from other colliding event listeners from other components in the app. Your examples worked fine btw.

Is there a downside in my change? It just did not seem right that colorpicker's mousedown event fires when clicking anywhere in the page.(?)

pe 11. joulukuuta 2015 klo 0.11 Ruhley notifications@github.com kirjoitti:

What browser are you using. It works fine for me in Chrome, FF, IE11 and Edge. I will see if I can get a hold of older IE versions.

— Reply to this email directly or view it on GitHub https://github.com/ruhley/angular-color-picker/pull/43#issuecomment-163764352 .

cahva commented 8 years ago

Ah sorry I didnt notice the corner thing. Ill check our app's other event listeners which cause this odd behaviour

cahva commented 8 years ago

Ok, found out the original problem. Actually it didn't work because the mousedown event returns false. After changing that to just return, it didn't break the app. I think returning false in event listener has special meaning so you might want to change that.

ruhley commented 8 years ago

You are right. The return false may cancel/prevent other mouse events. I have removed the return completely. Thanks for finding the problem :+1:

Fixed in commit 54bfa74cc600096d55f27b14da3eef52a44080f3 and release in v0.7.1