palantir/blueprint

Slider labels calculation for very small values leads to more than expected labels

Open

#4,679 建立於 2021年4月27日

在 GitHub 查看
 (1 留言) (0 反應) (0 負責人)TypeScript (20,263 star) (2,167 fork)batch import
P3Package: coreType: bughelp wanted

描述

Environment

  • Package version(s): 3.40.0
  • Operating System: Macos Big Sur
  • Browser name and version: Chrome 89

Code Sandbox

sandbox

Steps to reproduce

  1. Create a Slider
  2. Provide very small min, max and labelStepSize (see note below) values (you may also need to set stepSize to be able to move the handle)
  3. Let the component render
  4. Notice that there is a horizontal scrollbar in the sandbox result and labels go beyond the Example container

Note: I'm setting labelStepSize to be equal to (max-min)/9 (as seen in the sandbox code) so that the Slider generates 10 labels evenly spaced.

Actual behavior

The component renders more labels than expected:

image

Expected behavior

The component should have rendered only enough labels to reach the max value (10 labels in my use case).

Possible solution

The issue seems to be coming from the function getLabelValues

The for loop should be rewritten like this:

for (let i = min!; i < (max! + (labelStepSize ?? 1)); i += labelStepSize ?? 1) {
    values.push(i);
}

Instead of:

for (let i = min!; i < max! || Utils.approxEqual(i, max!); i += labelStepSize ?? 1) {
    values.push(i);
}

The reason is that in case of very small numbers, the approxEqual function would fail to stop the loop as the difference is lower than the default tolerance (0.00001) the function takes, and any tolerance would at some point fail given enough data points (I'm working with datasets often containing very small number, smaller than the ones I provided in the sandbox). By checking whether we reached the step after the max step we ensure we cover all cases.

貢獻者指南

Slider labels calculation for very small values leads to more than expected labels · palantir/blueprint#4679 | Good First Issue