From fee87685dbcad7ab55f5ea76273ea58b7cdf8705 Mon Sep 17 00:00:00 2001 From: Ivan Starkov Date: Wed, 3 May 2023 15:02:17 +0400 Subject: [PATCH] Fix scrub control (#1559) ## Description Fixes scrub control can set invalid unitless value ## Steps for reproduction 1. click button 2. expect xyz ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 5de6) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio-builder/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env.example` and the `builder/env-check.js` if mandatory --- .../style-panel/sections/space/scrub.tsx | 26 ++++++++- .../css-value-input/css-value-input.tsx | 57 ++++++++++++++----- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/apps/builder/app/builder/features/style-panel/sections/space/scrub.tsx b/apps/builder/app/builder/features/style-panel/sections/space/scrub.tsx index aa561f195d..0f8cb335f4 100644 --- a/apps/builder/app/builder/features/style-panel/sections/space/scrub.tsx +++ b/apps/builder/app/builder/features/style-panel/sections/space/scrub.tsx @@ -4,6 +4,10 @@ import { useState, useEffect, useRef } from "react"; import { useModifierKeys } from "../../shared/modifier-keys"; import type { StyleUpdateOptions } from "../../shared/use-style-data"; import type { SpaceStyleProperty, HoverTagret } from "./types"; +import { isValid } from "../../shared/parse-css-value"; +import { parseIntermediateOrInvalidValue } from "../../shared/css-value-input/parse-intermediate-or-invalid-value"; +import { toValue } from "@webstudio-is/css-engine"; +import type { CssValueInputValue } from "../../shared/css-value-input/css-value-input"; type Values = Partial>; @@ -61,12 +65,32 @@ export const useScrub = (props: { const { values, properties, props } = nonDependencies.current; - const value = { + let value: CssValueInputValue = { type: "unit", value: event.value, unit: unitRef.current, } as const; + if (isValid(property, toValue(value)) === false) { + value = parseIntermediateOrInvalidValue(property, { + type: "intermediate", + value: `${value.value}`, + unit: value.unit, + }); + + // In case of negative values for some properties, we might end up with invalid value. + if (value.type === "invalid") { + // Try return unitless + if (isValid(property, "0")) { + value = { + type: "unit", + unit: "number", + value: 0, + }; + } + } + } + const nextValues = { ...values }; for (const property of properties) { nextValues[property] = value; diff --git a/apps/builder/app/builder/features/style-panel/shared/css-value-input/css-value-input.tsx b/apps/builder/app/builder/features/style-panel/shared/css-value-input/css-value-input.tsx index bd84f49385..cb4925c9fd 100644 --- a/apps/builder/app/builder/features/style-panel/shared/css-value-input/css-value-input.tsx +++ b/apps/builder/app/builder/features/style-panel/shared/css-value-input/css-value-input.tsx @@ -34,7 +34,7 @@ import { toValue } from "@webstudio-is/css-engine"; import { useDebouncedCallback } from "use-debounce"; import type { StyleSource } from "../style-info"; import { toPascalCase } from "../keyword-utils"; - +import { isValid } from "../parse-css-value"; import { selectedInstanceBrowserStyleStore, selectedInstanceUnitSizesStore, @@ -43,11 +43,13 @@ import { convertUnits } from "./convert-units"; const useScrub = ({ value, + property, onChange, onChangeComplete, shouldHandleEvent, }: { value: CssValueInputValue; + property: StyleProperty; onChange: (value: CssValueInputValue) => void; onChangeComplete: (value: StyleValue) => void; shouldHandleEvent?: (node: Node) => boolean; @@ -68,13 +70,14 @@ const useScrub = ({ valueRef.current = value; const type = valueRef.current.type; - const unit = type === "unit" ? valueRef.current.unit : undefined; // Since scrub is going to call onChange and onChangeComplete callbacks, it will result in a new value and potentially new callback refs. // We need this effect to ONLY run when type or unit changes, but not when callbacks or value.value changes. useEffect(() => { const inputRefCurrent = inputRef.current; const scrubRefCurrent = scrubRef.current; + const unit = type === "unit" ? valueRef.current.unit : undefined; + if ( type !== "unit" || unit === undefined || @@ -84,6 +87,38 @@ const useScrub = ({ return; } + const validateValue = (numericValue: number) => { + let value: CssValueInputValue = { + type, + unit, + value: numericValue, + }; + + if ( + value.type === "unit" && + isValid(property, toValue(value)) === false + ) { + value = parseIntermediateOrInvalidValue(property, { + type: "intermediate", + value: `${value.value}`, + unit: value.unit, + }); + + // In case of negative values for some properties, we might end up with invalid value. + if (value.type === "invalid") { + // Try return unitless + if (isValid(property, "0")) { + value = { + type: "unit", + unit: "number", + value: 0, + }; + } + } + } + return value; + }; + return numericScrubControl(scrubRefCurrent, { // @todo: after this https://github.com/webstudio-is/webstudio-builder/issues/564 // we can switch back on using just initial value @@ -105,21 +140,16 @@ const useScrub = ({ scrubRef.current?.setAttribute("tabindex", "-1"); scrubRef.current?.focus(); - onChangeRef.current({ - type, - unit, - value: event.value, - }); + const value = validateValue(event.value); + + onChangeRef.current(value); }, onValueChange(event) { // Will work without but depends on order of setState updates // at text-control, now fixed in both places (order of updates is right, and batched here) + const value = validateValue(event.value); - onChangeCompleteRef.current({ - type, - unit, - value: event.value, - }); + onChangeCompleteRef.current(value); // Returning focus that we've moved above scrubRef.current?.removeAttribute("tabindex"); @@ -128,7 +158,7 @@ const useScrub = ({ }, shouldHandleEvent: shouldHandleEvent, }); - }, [type, unit, shouldHandleEvent]); + }, [type, shouldHandleEvent, property]); return [scrubRef, inputRef]; }; @@ -457,6 +487,7 @@ export const CssValueInput = ({ const [scrubRef, inputRef] = useScrub({ value, + property, onChange: props.onChange, onChangeComplete: (value) => onChangeComplete(value, "scrub-end"), shouldHandleEvent,