PitPik / colorPicker

Advanced javaScript color picker and color conversion / calculation (rgb, hsv, hsl, hex, cmyk, cmy, XYZ, Lab, alpha, WCAG 2.0, ...)
http://www.dematte.at/colorPicker/
MIT License
570 stars 136 forks source link

Weird bugs in jQuery implementation #12

Closed ghost closed 9 years ago

ghost commented 9 years ago

If I write this:

$('input.color').colorPicker();

colorPicker throws an error (Cannot read property 'customBG' of undefined (looks like it always require a config object)). If I pass any object, even

$('input.color').colorPicker({});

It throws another error (undefined is not a function). This points to function "convertColors", at this line (217 in colors.js file):

colors[typ] = convert[from + '2' + typ](colors[from]);

What should we do??

PitPik commented 9 years ago

Hi @ct-js

thanks, great finding. I was never spending enough time for the implementations,... so I guess this is the result ;o) Thanks again, I committed the fix already.

ghost commented 9 years ago

@PitPik , D:

TypeError: Cannot read property 'nodes' of undefined

This one appeared when I was trying to pick a color, just after clicking on color field.

Also I noticed that colorPicker is displayed in wrong place if it was fired on absolutely positioned elements, or on an element inside a block with position: relative|absolute styling. So colorPicker works with parent's local coordinates, but displayed at absolute ones.

ghost commented 9 years ago

Btw, how can we embed such a cute color ring displayed on http://www.dematte.at/cpn/ ? :3

PitPik commented 9 years ago

@ct-js The positioning bug is fixed... do you have a jsfiddle account? Could you set up a situation there so I can see what's going wrong. Here, everything seems to be fine when I'm trying to pick a color. For the ring,... just look at the source code of index.js

ghost commented 9 years ago

Well, it works fine on codepen.io... I can pick colors on it, but colorPicker doesn't work in my nw.js app. Maybe there are some node-like variables like fs, md, path, process, events etc. in source code? pen http://codepen.io/anon/pen/YPxBqL

I'm going to test colorPicker with its uncompressed version; will reply with error placement soon...

ghost commented 9 years ago

Now it doesn't even create colorPicker form T___T Used latest version

TypeError: Cannot read property 'getElementsByTagName' of undefined
    at getInstanceNodes (file:///D:/ct/build/app/js/main.js:1088:24) //  <--- this is its first line: colorPicker == undefined
    at initInstance (file:///D:/ct/build/app/js/main.js:928:25) // THIS.nodes = _nodes = getInstanceNodes(buildView(THIS), THIS);
    at new ColorPicker (file:///D:/ct/build/app/js/main.js:810:4) // the end of ColorPicker constructor: initInstance(this, options || {});
    at createInstance (file:///D:/ct/build/app/js/main.js:2198:13) // return new initConfig.klass(initConfig);
    at HTMLInputElement.<anonymous> (file:///D:/ct/build/app/js/main.js:2208:32)
    at HTMLInputElement.n.event.dispatch (file:///D:/ct/build/app/js/main.js:9:6444)
    at HTMLInputElement.r.handle (file:///D:/ct/build/app/js/main.js:9:3219)
PitPik commented 9 years ago

@ct-js Now I see your problem (...took me a while to find out): You probably forgot to load colorPicker.data.js. I think I made this a bit to confusing or it is just a lack of documentation (sorry for the confusion)... If you run in single file mode (or source file mode), you have to load ./colors.js (the color module), ./colorPicker.data.js (the HTML, CSS and images for the UI), ./colorPicker.js (the UI itself)... ...or ./color.all.min.js instead that contains all three files. The implementation file jQuery_implementation/jqColor.js must then be loaded to be able to run it with jQuery. jQuery_implementation/jQueryColorPicker.min.js contains again all four files... Let me know how this works out for you. Cheers

PitPik commented 9 years ago

BTW: the color circle on the demo page:

ghost commented 9 years ago

Well, this error disappeared, but I have Cannot read property 'nodes' of undefined again.

the color circle on the demo page

I meant the whole circular colorPicker :) It is really useful for painters to use such color pickers rather than rectangular ones. Color theory, triads, etc ;) Maybe there will be another colorPicker layout type? :)

PitPik commented 9 years ago

... in colorPicker.js there is 5 spots where I use .node. It is hard to 'guess' an error if I don't see the code or at least some more error information. Maybe you can get me more information?? BTW: are you also using AngularJS?

ghost commented 9 years ago

Maybe you can get me more information??

I'll try to...

BTW: are you also using AngularJS?

:mask: No, I'm not. JS + jQuery + Node.js + nouislider

PitPik commented 9 years ago

Hey, I just updated colorPicker.js (and all minified files). Maybe you want to try them first. It might bring you a solution.

PitPik commented 9 years ago

@ct-js Maybe http://www.dematte.at/cpn2/ would do it for you as well. I guess you don't need all the functionalities of colorPicker (it's actually great for more complex color manipulation, etc.). The other one will be only about 4KB all together and still has a rich and nice color model (works with a subset of colors.js)... What do you think? I'll be finishing it in a few days, the you can try to implement it into your project.

ghost commented 9 years ago

Trying old version. You are too fast :D So, I take

And concatenate them in exactly this way. The error points to the doEventListeners function found in jqColor.js file, exactly here.

The reason is that there are no cP in colorPickers array (it's empty as array but has evt: true), so the current one can't be retrieved. Besides that, it seems that no arguments except elm were specified.

Scope variables

PitPik commented 9 years ago

@ct-js I'm too fast :D that's funny! I just committed a new version of jqColor.js. Maybe that does it then...

ghost commented 9 years ago

Off:

    _html = '<div class="cp-color-picker"><div class="cp-z-slider"><div c' +
            'lass="cp-z-cursor"></div></div><div class="cp-xy-slider"><div cl' +
            'ass="cp-white"></div><div class="cp-xy-cursor"></div></div><div ' +
            'class="cp-alpha"><div class="cp-alpha-cursor"></div></div><div c' +
            'lass="cp-display"></div></div>'

Optimization? Which optimization?

:D String concatenation is a complex (and long) process for computer, you would better just escape line breaks, like this:

str = 'Olala, what a long \
string!';
ghost commented 9 years ago

I just committed a new version of jqColor.js. Maybe that does it then...

IT WORKS!!! :confetti_ball: :confetti_ball: :confetti_ball: :confetti_ball: :confetti_ball:

Thank yoooouuuu ^___^

PitPik commented 9 years ago

:D I know... this is work in progress... the HTML will go anyhow ;o) for now I just want it to work... then there will be optimization...

PitPik commented 9 years ago

Oh, great to hear that it finally works... If you find more issues, then let me know. I assume I can close this issue then. Cheers and good luck!!

ghost commented 9 years ago

V2: I guess _css variable should create a style tag, but it doesn't, so without it it is just a pile of invisible blocks. Maybe it is just WIP ^-^ But with css it works great for me too.

Thank you again :3 Truthly, your tool is one of two (another one is a small bootstrap component) well-looking and functioning colorPickers in the world for today. And your is more cool :sunglasses:

Yeah, the issue can be closed. (hooray!)

PitPik commented 9 years ago

@ct-js Hi again. The new one is onlin: https://github.com/PitPik/tinyColorPicker