Incorrect value decremented when step > minValue for counter Input

[rk-sf]

Summary:

When counter input set to max value, then decremented with step > minValue, first decrement is maxValue - (step - minValue) instead of just maxValue - step.
eg.
maxValue=1000
minValue=2
step=100
value=1000
decrement()
// value == 902 == 1000 - (100 - 2) == maxValue - (step - minValue)

• If you are working on the problem. Issues may be closed within 30 days if no one is assigned. Maintainers work on this project 20-30 hours per week and have their own prioritized backlog.

I'll likely not be working on the issue.

• How to duplicate a bug. Line numbers or code to paste into the documentation site examples is helpful.

Reproducible from the example page: https://react.lightningdesignsystem.com/components/inputs/

1. Navigate to section containing: "3. CONTROLLED COUNTER INPUT WITH MIN/MAX VALUES AND CUSTOM STEP SIZE"
2. Modify Example Code
3. Update counter-input-3 with

   <Input
       id="counter-input-3"
       label="My Label"
       minValue={2}
       maxValue={1000}
       onChange={(event, data) => {
           this.setState({ counter3value: data.value });
       }}
       step={100}
       value={this.state.counter3value}
       variant="counter"
   />

4. Navigate back to the relevant input.
5. Set value to 1000
6. Decrement
7. Note resulting value is 902 instead of 900

• If you are a Salesforce employee. You should also create a duplicate work item in your internal ticket system and link to this issue.

I'll go ahead and add a ticket

[welcome]

Thanks for opening your first issue! 👋
If you have found this library helpful, please star it. A maintainer will try to respond within 7 days. If you haven’t heard anything by then, please bump this thread.

[interactivellama]

Thanks @rk-sf for such a detailed issue. I'll add this to our backlog.

[kevinparkerson]

Been a while since I touched this component, but after some investigation I realized this is actually as-designed (though I admit it does seem weird). When I originally added the counter feature to input, I made sure to have it mirror the exact behavior of an HTML input type="number" element. The behavior reported in this issue is actually consistent with how it's handled in the browser, as can be seen in this fiddle:

https://jsfiddle.net/sxj6kpm8/

Basically, when handling minimum values that aren't multiples of the step size the browser decides to adjust the integer value immediately upon increment or decrement to be a multiple of the minimum value. Weirdly enough, it doesn't do anything at all if the maximum value isn't a multiple of the step size. This isn't the behavior I'd likely have chosen, but it's consistent across all browsers so I made sure this component was in alignment.

Even though this wasn't an actual bug due to odd browser implementation of inputs, good eye for pointing this out @rk-sf! Perhaps I should add a comment in the source code to highlight this peculiarity... what do you think @interactivellama?

[interactivellama]

Yes, please highlight this quirk for future consumers. I suggest adding it to the component overview comment above the class definition.

[interactivellama]

@kevinparkerson The counter variant is not a controlled input, it's uncontrolled.

If we had an onIncrement callback, then the consumer could change the number themselves, the other option would be a performStep callback that guts the logic on the component.

[interactivellama]

@rk-sf If we added a callback to override the internal logic of the control, would you use the counter?

Pseudo-code of proposal:

<Input
onRequestStepIncrement ( event, { numberTheCounterThinksItShouldBe: number, direction: string [up, down] } ) {
// ...custom logic
setState({ inputValue: customValue})
}
value={this.state.inputValue} />

[rk-sf]

@interactivellama I definitely would have used such an event if it were available at the time. Thanks for looking into it everyone!

[kevinparkerson]

@interactivellama I like that idea, allows for custom behavior if the default is not sufficient 👍