KOQ and numeric editor improvements (#747)

* initial * Reduce number of clicks by auto focusing input on render * pass setFocus boolen to inputs to determine if they should be focused on render * Tests and adjustments * changeset * adjustments * fixes * fixes * adjustment
iTwin · Oct 29, 2024 · 3c3e3af · 3c3e3af
1 parent 7f35490
commit 3c3e3af
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 28 deletions.
diff --git a/.changeset/light-weeks-explain.md b/.changeset/light-weeks-explain.md
@@ -0,0 +1,13 @@
+---
+"@itwin/presentation-components": minor
+---
+
+KoQ and numeric editor improvements
+
+Changes:
+
+- ReadOnly properties now open a disabled input in property grid.
+
+- KoQ input placeholder is now determined by initial value if one exists.
+
+- Selecting/clicking a numeric or KoQ input will select all the text.
diff --git a/packages/components/src/presentation-components/properties/inputs/NumericPropertyInput.tsx b/packages/components/src/presentation-components/properties/inputs/NumericPropertyInput.tsx
@@ -3,7 +3,7 @@
  * See LICENSE.md in the project root for license terms and full copyright notice.
  *--------------------------------------------------------------------------------------------*/
 
-import { forwardRef, useImperativeHandle, useRef, useState } from "react";
+import { forwardRef, useEffect, useImperativeHandle, useRef, useState } from "react";
 import { PrimitiveValue, PropertyRecord, PropertyValueFormat } from "@itwin/appui-abstract";
 import { PropertyEditorProps } from "@itwin/components-react";
 import { Input } from "@itwin/itwinui-react";
@@ -17,20 +17,10 @@ export interface NumericPropertyInputProps extends PropertyEditorProps {
 
 /** @internal */
 export const NumericPropertyInput = forwardRef<PropertyEditorAttributes, NumericPropertyInputProps>((props, ref) => {
-  const { onCommit, propertyRecord } = props;
+  const { onCommit, propertyRecord, setFocus } = props;
 
   const [inputValue, setInputValue] = useState<string>(() => getInputTargetFromPropertyRecord(propertyRecord) ?? "");
 
-  const divRef = useRef<HTMLInputElement>(null);
-  useImperativeHandle(
-    ref,
-    () => ({
-      getValue: () => parsePrimitiveValue(inputValue),
-      htmlElement: divRef.current,
-    }),
-    [inputValue],
-  );
-
   const handleChange = (newVal: string) => {
     setInputValue(newVal);
   };
@@ -42,11 +32,7 @@ export const NumericPropertyInput = forwardRef<PropertyEditorAttributes, Numeric
         newValue: parsePrimitiveValue(inputValue),
       });
   };
-  return (
-    <div ref={divRef}>
-      <NumericInput onChange={handleChange} value={inputValue} onBlur={commitInput} />
-    </div>
-  );
+  return <NumericInput onChange={handleChange} value={inputValue} onBlur={commitInput} isDisabled={propertyRecord.isReadonly} setFocus={setFocus} ref={ref} />;
 });
 NumericPropertyInput.displayName = "NumericPropertyInput";
 
@@ -70,14 +56,25 @@ function getInputTargetFromPropertyRecord(propertyRecord: PropertyRecord) {
 }
 
 /** @internal */
-export interface NumericInputProps {
+export interface NumericInputProps extends PropertyEditorProps {
   onChange: (newValue: string) => void;
   onBlur?: React.FocusEventHandler;
   value: string;
+  isDisabled?: boolean;
 }
 
 /** @internal */
-export const NumericInput = ({ value, onChange, onBlur }: NumericInputProps) => {
+export const NumericInput = forwardRef<PropertyEditorAttributes, NumericInputProps>(({ value, onChange, onBlur, isDisabled, setFocus }, ref) => {
+  const inputRef = useRef<HTMLInputElement>(null);
+  useImperativeHandle(
+    ref,
+    () => ({
+      getValue: () => parsePrimitiveValue(value),
+      htmlElement: inputRef.current,
+    }),
+    [value],
+  );
+
   const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
     const val = e.currentTarget.value;
     // Check if it is a correct number and it is not infinity.
@@ -103,5 +100,23 @@ export const NumericInput = ({ value, onChange, onBlur }: NumericInputProps) =>
     }
   };
 
-  return <Input data-testid="numeric-input" size="small" value={value} onChange={handleChange} onBlur={onBlur} />;
-};
+  useEffect(() => {
+    if (setFocus) {
+      inputRef.current && inputRef.current.focus();
+    }
+  }, [setFocus]);
+
+  return (
+    <Input
+      ref={inputRef}
+      disabled={isDisabled}
+      data-testid="numeric-input"
+      size="small"
+      value={value}
+      onChange={handleChange}
+      onBlur={onBlur}
+      onFocus={() => inputRef.current?.setSelectionRange(0, 9999)}
+    />
+  );
+});
+NumericInput.displayName = "NumericInput";
diff --git a/.../components/src/presentation-components/properties/inputs/QuantityPropertyEditorInput.tsx b/.../components/src/presentation-components/properties/inputs/QuantityPropertyEditorInput.tsx
@@ -3,7 +3,7 @@
  * See LICENSE.md in the project root for license terms and full copyright notice.
  *--------------------------------------------------------------------------------------------*/
 
-import { forwardRef, useImperativeHandle, useRef } from "react";
+import { forwardRef, useEffect, useImperativeHandle, useRef } from "react";
 import { PrimitiveValue, PropertyRecord, PropertyValueFormat } from "@itwin/appui-abstract";
 import { PropertyEditorProps } from "@itwin/components-react";
 import { Input } from "@itwin/itwinui-react";
@@ -41,7 +41,7 @@ QuantityPropertyEditorInput.displayName = "QuantityPropertyEditorInput";
 type QuantityPropertyValueInputProps = QuantityPropertyEditorImplProps & UseQuantityValueInputProps;
 
 const QuantityPropertyValueInput = forwardRef<PropertyEditorAttributes, QuantityPropertyValueInputProps>(
-  ({ propertyRecord, onCommit, koqName, schemaContext, initialRawValue }, ref) => {
+  ({ propertyRecord, onCommit, koqName, schemaContext, initialRawValue, setFocus }, ref) => {
     const { quantityValue, inputProps } = useQuantityValueInput({ koqName, schemaContext, initialRawValue });
 
     const inputRef = useRef<HTMLInputElement>(null);
@@ -72,7 +72,24 @@ const QuantityPropertyValueInput = forwardRef<PropertyEditorAttributes, Quantity
         });
     };
 
-    return <Input size="small" {...inputProps} ref={inputRef} onBlur={onBlur} />;
+    useEffect(() => {
+      if (setFocus && !inputProps.disabled) {
+        inputRef.current && inputRef.current.focus();
+      }
+    }, [inputProps.disabled, setFocus]);
+
+    return (
+      <Input
+        size="small"
+        {...inputProps}
+        disabled={propertyRecord.isReadonly || inputProps.disabled}
+        ref={inputRef}
+        onBlur={onBlur}
+        onFocus={() => {
+          inputRef.current?.setSelectionRange(0, 9999);
+        }}
+      />
+    );
   },
 );
 QuantityPropertyValueInput.displayName = "QuantityPropertyValueInput";
diff --git a/packages/components/src/presentation-components/properties/inputs/UseQuantityValueInput.tsx b/packages/components/src/presentation-components/properties/inputs/UseQuantityValueInput.tsx
@@ -3,7 +3,7 @@
  * See LICENSE.md in the project root for license terms and full copyright notice.
  *--------------------------------------------------------------------------------------------*/
 
-import { ChangeEventHandler, useEffect, useState } from "react";
+import { ChangeEventHandler, useEffect, useRef, useState } from "react";
 import { assert } from "@itwin/core-bentley";
 import { IModelApp } from "@itwin/core-frontend";
 import { FormatterSpec, ParserSpec } from "@itwin/core-quantity";
@@ -44,10 +44,11 @@ export function useQuantityValueInput({ initialRawValue, schemaContext, koqName
     quantityValue: QuantityValue;
     placeholder: string;
   }
+  const initialRawValueRef = useRef(initialRawValue);
 
   const [{ quantityValue, placeholder }, setState] = useState<State>(() => ({
     quantityValue: {
-      rawValue: initialRawValue,
+      rawValue: initialRawValueRef.current,
       formattedValue: "",
       roundingError: undefined,
     },
@@ -61,7 +62,7 @@ export function useQuantityValueInput({ initialRawValue, schemaContext, koqName
     }
 
     setState((prev): State => {
-      const newPlaceholder = formatter.applyFormatting(PLACEHOLDER_RAW_VALUE);
+      const newPlaceholder = formatter.applyFormatting(initialRawValueRef.current ?? PLACEHOLDER_RAW_VALUE);
       const newFormattedValue = prev.quantityValue.rawValue !== undefined ? formatter.applyFormatting(prev.quantityValue.rawValue) : "";
       const roundingError = getPersistenceUnitRoundingError(newFormattedValue, parser);
 

diff --git a/packages/components/src/test/properties/inputs/NumericPropertyInput.test.tsx b/packages/components/src/test/properties/inputs/NumericPropertyInput.test.tsx
@@ -40,6 +40,7 @@ describe("<NumericPropertyInput />", () => {
 
     const inputContainer = await waitFor(() => getByRole("textbox"));
 
+    await user.click(inputContainer);
     await user.type(inputContainer, "0");
 
     await waitFor(() => expect((ref.current?.getValue() as PrimitiveValue).value).to.be.eq(10));
@@ -64,6 +65,7 @@ describe("<NumericPropertyInput />", () => {
 
     const inputContainer = await waitFor(() => getByRole("textbox"));
 
+    await user.click(inputContainer);
     await user.type(inputContainer, "1");
     expect(queryByDisplayValue("-101")).to.not.be.null;
     expect((ref.current?.getValue() as PrimitiveValue).value).to.be.eq(-101);
@@ -187,6 +189,7 @@ describe("<NumericInput />", () => {
     const { getByRole, user } = render(<NumericInput onChange={spy} value="1e90" />);
     const inputContainer = await waitFor(() => getByRole("textbox"));
 
+    await user.click(inputContainer);
     await user.type(inputContainer, "1");
 
     expect(spy.called).to.be.false;
@@ -222,6 +225,7 @@ describe("<NumericInput />", () => {
     const { getByRole, user } = render(<NumericInput onChange={spy} value="+" />);
     const inputContainer = await waitFor(() => getByRole("textbox"));
 
+    await user.click(inputContainer);
     await user.type(inputContainer, ".");
 
     expect(spy).to.be.calledWith("+.");
@@ -232,6 +236,7 @@ describe("<NumericInput />", () => {
     const { getByRole, user } = render(<NumericInput onChange={spy} value="-" />);
     const inputContainer = await waitFor(() => getByRole("textbox"));
 
+    await user.click(inputContainer);
     await user.type(inputContainer, ".");
 
     expect(spy).to.be.calledWith("-.");
@@ -242,6 +247,7 @@ describe("<NumericInput />", () => {
     const { getByRole, user } = render(<NumericInput onChange={spy} value="1" />);
     const inputContainer = await waitFor(() => getByRole("textbox"));
 
+    await user.click(inputContainer);
     await user.type(inputContainer, "e");
 
     expect(spy).to.be.calledWith("1e");
@@ -252,6 +258,7 @@ describe("<NumericInput />", () => {
     const { getByRole, user } = render(<NumericInput onChange={spy} value="1e" />);
     const inputContainer = await waitFor(() => getByRole("textbox"));
 
+    await user.click(inputContainer);
     await user.type(inputContainer, "-");
 
     expect(spy).to.be.calledWith("1e-");
@@ -267,6 +274,13 @@ describe("<NumericInput />", () => {
     expect(spy).to.be.be.calledOnce;
   });
 
+  it("should focus on input if setFocus is true", async () => {
+    const { getByRole } = render(<NumericInput onChange={() => {}} value="1" setFocus={true} />);
+
+    const input = await waitFor(() => getByRole("textbox"));
+    await waitFor(() => expect(input).to.be.eq(document.activeElement));
+  });
+
   it("commits undefined value when propertyRecord value is NaN on `onBlur` event", async () => {
     const record = createRecord(Number.NaN);
     const spy = sinon.spy();

diff --git a/packages/components/src/test/properties/inputs/QuantityPropertyEditorInput.test.tsx b/packages/components/src/test/properties/inputs/QuantityPropertyEditorInput.test.tsx
@@ -139,7 +139,7 @@ describe("<QuantityPropertyEditorInput />", () => {
     const record = createRecord({ initialValue: undefined, quantityType: "TestKOQ" });
     const { getByRole, user } = render(
       <SchemaMetadataContextProvider imodel={{} as IModelConnection} schemaContextProvider={() => schemaContext}>
-        <QuantityPropertyEditorInput ref={ref} propertyRecord={record} onCommit={spy} />
+        <QuantityPropertyEditorInput ref={ref} propertyRecord={record} onCommit={spy} setFocus={true} />
       </SchemaMetadataContextProvider>,
     );
 
@@ -168,4 +168,20 @@ describe("<QuantityPropertyEditorInput />", () => {
       roundingError: 0.05,
     });
   });
+
+  it("should focus on input if setFocus is true", async () => {
+    const record = createRecord({ initialValue: undefined, quantityType: "TestKOQ" });
+    const ref = createRef<PropertyEditorAttributes>();
+
+    const { getByRole } = render(
+      <SchemaMetadataContextProvider imodel={{} as IModelConnection} schemaContextProvider={() => schemaContext}>
+        <QuantityPropertyEditorInput ref={ref} propertyRecord={record} setFocus={true} />
+      </SchemaMetadataContextProvider>,
+    );
+
+    const input = await waitFor(() => getByRole("textbox"));
+    await waitFor(() => {
+      expect(input).to.be.eq(document.activeElement);
+    });
+  });
 });