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

[BUG] When cancel Colorpicker, it set wrong value for component #2683

Closed
longdoan7421 opened this issue Mar 25, 2020 · 2 comments
Closed

[BUG] When cancel Colorpicker, it set wrong value for component #2683

longdoan7421 opened this issue Mar 25, 2020 · 2 comments

Comments

@longdoan7421
Copy link
Contributor

longdoan7421 commented Mar 25, 2020

  1. Are you using the latest release (older versions are NOT supported)?
    Yes.

  2. Are you facing the bug with your local copy of GrapesJS or with the current demo?
    Both of them.

  3. Steps to reproduce:

    • Drag a component then change its background color from input
    • Open color picker immediately
    • Then click to outside color picker (anywhere except canvas)
  4. What is the expected behavior?
    Background color should keep remain.

  5. What happens instead?
    Background color change to previous color or #000000.

  6. Screencast

    grapesjs-backgroundcolor

@longdoan7421 longdoan7421 changed the title [BUG] When cancel Colorpicker, it set wrong value for component which has background color is "none"1 [BUG] When cancel Colorpicker, it set wrong value for component which has background color is "none" Mar 25, 2020
@longdoan7421
Copy link
Contributor Author

longdoan7421 commented Mar 25, 2020

Hi @artf,
After seeing around the colorpicker, I think the problem is that the spectrum doesn't have the same value as model when user changes input.
So I attempted to sync the value of spectrum and model, and it could fix the issue. Here is my solution:

// InputColor.js
  /**
   * Set value to the model
   * @param {string} val
   * @param {Object} opts
   */
  setValue(val, opts = {}) {
    const model = this.model;
    const def = model.get('defaults');
    const value = !isUndefined(val) ? val : !isUndefined(def) ? def : '';
    const inputEl = this.getInputEl();
    const colorEl = this.getColorEl();
    const valueClr = value != 'none' ? value : '';
    inputEl.value = value;
    colorEl.get(0).style.backgroundColor = valueClr;

    // This prevents from adding multiple thumbs in spectrum
    if (opts.fromTarget || (opts.fromInput && !opts.avoidStore)) { // when input changed, update value in spectrum
      colorEl.spectrum('set', valueClr);
      this.noneColor = value == 'none';
    }
  },

What do you think about this? If it's ok, I could submit a PR.

Thank you.

@longdoan7421 longdoan7421 changed the title [BUG] When cancel Colorpicker, it set wrong value for component which has background color is "none" [BUG] When cancel Colorpicker, it set wrong value for component Mar 25, 2020
@longdoan7421
Copy link
Contributor Author

I found out that this bug also happen if you change the input with any value, then immediately open colorpick then cancel it by clicking outside (not canvas).

longdoan7421 pushed a commit to longdoan7421/grapesjs that referenced this issue Apr 1, 2020
@artf artf closed this as completed in 2fb6b3a Apr 16, 2020
artf added a commit that referenced this issue Apr 16, 2020
Update color picker value when user changes value from input. Fix #2683
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

1 participant