foundryvtt / foundryvtt

Public issue tracking and documentation for Foundry Virtual Tabletop - software connecting RPG gamers in a shared multiplayer environment with an intuitive interface and powerful API.
https://foundryvtt.com/
234 stars 9 forks source link

Allow NumberField#_toInput's `step` to be any integer if `config.integer` is set. #11484

Closed mclemente closed 1 month ago

mclemente commented 1 month ago

What happened?

// NumberField
_toInput() {
  config.step ??= this.step;
  if ( this.integer ) {
    config.step = 1;
  }
}

What ways of accessing Foundry can you encounter this issue in?

Reproduction Steps

  1. Create a setting with the following type:
    new foundry.data.fields.NumberField({
            initial: 0,
            min: 0,
            max: 100,
            step: 10,
                        integer: true
        })
  2. Try to change the value, be it through the slider or the field.

What core version are you reporting this for?

12.329

Relevant log output

No response

Bug Checklist

JPMeehan commented 1 month ago

Seems like you should just be using step and not integer

mclemente commented 1 month ago

Then there's no guarantee the number is an integer, e.g. someone sets the setting's value to a float.

Also, I forgot to add integer to the reproduction code.

aaclayton commented 1 month ago

Assigning a step to the input has some significant UX implications that may not be desired. If you wish this behavior you can set step: 1 in your field definition. I don't think I'm in favor of this proposal.

esheyw commented 1 month ago

Would validating that the step is itself an integer not be enough? Why must it be only ever 1?

mclemente commented 1 month ago

Atropos, this is a bug report, not a feature request. I'm not requesting for FoundryVTT to set step: 1 if integer: true (see my field on Reproduction Steps), it is already doing that and I'm reporting it as a bug since it overrides my own step value to 1 instead of rounding it to the closest integer. The code in the "What Happened" section is an excerpt from NumberField#_toInput, not a suggestion.

If I create a NumberField with { integer: true, step: 10 }, its step becomes 1. I could've made it more clear on my post, but I've reported a similar issue with NumberField earlier like that (https://github.com/foundryvtt/foundryvtt/issues/11256).