EastDesire / jscolor

JavaScript color picker with opacity (alpha channel) and customizable palette. Single file of plain JS with no dependencies.
http://jscolor.com
232 stars 72 forks source link

Click and drag handlers not registered in module mode #25

Closed brendon closed 3 years ago

brendon commented 3 years ago

Hi @EastDesire, I've been reorganising my code in Webpack and am dynamically fetching javascript from the server whenever particular elements appear on the page. One of these is a colour picker controller (via stimulus.js).

https://webpack.js.org/guides/lazy-loading/ https://github.com/danieldiekmeier/stimulus-controller-resolver

For some reason when jscolor is loaded in this way it doesn't register the click and drag handlers on the control. There are also no errors emitted. When I import jscolor in the main javascript pack it then works fine. I think there's an incompatibility with the way you're registering those handlers.

The colour palate shows up for selection, and is cross haired on the right place if an existing colour is supplied, but it's unable to be changed via clicking.

Just quickly checking if you have any ideas on what the problem might be.

brendon commented 3 years ago

Just to follow up. The code that causes the problem is here:

for (var r = 0; r < pickerDims.palette.rows; r++) {
  for (var c = 0; c < pickerDims.palette.cols && si < THIS._palette.length; c++, si++) {

pickerDims.palette.rows is 0 in my case"=:

image

Going further, getPaletteDims is failing on a missing thisObj._palette:

var sampleCount = thisObj._palette ? thisObj._palette.length : 0;

So it looks like this code is never called

        this.set__palette = function (val) {
            this.palette = val;
            this._palette = jsc.parsePaletteValue(val);
            this._paletteHasTransparency = jsc.containsTranparentColor(this._palette);
        };
brendon commented 3 years ago

Finally (last spam message!), I kick off my pickers like so:

new JSColor(this.element, {

I thought it was using JSColor from: import JSColor from '@eastdesire/jscolor' but if I remove this JSColor still exists on the window anyway, so now I'm just importing import '@eastdesire/jscolor' in my webpack entry point.

I think there remains quite a bit of work to seperate the code from dependency on the window object :)

EastDesire commented 3 years ago

Hi @brendon

I think there remains quite a bit of work to seperate the code from dependency on the window object :)

Yes, still much to be improved there.

Going through your messages and the quoted lines of code, I can't see any problem there. The two for loops won't get executed if pickerDims.palette.rows is 0. There shouldn't be any need for thisObj._palette to exist. If it doesn't exist, then sampleCount will be set to 0 using the last part of ternary expression thisObj._palette ? thisObj._palette.length : 0;

Are you getting any specific error messages at that point, or why do you think there is a problem with that line? I'd need to see how your code behaves in a browser in order to troubleshoot this, but I guess that's not possible, is it?

brendon commented 3 years ago

Hi @EastDesire, unfortunately it'll be a bit of work to reproduce the issue in a test environment. The main issue is that with pickerDims.palette.rows being 0, the click event handler on the palette is never added (because it's within that loop) so when the picker is shown, it's visible and correctly drawn, but none of its interactive nature works. This also applies to the darkness slider.

For now I'm treating it like a jQuery plugin and just loading it once in the main entry point and assuming it exists.

EastDesire commented 3 years ago

As it is now, I think loading it just once is the best approach.

Just for explanation, "palette" is called the color swatch at the bottom. So pickerDims.palette.rows = 0 means there are no rows in the palette (swatch), so there is no swatch to show and no need for its event handlers.

But how come the visible spectrum and darkness slider don't work is hard to guess )-: What might be happening is that jscolor.init() is never called because it could not be attached to document's DOMContentLoaded event. Can you try to call jscolor.init() manually at some point when window is available, just to see if it works?

brendon commented 3 years ago

Haha, yes that was a red-herring wasn't it!

Your suggestion worked! Here's what I'm doing with the stimulus controller:

import '@eastdesire/jscolor'

import { Controller } from 'stimulus'

export default class extends Controller {

  initialize() {
    JSColor.init()
  }

initialize runs just once when the controller is first used so this works well. My controllers are lazy-loaded so JSColor doesn't get downloaded until a colour picker controller appears on the screen.

Thank you so much for your suggestion. Slightly embarrassing for me that it was so simple :D