Skip to content

Commit

Permalink
fix(number-input): clamp mouse movements as well when value is clamped (
Browse files Browse the repository at this point in the history
#6331)

**Problem:**
Today when scrubbing the value on mouse movement we still keep track of
the original movement delta, causing an issue where it's hard to return
after passing the thresholds

<video
src="https://github.com/user-attachments/assets/4298be75-4449-45dd-a961-df89094300d7"></video>

**Fix:**
Keep track of the clamped mouse movement as well. This required adding a
way to invert our `calculateDragDelta` function, in order to retrieve
the "clamped" mouse movements after clamping the total value.

<video
src="https://github.com/user-attachments/assets/d7005f59-b12b-46d8-9a3d-9fad3961e717"></video>

**Manual Tests:**
I hereby swear that:

- [X] I opened a hydrogen project and it loaded
- [X] I could navigate to various routes in Preview mode

Fixes #6299
  • Loading branch information
liady authored Sep 6, 2024
1 parent 536b9c0 commit 44734d8
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 6 deletions.
21 changes: 21 additions & 0 deletions editor/src/uuiui/inputs/number-input.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { calculateDragDirectionDelta } from './number-input'

describe('number input', () => {
it('calculateDragDirectionDelta should be able to invert its results', () => {
const testCases = [
[5, 2],
[2, 5],
[-5.5, 2],
[-2, 5],
[0, 4],
[3.2, 1],
[-6, 3],
[-3, 6],
]

testCases.forEach(([delta, scaling]) => {
const { result, inverse } = calculateDragDirectionDelta(delta, scaling)
expect(inverse(result)).toEqual(delta)
})
})
})
36 changes: 30 additions & 6 deletions editor/src/uuiui/inputs/number-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,34 @@ function dragDeltaSign(delta: number): 1 | -1 {
return delta >= 0 ? 1 : -1
}

function calculateDragDirectionDelta(delta: number, scalingFactor: number): number {
export function calculateDragDirectionDelta(
delta: number,
scalingFactor: number,
): {
result: number
inverse: (value: number) => number
} {
const sign = dragDeltaSign(delta)
const rawAbsDelta = Math.abs(delta)
// Floor the value and then restore its sign so that it is rounded towards zero.
const scaledAbsDelta = Math.floor(rawAbsDelta / scalingFactor)
return sign * scaledAbsDelta
// save the diff for inverse calculation
const diff = rawAbsDelta - scaledAbsDelta * scalingFactor
return {
result: sign * scaledAbsDelta,
inverse: (value: number) => {
return sign * (Math.abs(value) * scalingFactor + diff)
},
}
}

function calculateDragDelta(delta: number, scalingFactor: number = 2): number {
function calculateDragDelta(
delta: number,
scalingFactor: number = 2,
): {
result: number
inverse: (value: number) => number
} {
return calculateDragDirectionDelta(delta, scalingFactor)
}

Expand Down Expand Up @@ -230,6 +249,7 @@ export const NumberInput = React.memo<NumberInputProps>(
const pointerOriginRef = React.useRef<HTMLDivElement>(null)

const accumulatedMouseDeltaX = React.useRef(0)
const clampedAccumulatedDelta = React.useRef(0)
// This is here to alleviate a circular reference issue that I stumbled into with the callbacks,
// it means that the cleanup callback isn't dependent on the event listeners, which result in
// a break in the circle.
Expand Down Expand Up @@ -306,14 +326,16 @@ export const NumberInput = React.memo<NumberInputProps>(

const setScrubValue = React.useCallback(
(transient: boolean) => {
const dragDelta = calculateDragDelta(accumulatedMouseDeltaX.current)
if (valueAtDragOrigin.current != null) {
const numericValue = clampValue(
const { result: dragDelta, inverse } = calculateDragDelta(clampedAccumulatedDelta.current)
const totalClampedValue = clampValue(
valueAtDragOrigin.current + stepSize * dragDelta,
minimum,
maximum,
)
const newValue = cssNumber(numericValue, valueUnit)
const clampedDelta = (totalClampedValue - valueAtDragOrigin.current) / stepSize
clampedAccumulatedDelta.current = inverse(clampedDelta)
const newValue = cssNumber(totalClampedValue, valueUnit)

if (transient) {
if (onTransientSubmitValue != null) {
Expand Down Expand Up @@ -403,6 +425,7 @@ export const NumberInput = React.memo<NumberInputProps>(
// Apply the movement to the accumulated delta, as the movement is
// relative to the last event.
accumulatedMouseDeltaX.current += e.movementX
clampedAccumulatedDelta.current += e.movementX

onThresholdPassed(e, () => {
if (!scrubThresholdPassed.current) {
Expand Down Expand Up @@ -632,6 +655,7 @@ export const NumberInput = React.memo<NumberInputProps>(
setDragOriginY(e.pageY)
setGlobalCursor?.(CSSCursor.ResizeEW)
accumulatedMouseDeltaX.current = 0
clampedAccumulatedDelta.current = 0
}
},
[
Expand Down

0 comments on commit 44734d8

Please sign in to comment.