Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Add setColor(string) method #91

Closed
melloware opened this issue Mar 7, 2023 · 9 comments
Closed

Feature Request: Add setColor(string) method #91

melloware opened this issue Mar 7, 2023 · 9 comments

Comments

@melloware
Copy link
Contributor

Add a first class method to set the color of an instance similar to..

 /**
   * Sets the active color and fires all necessary events.
   * @param {string} [color] Color value to override the active color.
   * @param {HTMLelement} [target] the element setting the color on
   */
  function setColor(color, target) {
    currentEl = target;
    oldColor = currentEl.value;
    attachVirtualInstance(target);
    currentFormat = getColorFormatFromStr(color);
    updatePickerPosition();
    setColorFromStr(color);
    pickColor();
    if (oldColor !== color) {
       currentEl.dispatchEvent(new Event('change', { bubbles: true }));
    }
  }
@mdbassit
Copy link
Owner

When you say an instance, do you mean a specific input field or a group of fields that use the same Coloris instance?

@melloware
Copy link
Contributor Author

Its the specific input I am trying to set the color for or "instance"

Coloris.setColor(newColor, input[0]);

@melloware
Copy link
Contributor Author

melloware commented Mar 10, 2023

Here is what I really do so it works for popup mode or inline mode.

/**
      * Sets the current color
      * @param {string} color the color to set
      */
    setColor: function(color) {
        if (!color) {
            return;
        }
        var newColor = color.toLowerCase();
        var input = this.popup ? this.input : this.jq.find('#clr-color-value');
        Coloris.setColor(newColor, input[0]);
    },

@mdbassit
Copy link
Owner

Coloris.setColor(newColor, input[0]);

I think a better and more efficient way is to just set the value of the target fields directly and then trigger an input (or change for #clr-color-value) event. You can replace the Coloris.setColor() method with this:

function setColor(color, target) {
  const colorValue = document.querySelector('#clr-color-value');
  // Color fields listen for an "input" events while the color value field
  // within the picker listens for a "change" event
  const event = target === colorValue ? 'change' : 'input';

  target.value = color;
  target.dispatchEvent(new Event(event, { bubbles: true }));
}

@mdbassit
Copy link
Owner

Here is what I really do so it works for popup mode or inline mode.

Or even better, here is a rewrite of your code above:

/**
  * Sets the current color
  * @param {string} color the color to set
  */
setColor: function(color) {
    if (!color) {
        return;
    }

    var newColor = color.toLowerCase();
    var input = this.popup ? this.input : this.jq.find('#clr-color-value');
    var event = this.popup ? 'input' : 'change';
    var target = input[0];
    
    target.value = color;
    target.dispatchEvent(new Event(event, { bubbles: true }));
},

@melloware
Copy link
Contributor Author

This is pretty close. The only difference is with mine because our wrapper send AJAX events on change of color I need the change event fired for either popup or inline and your input method does not fire the change.

This was vital for us so the change event fires for inline or popup and ONLY fires if the color actually changes.

if (oldColor !== color) {
       currentEl.dispatchEvent(new Event('change', { bubbles: true }));
    }

@mdbassit
Copy link
Owner

How about this:

/**
  * Sets the current color
  * @param {string} color the color to set
  */
setColor: function(color) {
    if (!color) {
        return;
    }

    var newColor = color.toLowerCase();
    var input = this.popup ? this.input : this.jq.find('#clr-color-value');
    var target = input[0];
    var oldColor = target.value.toLowerCase();
    var triggerInputEvent = !!this.popup;
    
    if (color.toLowerCase() !== oldColor) {
        target.value = color;
        triggerInputEvent && target.dispatchEvent(new Event('input', { bubbles: true }));
        target.dispatchEvent(new Event('change', { bubbles: true }));
    }
},

@melloware
Copy link
Contributor Author

This seems to work!

Repository owner deleted a comment from momadic Mar 20, 2023
@mdbassit
Copy link
Owner

Should I close this then?

@melloware melloware closed this as not planned Won't fix, can't repro, duplicate, stale Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants