change() fired on original input despite docs

[adamhooper]

This bug was introduced in e9af7a0

It conflicts with the documentation here: http://bgrins.github.io/spectrum/#toc36

I think the solution is to revert e9af7a0 entirely. Judging from a git blame, there's more code in spectrum that correctly fires change().

[adamhooper]

To be clear: the problem is that .spectrum('set', color) fires a "change" event.

[bgrins]

Yes, I see that. It should not be calling change, I think that was intended so that the 'change' event would fire on the input, but I'll have to look closer to see if that was fixing a different issue.

[bgrins]

So, I made a jsFiddle with this case, and I'm actually not seeing the 'change' event firing after calling set: http://jsfiddle.net/ctkY3/787/. I think I need to make the documentation more clear, but I'm thinking it is OK to fire a change event on the original input, but not to fire the change callback registered for spectrum.

Here is my current thinking, let me know if there is a reason this you think this shouldn't be the case (given that we update the docs to be more clear):

$("element").spectrum({
  change: function() {
    // This should not be fired from calling 'set'
  }
});

$("element").on("change", function() {
  // This should be OK to fire after calling 'set'
}); 

[adamhooper]

I think I start from a different mindset. (I'm using spectrum as a shim, so I want it to be as similar as possible to HTML5.)

I expect $("element").spectrum('set', color); to have the same semantics as $("element").val(color);. That, to me, makes sense: it lets me substitute one call for another.

The alternative, which spectrum espouses now, is to make $("element").spectrum('set', color) equivalent to $("element").val(color).change(). While that may seem more intuitive, it's only intuitive for those who aren't aware of how jQuery does things. To those who are comfortable with jQuery, it's a frustrating problem that will occur again and again.

A doc change would help, yes. But I want to use logic like this:

$("element").closest("form").on('change', function() {
  $(this).submit(); // send updates to server when user enters stuff
});

And to do that with Spectrum as it is now, I need to ignore change events from spectrum. Otherwise, I can't change the field to use server-supplied (or logic-supplied) values without an infinite loop. So I'd need code like:

val workingAroundSpectrum = false;
$.fn.setSpectrumColor = function() {
  val previousWorkingAroundSpectrum = workingAroundSpectrum; // support nesting
  workingAroundSpectrum = true;
  $(this).spectrum('set', color); // may fire lots of complex code
  workingAroundSpectrum = previousWorkingAroundSpectrum;
  return this;
}

$("element").closest("form").on('change', function() {
  if (workingAroundSpectrum) return;
  $(this).submit();
});

... and then I need to avoid calling $.fn.spectrum('set', ...); directly.

It's a lot of work, and I don't think it's intuitive. It's certainly different from .val(), anyway.

[bgrins]

I hear what you are saying. It doesn't map to the behavior of .val(). And there is an easier workaround for the inverse problem (expecting change to fire on the input) by simply triggering $("input").spectrum("set", "red").change();.

We do need to make sure that change does still get fired if it is an actual changed triggered through the UI (inside of updateOriginalInput. I think that if fireCallback is true here we still want to call .change(): https://github.com/bgrins/spectrum/blob/master/spectrum.js#L687

[adamhooper]

That makes sense to me.

On Tue, Jun 25, 2013 at 10:10 PM, Brian Grinstead
notifications@github.comwrote:

I hear what you are saying. It doesn't map to the behavior of .val(). And
there is an easier workaround for the inverse problem (expecting changeto fire on the input) by simply triggering $("input").spectrum("set",
"red").change();.

We do need to make sure that change does still get fired if it is an
actual changed triggered through the UI (inside of updateOriginalInput. I
think that if fireCallback is true here we still want to call .change():
https://github.com/bgrins/spectrum/blob/master/spectrum.js#L687

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/97#issuecomment-20023251
.

My Phone (mobile): +1 613 986 3339
My Website: http://adamhooper.com
My Blog: http://adamhooper.com/blog
My Twitter: http://twitter.com/adamhooper

[bgrins]

@adamhooper this should now be working as expected. Can you confirm the fix at https://github.com/bgrins/spectrum/blob/master/spectrum.js?

[adamhooper]

I'll try it out next Tuesday. Thank you for your quick action!

[bgrins]

@adamhooper have you had a chance to pull this down and test it?

[adamhooper]

Yup, works for me! Thank you for your great work.