Skip to content

Commit

Permalink
refactor logic to single function
Browse files Browse the repository at this point in the history
  • Loading branch information
@brianlund-wandb
brianlund-wandb committed Dec 17, 2024
1 parent 01b7661 commit c9a250f
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 23 deletions.
71 changes: 63 additions & 8 deletions weave-js/src/common/components/elements/SliderInput.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,51 +1,104 @@
import {getClosestTick} from './SliderInput';

describe('getClosestTick', () => {
test('value within range, no ticks', () => {
const ticks = undefined;
const min = 1;
const max = 10;
const previous = 4;
const val = 3;
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(3);
});
test('lower below min coerced to min', () => {
const ticks = [2, 4, 6, 8, 10];
const min = ticks[0];
const max = ticks[ticks.length - 1];
const previous = 2;
const val = 1;
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(2);
});
// not convinced this is a realistic test
test.skip('upper above non-inclusive max', () => {
const ticks = [2, 4, 6, 8, 10];
const min = ticks[0];
const max = 12;
const previous = 10;
const val = 12;
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(10);
});
test('upper above max coerced to max', () => {
const ticks = [2, 4, 6, 8, 10];
const min = ticks[0];
const max = ticks[ticks.length - 1];
const previous = 10;
const val = 11;
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(10);
});
test('lower previous value returns next greater', () => {
const ticks = [2, 4, 6, 8, 10];
const min = ticks[0];
const max = ticks[ticks.length - 1];
const previous = 4;
const val = 5;
expect(getClosestTick(ticks, val, previous)).toBe(6);
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(6);
});
test('lower previous value returns next greater, large step', () => {
const ticks = [2, 4, 60, 80, 10];
const min = ticks[0];
const max = ticks[ticks.length - 1];
const previous = 4;
const val = 5;
expect(getClosestTick(ticks, val, previous)).toBe(60);
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(60);
});
test('greater previous value returns next lesser', () => {
const ticks = [2, 4, 6, 8, 10];
const min = ticks[0];
const max = ticks[ticks.length - 1];
const previous = 4;
const val = 3;
const actual = getClosestTick(ticks, val, previous);
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(2);
});
test('greater previous value returns next lesser, consecutive', () => {
const ticks = [1, 2, 3, 4, 5, 6];
const min = ticks[0];
const max = ticks[ticks.length - 1];
const previous = 4;
const val = 3;
const actual = getClosestTick(ticks, val, previous);
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(3);
});
test('lower previous value returns next greater, consecutive', () => {
const ticks = [1, 2, 3, 4, 5, 6];
const min = ticks[0];
const max = ticks[ticks.length - 1];
const previous = 3;
const val = 4;
const actual = getClosestTick(ticks, val, previous);
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(4);
});
test('lower previous value returns next greater, erratic', () => {
const ticks = [1, 4, 5, 7, 9, 12];
const min = ticks[0];
const max = ticks[ticks.length - 1];
const previous = 9;
const val = 10;
const actual = getClosestTick(ticks, val, previous);
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(12);
});
test('greater previous value returns next lower, erratic', () => {
const ticks = [1, 2, 5, 7, 9, 12];
const min = ticks[0];
const max = ticks[ticks.length - 1];
const previous = 5;
const val = 4;
const actual = getClosestTick(ticks, val, previous);
const actual = getClosestTick(val, previous, min, max, ticks);
expect(actual).toBe(2);
});

Expand All @@ -56,10 +109,12 @@ describe('getClosestTick', () => {
ticks.push(i);
}

const min = ticks[0];
const max = ticks[ticks.length - 1];
const previous = 500000;
const val = 500001;
const start = Date.now();
const actual = getClosestTick(ticks, val, previous);
const actual = getClosestTick(val, previous, min, max, ticks);
const end = Date.now();
const duration = end - start;
expect(actual).toBe(500002);
Expand Down
39 changes: 24 additions & 15 deletions weave-js/src/common/components/elements/SliderInput.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export interface SliderInputProps {
disabled?: boolean;
strideLength?: number;
// if true, the slider will be restricted to props.max, but the input will be unbounded (https://wandb.atlassian.net/browse/WB-5666)
allowGreaterThanMax?: boolean;
// allowGreaterThanMax?: boolean;
onChange(value: number): void;
}

Expand All @@ -47,7 +47,7 @@ const SliderInput: React.FC<SliderInputProps> = React.memo(
ticks,
disabled,
strideLength,
allowGreaterThanMax,
// allowGreaterThanMax,
onChange,
}) => {
const [sliderValue, setSliderValue] = React.useState(value ?? 0);
Expand All @@ -71,19 +71,11 @@ const SliderInput: React.FC<SliderInputProps> = React.memo(
if (newVal == null || !_.isFinite(newVal)) {
return;
}
if (newVal > max && !allowGreaterThanMax) {
newVal = max;
}
if (newVal < min) {
newVal = min;
}
if (ticks != null) {
newVal = getClosestTick(ticks, newVal, sliderValue);
}
newVal = getClosestTick(newVal, sliderValue, min, max, ticks);
setSliderValue(newVal);
onChangeDebounced(newVal);
},
[ticks, min, max, allowGreaterThanMax, sliderValue, onChangeDebounced]
[ticks, min, max, sliderValue, onChangeDebounced]
);

React.useEffect(() => {
Expand Down Expand Up @@ -138,7 +130,7 @@ const SliderInput: React.FC<SliderInputProps> = React.memo(
strideLength={strideLength}
disabled={disabled ?? false}
min={min}
max={allowGreaterThanMax ? undefined : max}
max={max}
value={value}
ticks={ticks}
onChange={update}
Expand Down Expand Up @@ -173,10 +165,27 @@ const SliderInput: React.FC<SliderInputProps> = React.memo(
export default SliderInput;

export function getClosestTick(
ticks: number[],
val: number,
prev: number
prev: number,
min: number,
max: number,
ticks?: number[]
): number {
// if min/max not in ticks, allow coercion to nearest value
if (val > max) {
return max;
}
if (val < min) {
return min;
}
if (ticks === null || ticks === undefined) {
return val;
}
if (ticks.includes(val)) {
// happy path, avoid computation
return val;
}

let closest = val;
const increasing = val > prev;
let minDiff = Number.MAX_VALUE;
Expand Down

0 comments on commit c9a250f

Please sign in to comment.