mirror of
https://github.com/webstudio-is/webstudio.git
synced 2026-11-08 21:15:18 +09:00
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
This commit is contained in:
1 parent
bcdfc9f83d
commit
fee87685db
2 files changed
+69
-14
No files matched your search
@@ -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<Record<SpaceStyleProperty, StyleValue>>;
|
||||
|
||||
@@ -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;
|
||||
|
||||
+44
-13
@@ -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,
|
||||
|
||||
Reference in new issue
Block a user