From fd3a8cec35bbcf00dff12ff6534da24f8376b960 Mon Sep 17 00:00:00 2001 From: Aria Minaei Date: Mon, 2 Jan 2023 22:10:55 +0100 Subject: [PATCH] Nudging numbers will now use `nudgeMultiplier` even if the number has a range Possibly fixes #314 --- theatre/core/src/propTypes/index.ts | 34 +++++++++++++++---- .../uiComponents/form/BasicNumberInput.tsx | 11 +++--- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/theatre/core/src/propTypes/index.ts b/theatre/core/src/propTypes/index.ts index e88cc23..fbb16cc 100644 --- a/theatre/core/src/propTypes/index.ts +++ b/theatre/core/src/propTypes/index.ts @@ -326,7 +326,9 @@ export const number = ( label: opts.label, nudgeFn: opts.nudgeFn ?? defaultNumberNudgeFn, nudgeMultiplier: - typeof opts.nudgeMultiplier === 'number' ? opts.nudgeMultiplier : 1, + typeof opts.nudgeMultiplier === 'number' + ? opts.nudgeMultiplier + : undefined, interpolate: _interpolateNumber, deserializeAndSanitize: numberDeserializer(opts.range), } @@ -703,7 +705,10 @@ export interface PropTypeConfig_Number extends ISimplePropType<'number', number> { range?: [min: number, max: number] nudgeFn: NumberNudgeFn - nudgeMultiplier: number + /** + * See {@link defaultNumberNudgeFn} to see how `nudgeMultiplier` is treated. + */ + nudgeMultiplier: number | undefined } export type NumberNudgeFn = (p: { @@ -713,6 +718,16 @@ export type NumberNudgeFn = (p: { config: PropTypeConfig_Number }) => number +/** + * This is the default nudging behavior. It'll be used if `config.nudgeFn` is empty in {@link number} `types.number(defaultValue, config)`. + * + * Its behavior is as follows: + * - If `config.nudgeMultiplier` is set, then it'll be used as the unit of incrementing/decrementing the prop's value. + * For example, if `types.number(0, {nudgeMultiplier: 0.5})`, then nudging the number will make its value go up/down by 0.5, so: 0, 0.5, 1.0, -0.5, ... + * Note that if the prop's value is, say, 0.1, then nudging it will still make its value go up/down by 0.5, so: 0.6, 1.1, -0.6, ... + * - Otherwise, the amount of nudge will be determined based on whether the number has a range. + * + */ const defaultNumberNudgeFn: NumberNudgeFn = ({ config, deltaX, @@ -720,13 +735,18 @@ const defaultNumberNudgeFn: NumberNudgeFn = ({ magnitude, }) => { const {range} = config - if (range && !range.includes(Infinity) && !range.includes(-Infinity)) { - return ( - deltaFraction * (range[1] - range[0]) * magnitude * config.nudgeMultiplier - ) + console.log(deltaX, deltaFraction, config) + + if ( + !config.nudgeMultiplier && + range && + !range.includes(Infinity) && + !range.includes(-Infinity) + ) { + return deltaFraction * (range[1] - range[0]) * magnitude } - return deltaX * magnitude * config.nudgeMultiplier + return deltaX * magnitude * (config.nudgeMultiplier ?? 1) } export interface PropTypeConfig_Boolean diff --git a/theatre/studio/src/uiComponents/form/BasicNumberInput.tsx b/theatre/studio/src/uiComponents/form/BasicNumberInput.tsx index af4a86e..8398427 100644 --- a/theatre/studio/src/uiComponents/form/BasicNumberInput.tsx +++ b/theatre/studio/src/uiComponents/form/BasicNumberInput.tsx @@ -1,6 +1,6 @@ import {clamp, isInteger, round} from 'lodash-es' -import type {MutableRefObject} from 'react'; -import { useEffect} from 'react' +import type {MutableRefObject} from 'react' +import {useEffect} from 'react' import {useState} from 'react' import React, {useMemo, useRef} from 'react' import styled from 'styled-components' @@ -232,9 +232,12 @@ const BasicNumberInput: React.FC<{ bodyCursorBeforeDrag.current = document.body.style.cursor return { - // note: we use mx because we need to constrain the `valueDuringDragging` - // and dx will keep accumulating past any constraints onDrag(_dx: number, _dy: number, e: MouseEvent, mx: number) { + // We use `mx` here because it allows us to offer better UX when dragging + // a value beyond its range. If we were to use `_dx`, and the number had a range, + // and the user nudged the number beyond its range, they would have to un-nudge all + // the way back until the number's value is within its range. But with `mx`, + // as soon as they reverse their mouse drag, the number will jump back to its range. const deltaX = e.altKey ? mx / 10 : mx const newValue = valueDuringDragging +