freqdec / fd-slider

An Unobtrusive Accessible Slider script that can also be used as an HTML5 Input Range polyfill solution
Other
264 stars 55 forks source link

input type="reset" not supported #10

Closed fgnass closed 12 years ago

fgnass commented 12 years ago

In contrast to native sliders, fd-sliders don't get reset when <input type="reset"> is pressed.

Example: http://jsfiddle.net/fgnass/8HU6C/

freqdec commented 12 years ago

The reset event is nasty - it fires before the associated input element has its value "reset" so even if I traverse the DOM, looking for the parent form and attach an onreset handler to it, the event will fire before the input has a working value that the script could use.

I imagine It's possible to use a setTimeout in the reset handler to catch the updated input element value but that just reeks of hack.

I'm in two minds about adding code to fix this to be honest. The amount of code needed might not outweigh the benefit of adding it - I'll write a fix and see if it's not too hacky and long winded and keep you updated.

fgnass commented 12 years ago

Two ideas that might simplify the code: Instead of reading the actual value, you could reset the slider to HTMLInputEement.defaultValue ... and instead of traversing the DOM you could just use the form property. In pseudo-code:

if(inp.form) addEvent(inp.form, 'reset', function() { inp.value = inp.defaultValue; }
freqdec commented 12 years ago

...and that works like a charm everywhere but Internet Explorer - which seems to lose all recognition of the "inp" variable e.g adding the following code (1102) will always alert the id of the input element associated with the last slider/range created on the page:

if(inp.form) {                        
  addEvent(inp.form, "reset", function() { alert(inp.id); });                                
};

The same code in Firefox alerts the correct id. Shall look into it further this week to see why IE is being - well - different.

fgnass commented 12 years ago

My first guess would be that inp gets reassigned upon each iteration within some IE-only code branch. If that's the case, a quick fix would be to save the original reference in a closure:

(function(el) {
  if(el.form) {                        
    addEvent(el.form, "reset", function() { alert(el.id); });                                
  }
})(inp);
freqdec commented 12 years ago

I had already tried a closure with no success which lead me to look into the addEvent function. I'm currently using the old addEvent function created by John Resig as part of his "Flexible Javascript Events". Even John warns not to use the function though - so I've trimmed it back to the basics and all is well. Well, almost...

I still have the problem of the reset event clearing the input value completely (if the initial state of the input was undefined). This can be worked around by programmatically setting the input.defaultValue to the sliders internal default value during the slider creation phase.

Shall work on this tonight and hopefully drop a solution in a few days.

freqdec commented 12 years ago

OK - just committed a fix that, fingers crossed, works for both the polyfill & non-polyfill sliders (both text inputs and selectlists). I also fixed a bug that left the slider in the tabindex when disabled and updated the tooltip CSS with a better animation.

Be warned though: I also rewrote the CSS file to run in IE8 under IE7 compat mode. I haven't tested the result in IE6 or IE7 standalone yet so visually speaking, things might break. I shall test the changes during the week and open an issue if IE6 or IE7 are being silly.

freqdec commented 12 years ago

Forgot to say. Don't forget to follow the instructions in the CSS file to find/replace the filepath for IE.

freqdec commented 12 years ago

Just checked in IE7 standalone and, apart from a stupid mistake in an image reference (line 469 within the CSS file should say !blank - not !handleglow) which I shall fix tonight, all appears to be hunky dory.

Looks like my weekend of wrestling with various versions of IE was fruitful. If you spot no errors in the reset code, I shall close this issue (and perhaps label it as an enhancement request).

Also, thanks for your valuable input on this - If only everyone could be as helpful as you were!