googlefonts / fontra

A browser-based font editor
https://fontra.xyz
GNU General Public License v3.0
494 stars 21 forks source link

[range-slider] Numeric input does not get clamped when setting value #1206

Open justvanrossum opened 6 months ago

justvanrossum commented 6 months ago

If a range-slider has a minimum value of 100, setting it to a value lower than that will have the numeric field happily display the lower value. It should be clamped so it matches the slider value.

ollimeier commented 6 months ago

Hi @justvanrossum, has this been fixed meanwhile? Because I cannot reproduce it:

https://github.com/googlefonts/fontra/assets/21055547/0e32a916-ba8d-494b-a94d-149e65952e08

justvanrossum commented 6 months ago

Ah, I see my description is not quite clear. It is about programmatic setting of the value: if you programmatically set the value to an out-of-range value, the numeric field happily displays it, while the slider is (obviously) clamped.

ollimeier commented 6 months ago

I am very sorry, but I still don't get it. What do you mean with programmatically set the value?

justvanrossum commented 6 months ago

"Programmatically" means that a program sets the value, not a person. Code that sets the value of the range-slider as opposed to a person typing a number or dragging the slider.

ollimeier commented 6 months ago

The only thing I can imagine, would be to solve it via some code like this:

const inputs = document.getElementsByTagName('input');
for (const el of inputs){
  if (el.type != 'number'){
    continue;
  }
  if (el.min){
    if (parseFloat(el.value) < parseFloat(el.min)) {
      el.value = el.min;
    }
  }
  if (el.max){
    if (parseFloat(el.value) > parseFloat(el.max)){
      el.value = el.max;
    }
  }
}
ollimeier commented 6 months ago

@justvanrossum I am wondering how a unittest would look like for such a situation? Any idea?

justvanrossum commented 6 months ago

We currently can't unittest UI elements.

I rather think the fix will be part of the valueFormatted() method

https://github.com/googlefonts/fontra/blob/e7dc9613327216c8ae711937140db7e049e6b816/src/fontra/client/web-components/range-slider.js#L186-L190

To test I think we need to construct a URL with an out-of-range axis value. Check the urlfragment.py module to assemble/disassemble Fontra URL fragments.

justvanrossum commented 3 months ago

Coming back here to register this observation: this "bug" has occasionally been useful for detecting bad data. Perhaps we should leave it like it is: if there are out-of-range values, we can at least see them in the numeric input as such.