Merge pull request #3442 from quantified-uncertainty/form-improvements

@quri/ui form improvements
quantified-uncertainty · Nov 17, 2024 · af0cd96 · af0cd96
2 parents f90c16a + 7aecb59
commit af0cd96
Show file tree

Hide file tree

Showing 19 changed files with 183 additions and 119 deletions.
diff --git a/.changeset/four-moles-tickle.md b/.changeset/four-moles-tickle.md
@@ -0,0 +1,5 @@
+---
+"@quri/ui": patch
+---
+
+support `tooltip` prop in all form fields
diff --git a/.changeset/gorgeous-schools-mix.md b/.changeset/gorgeous-schools-mix.md
@@ -0,0 +1,5 @@
+---
+"@quri/ui": patch
+---
+
+support `layout` prop in form fields, remove `inlineLabel` prop
diff --git a/packages/hub/src/app/ai/Sidebar.tsx b/packages/hub/src/app/ai/Sidebar.tsx
@@ -17,7 +17,6 @@ import {
   SelectStringFormField,
   StyledTab,
   TextAreaFormField,
-  TextTooltip,
 } from "@quri/ui";
 
 import { LoadMoreViaSearchParam } from "@/components/LoadMoreViaSearchParam";
@@ -175,53 +174,36 @@ Outputs:
           </div>
         </StyledTab.Group>
         <div className="flex flex-col gap-2">
-          <div className="flex items-center justify-between">
-            <TextTooltip
-              text="Enhance the model's numerical calculations and reasoning."
-              placement="bottom"
-            >
-              <span className="cursor-help text-sm underline decoration-dotted">
-                Numeric Steps
-              </span>
-            </TextTooltip>
-            <span className="w-16">
-              <NumberFormField<FormShape> name="numericSteps" size="small" />
-            </span>
-          </div>
-          <div className="flex items-center justify-between">
-            <TextTooltip
-              text="Improve the model's documentation and formatting."
-              placement="bottom"
-            >
-              <span className="cursor-help text-sm underline decoration-dotted">
-                Documentation Steps
-              </span>
-            </TextTooltip>
-            <span className="w-16">
-              <NumberFormField<FormShape> name="styleGuideSteps" size="small" />
-            </span>
-          </div>
-        </div>
-        <div className="flex items-center justify-between">
-          <TextTooltip
-            text="Choose the LLM to use. Sonnet is much better than Haiku, but is around 3x more expensive. We use the latest versions of Sonnet 3.5 and Haiku 3.5."
-            placement="bottom"
-          >
-            <span className="cursor-help text-sm underline decoration-dotted">
-              Model
-            </span>
-          </TextTooltip>
-          <span className="w-40">
-            <SelectStringFormField<FormShape, LlmId>
-              name="model"
-              size="small"
-              options={MODEL_CONFIGS.filter(
-                (model) => model.provider === "anthropic"
-              ).map((model) => model.id)}
-              required
-            />
-          </span>
+          <NumberFormField<FormShape>
+            name="numericSteps"
+            label="Numeric Steps"
+            tooltip="Enhance the model's numerical calculations and reasoning."
+            inputWidth="w-16"
+            size="small"
+            layout="row"
+            rules={{ min: 0 }}
+          />
+          <NumberFormField<FormShape>
+            name="styleGuideSteps"
+            label="Documentation Steps"
+            tooltip="Improve the model's documentation and formatting."
+            inputWidth="w-16"
+            size="small"
+            layout="row"
+            rules={{ min: 0 }}
+          />
         </div>
+        <SelectStringFormField<FormShape, LlmId>
+          label="Model"
+          tooltip="Choose the LLM to use. Sonnet is much better than Haiku, but is around 3x more expensive. We use the latest versions of Sonnet 3.5 and Haiku 3.5."
+          layout="row"
+          name="model"
+          size="small"
+          options={MODEL_CONFIGS.filter(
+            (model) => model.provider === "anthropic"
+          ).map((model) => model.id)}
+          required
+        />
         <Button wide onClick={handleSubmit} disabled={isSubmitDisabled}>
           Start Workflow
         </Button>

diff --git a/packages/ui/src/forms/common/ControlledFormField.tsx b/packages/ui/src/forms/common/ControlledFormField.tsx
@@ -34,19 +34,11 @@ export function ControlledFormField<
 >({
   name,
   rules,
-  label,
-  description,
-  inlineLabel,
-  standaloneLabel,
   children,
+  ...layoutProps
 }: ControlledFormFieldProps<TValues, TValueType, TName>) {
   return (
-    <FieldLayout
-      label={label}
-      description={description}
-      inlineLabel={inlineLabel}
-      standaloneLabel={standaloneLabel}
-    >
+    <FieldLayout {...layoutProps}>
       <ControlledFormInput name={name} rules={rules}>
         {children}
       </ControlledFormInput>
@@ -58,6 +50,6 @@ export function ControlledFormField<
  * Usage example:
  *
  * <FormField name="slug" label="Slug" description="Model slug">{
- *     props => <StyledInput type="text" {...props} />
+ *     props => <StyledInput {...props} />
  * }</FormField>
  */
diff --git a/packages/ui/src/forms/common/FormField.tsx b/packages/ui/src/forms/common/FormField.tsx
@@ -26,15 +26,15 @@ export function FormField<
   rules,
   label,
   description,
-  inlineLabel,
+  layout,
   standaloneLabel,
   children,
 }: FormFieldProps<TValues, TName>) {
   return (
     <FieldLayout
       label={label}
       description={description}
-      inlineLabel={inlineLabel}
+      layout={layout}
       standaloneLabel={standaloneLabel}
     >
       <FormInput name={name} rules={rules}>
@@ -48,6 +48,6 @@ export function FormField<
  * Usage example:
  *
  * <FormField name="slug" label="Slug" description="Model slug">{
- *     props => <StyledInput type="text" {...props} />
+ *     props => <StyledInput {...props} />
  * }</FormField>
  */
diff --git a/packages/ui/src/forms/common/FormFieldLayout.tsx b/packages/ui/src/forms/common/FormFieldLayout.tsx
@@ -1,49 +1,76 @@
 import { clsx } from "clsx";
 import { FC, PropsWithChildren } from "react";
 
+import { TextTooltip } from "../../components/TextTooltip.js";
+
 export type FormFieldLayoutProps = {
   label?: string;
   // TODO - allow ReactNode for inline links and other formatting?
   description?: string;
-  // useful for checkboxes
-  inlineLabel?: boolean;
+  layout?: "col" | "row" | "reverse-row";
   // If set, label won't wrap children and won't be clickable. This is useful for some custom fields where outer label leads to too large clickable area.
   standaloneLabel?: boolean;
+  tooltip?: string;
 };
 
 export const FieldLayout: FC<PropsWithChildren<FormFieldLayoutProps>> = ({
   label,
   description,
-  inlineLabel,
   standaloneLabel,
+  layout = "col",
+  tooltip,
   children,
 }) => {
   const OuterTag = standaloneLabel ? "div" : "label";
   const InnerTag = standaloneLabel ? "label" : "div";
 
-  // TODO - use <label forHtml=...> (but this would require a `useId`)
-  return (
-    <OuterTag className="block">
-      <div
+  let labelEl: JSX.Element | null = null;
+  if (label) {
+    labelEl = (
+      <InnerTag
         className={clsx(
-          inlineLabel && "flex flex-row-reverse items-end justify-end gap-2"
+          "mb-1 text-sm font-medium",
+          // TODO - add `disabled` prop?
+          "text-gray-900",
+          layout !== "col" && "leading-none",
+          tooltip && "cursor-help underline decoration-dotted"
         )}
       >
-        {label !== undefined && (
-          <InnerTag
-            className={clsx(
-              "mb-1 text-sm font-medium",
-              // TODO - add `disabled` prop?
-              "text-gray-900",
-              inlineLabel && "leading-none"
-            )}
-          >
-            {label}
-          </InnerTag>
-        )}
-        {children}
-      </div>
-      {description !== undefined && (
+        {label}
+      </InnerTag>
+    );
+  }
+  if (labelEl && tooltip) {
+    labelEl = (
+      <TextTooltip text={tooltip} placement="bottom">
+        {labelEl}
+      </TextTooltip>
+    );
+  }
+
+  // TODO - use <label forHtml=...> (but this would require a `useId`)
+  return (
+    <OuterTag className="block">
+      {layout === "col" && (
+        <div>
+          {labelEl}
+          {children}
+        </div>
+      )}
+      {layout === "reverse-row" && (
+        <div className="flex flex-row-reverse items-end justify-end gap-2">
+          <div>{labelEl}</div>
+          <div>{children}</div>
+        </div>
+      )}
+      {layout === "row" && (
+        <div className="flex items-center justify-between gap-2">
+          <div>{labelEl}</div>
+          <div>{children}</div>
+        </div>
+      )}
+
+      {description && (
         <div className="mt-1 text-sm text-slate-600">{description}</div>
       )}
     </OuterTag>

diff --git a/packages/ui/src/forms/common/FormInput.tsx b/packages/ui/src/forms/common/FormInput.tsx
@@ -36,6 +36,6 @@ export function FormInput<
 /* Usage example:
  *
  * <FormInput name="foo">
- *   {(props) => <StyledInput type="text" {...props} />}
+ *   {(props) => <StyledInput {...props} />}
  * </FormInput>
  */
diff --git a/packages/ui/src/forms/common/types.ts b/packages/ui/src/forms/common/types.ts
@@ -22,14 +22,14 @@ type NumberRules<
   TName extends FieldPath<TValues> = FieldPath<TValues>,
 > = Omit<StringRules<TValues, TName>, "pattern">; // should this also exclude maxLength?
 
-// These types is useful for specific field/*FormField declarations.
+// These types are useful for specific field/*FormField declarations.
 export type CommonStringFieldProps<
   TValues extends FieldValues,
   TName extends FieldPathByValue<TValues, string> = FieldPathByValue<
     TValues,
     string
   >,
-> = Omit<FormFieldLayoutProps, "inlineLabel"> & {
+> = FormFieldLayoutProps & {
   name: TName;
   rules?: StringRules<TValues, TName>;
 };
@@ -40,15 +40,16 @@ export type CommonNumberFieldProps<
     TValues,
     number | undefined
   > = FieldPathByValue<TValues, number | undefined>,
-> = Omit<FormFieldLayoutProps, "inlineLabel"> & {
+> = FormFieldLayoutProps & {
   name: TName;
   rules?: NumberRules<TValues, TName>;
+  inputWidth?: `w-${number}`;
 };
 
 export type CommonUnknownFieldProps<
   TValues extends FieldValues,
   TName extends FieldPath<TValues> = FieldPath<TValues>,
-> = Omit<FormFieldLayoutProps, "inlineLabel"> & {
+> = FormFieldLayoutProps & {
   name: TName;
   // TODO - allow more rules?
   rules?: Pick<RegisterOptions<TValues, TName>, "required" | "validate">;

diff --git a/packages/ui/src/forms/fields/CheckboxFormField.tsx b/packages/ui/src/forms/fields/CheckboxFormField.tsx
@@ -9,7 +9,10 @@ export function CheckboxFormField<
   TName extends FieldPath<TValues> = FieldPath<TValues>,
 >({ ...fieldProps }: CommonUnknownFieldProps<TValues, TName>) {
   return (
-    <ControlledFormField {...fieldProps} inlineLabel>
+    <ControlledFormField
+      {...fieldProps}
+      layout={fieldProps.layout ?? "reverse-row"}
+    >
       {({ value, onChange }) => (
         <StyledCheckbox checked={value} onChange={onChange} />
       )}

diff --git a/packages/ui/src/forms/fields/ColorFormField.tsx b/packages/ui/src/forms/fields/ColorFormField.tsx
@@ -12,7 +12,10 @@ export function ColorFormField<
   >,
 >({ ...fieldProps }: CommonStringFieldProps<TValues, TName>) {
   return (
-    <ControlledFormField<TValues, string, TName> {...fieldProps} inlineLabel>
+    <ControlledFormField<TValues, string, TName>
+      {...fieldProps}
+      layout="reverse-row"
+    >
       {({ value, onChange }) => (
         <StyledColorInput value={value} onChange={onChange} />
       )}

diff --git a/packages/ui/src/forms/fields/NumberFormField.tsx b/packages/ui/src/forms/fields/NumberFormField.tsx
@@ -2,7 +2,10 @@ import { FieldPathByValue, FieldValues } from "react-hook-form";
 
 import { ControlledFormField } from "../common/ControlledFormField.js";
 import { CommonNumberFieldProps } from "../common/types.js";
-import { StyledInput, type StyledInputProps } from "../styled/StyledInput.js";
+import {
+  getInputClassName,
+  type StyledInputProps,
+} from "../styled/StyledInput.js";
 
 export function NumberFormField<
   TValues extends FieldValues,
@@ -13,6 +16,7 @@ export function NumberFormField<
 >({
   placeholder,
   rules = {},
+  inputWidth,
   ...fieldProps
 }: CommonNumberFieldProps<TValues, TName> &
   Pick<StyledInputProps, "size" | "placeholder">) {
@@ -23,7 +27,11 @@ export function NumberFormField<
     <ControlledFormField {...fieldProps} rules={rules}>
       {({ onChange, onBlur, value }) => {
         return (
-          <StyledInput
+          <input
+            className={getInputClassName(
+              fieldProps.size ?? "normal",
+              inputWidth
+            )}
             // type="number" is kinda broken in Firefox.
             // See also: https://news.ycombinator.com/item?id=32852643
             // We implement something number-like manually here.

diff --git a/packages/ui/src/forms/fields/SelectFormField.tsx b/packages/ui/src/forms/fields/SelectFormField.tsx
@@ -75,8 +75,6 @@ export function SelectFormField<
     TValue
   >,
 >({
-  label,
-  description,
   name,
   options,
   required = false,
@@ -90,7 +88,8 @@ export function SelectFormField<
   placeholder,
   optionToFieldValue: maybeOptionToFieldValue,
   fieldValueToOption: maybeFieldValueToOption,
-}: Pick<FormFieldLayoutProps, "label" | "description"> & {
+  ...layoutProps
+}: FormFieldLayoutProps & {
   name: TName;
   required?: boolean;
   disabled?: boolean;
@@ -153,9 +152,8 @@ export function SelectFormField<
     >
       <ControlledFormField<TValues, TValue, TName>
         name={name}
-        label={label}
-        description={description}
         rules={{ required }}
+        {...layoutProps}
       >
         {({ name, value, onChange }) => {
           /* `selectValue` can be null while `value` is not null.

diff --git a/packages/ui/src/forms/fields/SelectStringFormField.tsx b/packages/ui/src/forms/fields/SelectStringFormField.tsx
@@ -22,7 +22,7 @@ export function SelectStringFormField<
   options,
   size = "normal",
   ...props
-}: Pick<FormFieldLayoutProps, "label" | "description"> & {
+}: FormFieldLayoutProps & {
   name: TName;
   size?: Size;
   options: NonNullable<TValueType>[];