[Fluent/InspectorV2] Fix bugs in number/text input; Add input validation UX; Refactor colorPicker to use new components

packages/dev/inspector-v2/src/components/properties/animation/animationGroupProperties.tsx

@@ -78,8 +78,7 @@ export const AnimationGroupControlProperties: FunctionComponent<{ animationGroup
                     />
                     <BoundProperty component={SwitchPropertyLine} label="Is additive" target={animationGroup} propertyKey="isAdditive" />
                     <BoundProperty component={NumberInputPropertyLine} label="Weight" target={animationGroup} propertyKey="weight" step={0.1} />
-                    <BoundProperty component={NumberInputPropertyLine} label="Play order" target={animationGroup} propertyKey="playOrder" step={0} />
-                    {/* TODO: Hey georgie :<Play order> should be integer (even when typing)*/}
+                    <BoundProperty component={NumberInputPropertyLine} label="Play order" target={animationGroup} propertyKey="playOrder" forceInt />
                 </>
             </Collapse>
         </>

packages/dev/sharedUiComponents/src/fluent/hoc/propertyLines/inputPropertyLine.tsx

@@ -1,27 +1,33 @@
 import { PropertyLine } from "./propertyLine";
 import type { PropertyLineProps } from "./propertyLine";
 import type { FunctionComponent } from "react";
-import { NumberInput, TextInput } from "../../primitives/input";
-import type { InputProps } from "../../primitives/input";
-
+import type { TextInputProps } from "../../primitives/textInput";
+import { TextInput } from "../../primitives/textInput";
+import type { SpinButtonProps } from "../../primitives/spinButton";
+import { SpinButton } from "../../primitives/spinButton";
 /**
  * Wraps a text input in a property line
  * @param props - PropertyLineProps and InputProps
  * @returns property-line wrapped input component
  */
-export const TextInputPropertyLine: FunctionComponent<InputProps<string> & PropertyLineProps<string>> = (props) => (
+export const TextInputPropertyLine: FunctionComponent<TextInputProps & PropertyLineProps<string>> = (props) => (
     <PropertyLine {...props}>
         <TextInput {...props} />
     </PropertyLine>
 );
 
 /**
  * Wraps a number input in a property line
+ * To force integer values, use forceInt param (this is distinct from the 'step' param, which will still allow submitting an integer value. forceInt will not)
  * @param props - PropertyLineProps and InputProps
  * @returns property-line wrapped input component
  */
-export const NumberInputPropertyLine: FunctionComponent<InputProps<number> & PropertyLineProps<number>> = (props) => (
-    <PropertyLine {...props}>
-        <NumberInput {...props} />
-    </PropertyLine>
-);
+export const NumberInputPropertyLine: FunctionComponent<SpinButtonProps & PropertyLineProps<number> & { forceInt?: boolean }> = (props) => {
+    const { forceInt, ...rest } = props;
+    const propsToAdd = forceInt ? { step: 0, validator: Number.isInteger } : {};
+    return (
+        <PropertyLine {...rest}>
+            <SpinButton {...rest} {...propsToAdd} />
+        </PropertyLine>
+    );
+};

packages/dev/sharedUiComponents/src/fluent/primitives/infoLabel.tsx

@@ -0,0 +1,22 @@
+import type { FunctionComponent } from "react";
+import { InfoLabel as FluentInfoLabel } from "@fluentui/react-components";
+
+export type InfoLabelProps = {
+    htmlFor: string; // required ID of the element whose label we are applying
+    info?: JSX.Element;
+    label: string;
+};
+export type InfoLabelParentProps = Omit<InfoLabelProps, "htmlFor">;
+
+/**
+ * Renders a label with an optional popup containing more info
+ * @param props
+ * @returns
+ */
+export const InfoLabel: FunctionComponent<InfoLabelProps> = (props) => {
+    return (
+        <FluentInfoLabel htmlFor={props.htmlFor} info={props.info}>
+            {props.label}
+        </FluentInfoLabel>
+    );
+};

packages/dev/sharedUiComponents/src/fluent/primitives/primitive.ts

@@ -1,3 +1,5 @@
+import type { InfoLabelParentProps } from "./infoLabel";
+
 export type ImmutablePrimitiveProps<ValueT> = {
     /**
      * The value of the property to be displayed and modified.
@@ -17,6 +19,11 @@ export type ImmutablePrimitiveProps<ValueT> = {
      * Optional title for the component, used for tooltips or accessibility.
      */
     title?: string;
+
+    /**
+     * Optional information to display as an infoLabel popup aside the component.
+     */
+    infoLabel?: InfoLabelParentProps;
 };
 
 export type PrimitiveProps<T> = ImmutablePrimitiveProps<T> & {

packages/dev/sharedUiComponents/src/fluent/primitives/spinButton.tsx

@@ -1,13 +1,15 @@
-import { makeStyles, SpinButton as FluentSpinButton } from "@fluentui/react-components";
+import { makeStyles, SpinButton as FluentSpinButton, useId } from "@fluentui/react-components";
 import type { SpinButtonOnChangeData, SpinButtonChangeEvent } from "@fluentui/react-components";
-import type { FunctionComponent } from "react";
-import { useCallback, useState } from "react";
+import type { FunctionComponent, KeyboardEvent, FocusEvent } from "react";
+import { useEffect, useState, useRef } from "react";
 import type { PrimitiveProps } from "./primitive";
+import { InfoLabel } from "./infoLabel";
 
 const useSpinStyles = makeStyles({
     base: {
         display: "flex",
         flexDirection: "column",
+        width: "100px",
     },
 });
 
@@ -16,33 +18,79 @@ export type SpinButtonProps = PrimitiveProps<number> & {
     step?: number; // Optional step value for the spin button
     min?: number;
     max?: number;
+    validator?: (value: number) => boolean;
 };
 
 export const SpinButton: FunctionComponent<SpinButtonProps> = (props) => {
     const classes = useSpinStyles();
+    const { min, max } = props;
 
-    const [spinButtonValue, setSpinButtonValue] = useState(props.value);
-
-    const onSpinButtonChange = useCallback(
-        (_ev: SpinButtonChangeEvent, data: SpinButtonOnChangeData) => {
-            // Stop propagation of the event to prevent it from bubbling up
-            _ev.stopPropagation();
-
-            if (data.value != null) {
-                setSpinButtonValue(data.value);
-            } else if (data.displayValue !== undefined) {
-                const newValue = parseFloat(data.displayValue);
-                if (!Number.isNaN(newValue)) {
-                    setSpinButtonValue(newValue);
-                }
-            }
-        },
-        [setSpinButtonValue]
-    );
+    const [value, setValue] = useState(props.value);
+    const lastCommittedValue = useRef(props.value);
+
+    useEffect(() => {
+        if (props.value != lastCommittedValue.current) {
+            setValue(props.value); // Update local state when props.value changes
+            lastCommittedValue.current = props.value;
+        }
+    }, [props.value]);
+
+    const validateValue = (numericValue: number): boolean => {
+        const outOfBounds = (min !== undefined && numericValue < min) || (max !== undefined && numericValue > max);
+        const failsValidator = props.validator && !props.validator(numericValue);
+        const invalid = !!outOfBounds || !!failsValidator || isNaN(numericValue);
+        return !invalid;
+    };
+
+    const tryCommitValue = (currVal: number) => {
+        // Only commit if valid and different from last committed value
+        if (validateValue(currVal) && currVal !== lastCommittedValue.current) {
+            lastCommittedValue.current = currVal;
+            props.onChange(currVal);
+        }
+    };
+
+    const handleChange = (event: SpinButtonChangeEvent, data: SpinButtonOnChangeData) => {
+        event.stopPropagation(); // Prevent event propagation
+        if (data.value != null) {
+            setValue(data.value); // Update local state. Do not notify parent
+        }
+    };
+
+    const handleKeyDown = (event: KeyboardEvent<HTMLInputElement>) => {
+        event.stopPropagation(); // Prevent event propagation
+
+        // Prevent Enter key from causing form submission or value reversion
+        if (event.key === "Enter") {
+            event.preventDefault();
+
+            // Update local state and try to commit the value if valid
+            const currVal = parseFloat((event.target as any).value);
+            setValue(currVal);
+            tryCommitValue(currVal);
+        }
+    };
+
+    const handleBlur = (event: FocusEvent<HTMLInputElement>) => {
+        event.stopPropagation(); // Prevent event propagation
+        // Try to commit the current value when losing focus
+        const currVal = parseFloat(event.target.value);
+        setValue(currVal);
+        tryCommitValue(currVal);
+    };
+
+    const invalidStyle = !validateValue(value)
+        ? {
+              backgroundColor: "#fdeaea",
+              borderColor: "#d13438",
+          }
+        : {};
 
+    const id = useId("spin-button");
     return (
         <div className={classes.base}>
-            <FluentSpinButton {...props} value={spinButtonValue} onChange={onSpinButtonChange} />
+            {props.infoLabel && <InfoLabel {...props.infoLabel} htmlFor={id} />}
+            <FluentSpinButton {...props} id={id} size="small" value={value} onChange={handleChange} onKeyDown={handleKeyDown} onBlur={handleBlur} style={invalidStyle} />
         </div>
     );
 };

packages/dev/sharedUiComponents/src/fluent/primitives/syncedSlider.tsx

@@ -1,6 +1,6 @@
 import type { SliderOnChangeData } from "@fluentui/react-components";
 import { makeStyles, Slider, tokens } from "@fluentui/react-components";
-import { NumberInput } from "./input";
+import { SpinButton } from "./spinButton";
 import type { ChangeEvent, FunctionComponent } from "react";
 import { useEffect, useState, useRef } from "react";
 import type { PrimitiveProps } from "./primitive";
@@ -17,7 +17,6 @@ const useSyncedSliderStyles = makeStyles({
         minWidth: "40px", // Minimum width for slider to remain usable
     },
     input: {
-        width: "40px", // Fixed width for input - always 40px
         flexShrink: 0,
     },
 });
@@ -80,12 +79,9 @@ export const SyncedSliderInput: FunctionComponent<SyncedSliderProps> = (props) =
         isDraggingRef.current = false;
     };
 
-    const handleInputChange = (value: string | number) => {
-        const newValue = Number(value);
-        if (!isNaN(newValue)) {
-            setValue(newValue);
-            props.onChange(newValue); // Input always updates immediately
-        }
+    const handleInputChange = (value: number) => {
+        setValue(value);
+        props.onChange(value); // Input always updates immediately
     };
 
     return (
@@ -104,7 +100,7 @@ export const SyncedSliderInput: FunctionComponent<SyncedSliderProps> = (props) =
                     onPointerUp={handleSliderPointerUp}
                 />
             )}
-            <NumberInput {...props} className={classes.input} value={Math.round(value / step) * step} onChange={handleInputChange} step={step} />
+            <SpinButton {...props} className={classes.input} value={Math.round(value / step) * step} onChange={handleInputChange} step={props.step} />
         </div>
     );
 };