feat: improve binding validation (#2730)

Ref https://github.com/webstudio-is/webstudio/issues/2532

Here changed how bindings are validated

- closing with broken expression will show errors and prevent close
- the next close will not block and keep the old valid expression
- close with valid expression invoked saving
- props no longer can change the type of input
- binding button becomes red and gets tooltip when value has invalid
type
- binding popover also shows non-blocking error about type
- each control provides own validation

<img width="465" alt="image"
src="https://github.com/webstudio-is/webstudio/assets/5635476/c19cca91-4558-4570-b257-1c10689a1113">
<img width="247" alt="Screenshot 2024-01-05 at 17 13 15"
src="https://github.com/webstudio-is/webstudio/assets/5635476/8702b2f2-1756-4262-924f-83f1d92eb4b4">

apps/builder/app/builder/features/settings-panel/controls/boolean.tsx

@@ -19,8 +19,9 @@ export const BooleanControl = ({
   readOnly,
   onChange,
   onDelete,
-}: ControlProps<"boolean", "boolean" | "expression">) => {
+}: ControlProps<"boolean">) => {
   const id = useId();
+  const label = getLabel(meta, propName);
   const { scope, aliases } = useStore($selectedInstanceScope);
   const expression =
     prop?.type === "expression" ? prop.value : JSON.stringify(computedValue);
@@ -38,7 +39,7 @@ export const BooleanControl = ({
       gap="2"
     >
       <Label htmlFor={id} description={meta.description} readOnly={readOnly}>
-        {getLabel(meta, propName)}
+        {label}
       </Label>
       <Box css={{ position: "relative" }}>
         <Switch
@@ -56,6 +57,11 @@ export const BooleanControl = ({
         <BindingPopover
           scope={scope}
           aliases={aliases}
+          validate={(value) => {
+            if (value !== undefined && typeof value !== "boolean") {
+              return `${label} expects a boolean value`;
+            }
+          }}
           value={expression}
           onChange={(newExpression) =>
             onChange({ type: "expression", value: newExpression })

apps/builder/app/builder/features/settings-panel/controls/check.tsx

@@ -40,10 +40,7 @@ export const CheckControl = ({
   readOnly,
   onChange,
   onDelete,
-}: ControlProps<
-  "check" | "inline-check" | "multi-select",
-  "string[]" | "expression"
->) => {
+}: ControlProps<"check" | "inline-check" | "multi-select">) => {
   const value = Array.isArray(computedValue)
     ? computedValue.map((item) => String(item))
     : [];
@@ -52,6 +49,7 @@ export const CheckControl = ({
   const options = Array.from(new Set([...meta.options, ...value]));
 
   const id = useId();
+  const label = getLabel(meta, propName);
   const { scope, aliases } = useStore($selectedInstanceScope);
   const expression =
     prop?.type === "expression" ? prop.value : JSON.stringify(computedValue);
@@ -64,7 +62,7 @@ export const CheckControl = ({
           description={meta.description}
           readOnly={readOnly}
         >
-          {getLabel(meta, propName)}
+          {label}
         </Label>
       }
       deletable={deletable}
@@ -94,6 +92,14 @@ export const CheckControl = ({
         <BindingPopover
           scope={scope}
           aliases={aliases}
+          validate={(value) => {
+            const valid =
+              Array.isArray(value) &&
+              value.every((item) => typeof item === "string");
+            if (value !== undefined && valid === false) {
+              return `${label} expects an array of strings`;
+            }
+          }}
           value={expression}
           onChange={(newExpression) =>
             onChange({ type: "expression", value: newExpression })

apps/builder/app/builder/features/settings-panel/controls/code.tsx

@@ -21,7 +21,7 @@ export const CodeControl = ({
   readOnly,
   onChange,
   onDelete,
-}: ControlProps<"code", "string" | "expression">) => {
+}: ControlProps<"code">) => {
   const metaOverride = {
     ...meta,
     control: "text" as const,
@@ -65,6 +65,11 @@ export const CodeControl = ({
         <BindingPopover
           scope={scope}
           aliases={aliases}
+          validate={(value) => {
+            if (value !== undefined && typeof value !== "string") {
+              return `${label} expects a string value`;
+            }
+          }}
           value={expression}
           onChange={(newExpression) =>
             onChange({ type: "expression", value: newExpression })

apps/builder/app/builder/features/settings-panel/controls/combined.tsx

@@ -14,7 +14,7 @@ export const renderControl = ({
   meta,
   prop,
   ...rest
-}: ControlProps<string, string> & { key?: string }) => {
+}: ControlProps<string> & { key?: string }) => {
   const computed = rest.computedValue;
 
   // never render parameter props
@@ -27,107 +27,53 @@ export const renderControl = ({
     return;
   }
 
-  if (
-    meta.control === "json" &&
-    (prop === undefined ||
-      prop.type === "json" ||
-      (prop.type === "expression" && typeof computed === "object"))
-  ) {
+  if (meta.control === "json") {
     return <JsonControl meta={meta} prop={prop} {...rest} />;
   }
 
-  if (
-    meta.control === "text" &&
-    (prop === undefined ||
-      prop.type === "string" ||
-      (prop.type === "expression" && typeof computed === "string"))
-  ) {
+  if (meta.control === "text") {
     return <TextControl meta={meta} prop={prop} {...rest} />;
   }
 
-  if (
-    meta.control === "code" &&
-    (prop === undefined ||
-      prop.type === "string" ||
-      (prop.type === "expression" && typeof computed === "string"))
-  ) {
+  if (meta.control === "code") {
     return <CodeControl meta={meta} prop={prop} {...rest} />;
   }
 
-  if (
-    meta.control === "color" &&
-    (prop === undefined || prop.type === "string")
-  ) {
+  if (meta.control === "color") {
     return (
       <TextControl meta={{ ...meta, control: "text" }} prop={prop} {...rest} />
     );
   }
 
-  if (
-    meta.control === "number" &&
-    (prop === undefined ||
-      prop.type === "number" ||
-      (prop.type === "expression" && typeof computed === "number"))
-  ) {
+  if (meta.control === "number") {
     return <NumberControl meta={meta} prop={prop} {...rest} />;
   }
 
-  if (
-    meta.control === "boolean" &&
-    (prop === undefined ||
-      prop.type === "boolean" ||
-      (prop.type === "expression" && typeof computed === "boolean"))
-  ) {
+  if (meta.control === "boolean") {
     return <BooleanControl meta={meta} prop={prop} {...rest} />;
   }
 
   if (
-    (meta.control === "check" ||
-      meta.control === "inline-check" ||
-      meta.control === "multi-select") &&
-    (prop === undefined ||
-      prop.type === "string[]" ||
-      (prop.type === "expression" && typeof computed === "object"))
+    meta.control === "check" ||
+    meta.control === "inline-check" ||
+    meta.control === "multi-select"
   ) {
     return <CheckControl meta={meta} prop={prop} {...rest} />;
   }
 
-  if (
-    (meta.control === "radio" || meta.control === "inline-radio") &&
-    (prop === undefined ||
-      prop.type === "string" ||
-      (prop.type === "expression" && typeof computed === "string"))
-  ) {
+  if (meta.control === "radio" || meta.control === "inline-radio") {
     return <RadioControl meta={meta} prop={prop} {...rest} />;
   }
 
-  if (
-    meta.control === "select" &&
-    (prop === undefined ||
-      prop.type === "string" ||
-      (prop.type === "expression" && typeof computed === "string"))
-  ) {
+  if (meta.control === "select") {
     return <SelectControl meta={meta} prop={prop} {...rest} />;
   }
 
-  if (
-    meta.control === "file" &&
-    (prop === undefined ||
-      prop.type === "asset" ||
-      prop.type === "string" ||
-      (prop.type === "expression" && typeof computed === "string"))
-  ) {
+  if (meta.control === "file") {
     return <FileControl meta={meta} prop={prop} {...rest} />;
   }
 
-  if (
-    meta.control === "url" &&
-    (prop === undefined ||
-      prop.type === "string" ||
-      prop.type === "page" ||
-      prop.type === "asset" ||
-      (prop.type === "expression" && typeof computed === "string"))
-  ) {
+  if (meta.control === "url") {
     return <UrlControl meta={meta} prop={prop} {...rest} />;
   }
 

apps/builder/app/builder/features/settings-panel/controls/file.tsx

@@ -56,7 +56,7 @@ export const FileControl = ({
   deletable,
   onChange,
   onDelete,
-}: ControlProps<"file", "asset" | "string" | "expression">) => {
+}: ControlProps<"file">) => {
   const id = useId();
 
   const localStringValue = useLocalValue(
@@ -78,6 +78,7 @@ export const FileControl = ({
     }
   );
 
+  const label = getLabel(meta, propName);
   const { scope, aliases } = useStore($selectedInstanceScope);
   const expression =
     prop?.type === "expression" ? prop.value : JSON.stringify(computedValue);
@@ -86,7 +87,7 @@ export const FileControl = ({
     <VerticalLayout
       label={
         <Label htmlFor={id} description={meta.description}>
-          {getLabel(meta, propName)}
+          {label}
         </Label>
       }
       deletable={deletable}
@@ -97,6 +98,11 @@ export const FileControl = ({
         <BindingPopover
           scope={scope}
           aliases={aliases}
+          validate={(value) => {
+            if (value !== undefined && typeof value !== "string") {
+              return `${label} expects a string value or file`;
+            }
+          }}
           value={expression}
           onChange={(newExpression) =>
             onChange({ type: "expression", value: newExpression })

apps/builder/app/builder/features/settings-panel/controls/json.tsx

@@ -24,7 +24,7 @@ export const JsonControl = ({
   readOnly,
   onChange,
   onDelete,
-}: ControlProps<"json", "json" | "expression">) => {
+}: ControlProps<"json">) => {
   const valueString = formatValue(computedValue ?? "");
   const localValue = useLocalValue(valueString, (value) => {
     try {

apps/builder/app/builder/features/settings-panel/controls/number.tsx

@@ -21,7 +21,7 @@ export const NumberControl = ({
   deletable,
   readOnly,
   onDelete,
-}: ControlProps<"number", "number" | "expression">) => {
+}: ControlProps<"number">) => {
   const id = useId();
 
   const [isInvalid, setIsInvalid] = useState(false);
@@ -42,6 +42,7 @@ export const NumberControl = ({
     }
   );
 
+  const label = getLabel(meta, propName);
   const { scope, aliases } = useStore($selectedInstanceScope);
   const expression =
     prop?.type === "expression" ? prop.value : JSON.stringify(computedValue);
@@ -50,7 +51,7 @@ export const NumberControl = ({
     <ResponsiveLayout
       label={
         <Label htmlFor={id} description={meta.description} readOnly={readOnly}>
-          {getLabel(meta, propName)}
+          {label}
         </Label>
       }
       deletable={deletable}
@@ -77,6 +78,11 @@ export const NumberControl = ({
         <BindingPopover
           scope={scope}
           aliases={aliases}
+          validate={(value) => {
+            if (value !== undefined && typeof value !== "number") {
+              return `${label} expects a number value`;
+            }
+          }}
           value={expression}
           onChange={(newExpression) =>
             onChange({ type: "expression", value: newExpression })

apps/builder/app/builder/features/settings-panel/controls/radio.tsx

@@ -27,7 +27,7 @@ export const RadioControl = ({
   readOnly,
   onChange,
   onDelete,
-}: ControlProps<"radio" | "inline-radio", "string" | "expression">) => {
+}: ControlProps<"radio" | "inline-radio">) => {
   const value = computedValue === undefined ? undefined : String(computedValue);
   // making sure that the current value is in the list of options
   const options =
@@ -36,6 +36,7 @@ export const RadioControl = ({
       : [value, ...meta.options];
 
   const id = useId();
+  const label = getLabel(meta, propName);
   const { scope, aliases } = useStore($selectedInstanceScope);
   const expression =
     prop?.type === "expression" ? prop.value : JSON.stringify(computedValue);
@@ -44,7 +45,7 @@ export const RadioControl = ({
     <VerticalLayout
       label={
         <Label htmlFor={id} description={meta.description} readOnly={readOnly}>
-          {getLabel(meta, propName)}
+          {label}
         </Label>
       }
       deletable={deletable}
@@ -73,6 +74,18 @@ export const RadioControl = ({
         <BindingPopover
           scope={scope}
           aliases={aliases}
+          validate={(value) => {
+            if (
+              value !== undefined &&
+              meta.options.includes(String(value)) === false
+            ) {
+              const formatter = new Intl.ListFormat(undefined, {
+                type: "disjunction",
+              });
+              const options = formatter.format(meta.options);
+              return `${label} expects one of ${options}`;
+            }
+          }}
           value={expression}
           onChange={(newExpression) =>
             onChange({ type: "expression", value: newExpression })

apps/builder/app/builder/features/settings-panel/controls/select-asset.tsx

@@ -7,12 +7,13 @@ import {
   SmallIconButton,
   Text,
 } from "@webstudio-is/design-system";
+import { TrashIcon } from "@webstudio-is/icons";
+import type { Prop } from "@webstudio-is/sdk";
 import { assetsStore } from "~/shared/nano-states";
 import { FloatingPanel } from "~/builder/shared/floating-panel";
 import { ImageManager } from "~/builder/shared/image-manager";
 import { type ControlProps } from "../shared";
 import { acceptToMimeCategories } from "@webstudio-is/asset-uploader";
-import { TrashIcon } from "@webstudio-is/icons";
 
 // tests whether we can use ImageManager for the given "accept" value
 const isImageAccept = (accept?: string) => {
@@ -23,11 +24,11 @@ const isImageAccept = (accept?: string) => {
   );
 };
 
-type AssetControlProps = ControlProps<unknown, "asset">;
+type AssetControlProps = ControlProps<unknown>;
 
 type Props = {
   accept?: string;
-  prop: AssetControlProps["prop"];
+  prop?: Extract<Prop, { type: "asset" }>;
   onChange: AssetControlProps["onChange"];
   onDelete: AssetControlProps["onDelete"];
 };

apps/builder/app/builder/features/settings-panel/controls/select.tsx

@@ -21,7 +21,7 @@ export const SelectControl = ({
   readOnly,
   onChange,
   onDelete,
-}: ControlProps<"select", "string" | "expression">) => {
+}: ControlProps<"select">) => {
   const id = useId();
 
   const value = computedValue === undefined ? undefined : String(computedValue);
@@ -31,6 +31,7 @@ export const SelectControl = ({
       ? meta.options
       : [value, ...meta.options];
 
+  const label = getLabel(meta, propName);
   const { scope, aliases } = useStore($selectedInstanceScope);
   const expression =
     prop?.type === "expression" ? prop.value : JSON.stringify(computedValue);
@@ -39,7 +40,7 @@ export const SelectControl = ({
     <VerticalLayout
       label={
         <Label htmlFor={id} description={meta.description} readOnly={readOnly}>
-          {getLabel(meta, propName)}
+          {label}
         </Label>
       }
       deletable={deletable}
@@ -64,6 +65,18 @@ export const SelectControl = ({
         <BindingPopover
           scope={scope}
           aliases={aliases}
+          validate={(value) => {
+            if (
+              value !== undefined &&
+              meta.options.includes(String(value)) === false
+            ) {
+              const formatter = new Intl.ListFormat(undefined, {
+                type: "disjunction",
+              });
+              const options = formatter.format(meta.options);
+              return `${label} expects one of ${options}`;
+            }
+          }}
           value={expression}
           onChange={(newExpression) =>
             onChange({ type: "expression", value: newExpression })

apps/builder/app/builder/features/settings-panel/controls/text.tsx

@@ -21,7 +21,7 @@ export const TextControl = ({
   readOnly,
   onChange,
   onDelete,
-}: ControlProps<"text", "string" | "expression">) => {
+}: ControlProps<"text">) => {
   const localValue = useLocalValue(String(computedValue ?? ""), (value) => {
     if (prop?.type === "expression") {
       updateExpressionValue(prop.value, value);
@@ -55,6 +55,11 @@ export const TextControl = ({
       <BindingPopover
         scope={scope}
         aliases={aliases}
+        validate={(value) => {
+          if (value !== undefined && typeof value !== "string") {
+            return `${label} expects a string value`;
+          }
+        }}
         value={expression}
         onChange={(newExpression) =>
           onChange({ type: "expression", value: newExpression })

apps/builder/app/builder/features/settings-panel/controls/url.tsx

@@ -34,10 +34,7 @@ import {
 } from "../shared";
 import { SelectAsset } from "./select-asset";
 
-type UrlControlProps = ControlProps<
-  "url",
-  "string" | "page" | "asset" | "expression"
->;
+type UrlControlProps = ControlProps<"url">;
 
 type BaseControlProps = {
   id: string;
@@ -444,6 +441,7 @@ export const UrlControl = ({
 
   const BaseControl = modes[mode].control;
 
+  const label = getLabel(meta, propName);
   const { scope, aliases } = useStore($selectedInstanceScope);
   const expression =
     prop?.type === "expression" ? prop.value : JSON.stringify(computedValue);
@@ -452,7 +450,7 @@ export const UrlControl = ({
     <VerticalLayout
       label={
         <Label htmlFor={id} description={meta.description}>
-          {getLabel(meta, propName)}
+          {label}
         </Label>
       }
       deletable={deletable}
@@ -498,6 +496,11 @@ export const UrlControl = ({
         <BindingPopover
           scope={scope}
           aliases={aliases}
+          validate={(value) => {
+            if (value !== undefined && typeof value !== "string") {
+              return `${label} expects a string value, page or file`;
+            }
+          }}
           value={expression}
           onChange={(newExpression) =>
             onChange({ type: "expression", value: newExpression })

apps/builder/app/builder/features/settings-panel/props-section/props-section.tsx

@@ -1,4 +1,4 @@
-import { Fragment, useState } from "react";
+import { useState } from "react";
 import { useStore } from "@nanostores/react";
 import type { Instance } from "@webstudio-is/sdk";
 import {
@@ -98,55 +98,48 @@ const renderProperty = (
   }: PropsSectionProps,
   { prop, propName, meta, readOnly }: PropAndMeta,
   deletable?: boolean
-) => (
-  // fix the issue with changing type while binding expression
-  // old prop value is getting preserved in useLocalValue and saved into variable
-  // when unmount to reproduce try to edit body id prop
-  // and then bind json variable to it
-  <Fragment key={(prop?.type ?? "") + propName}>
-    {renderControl({
-      key: propName,
-      instanceId,
-      meta,
-      prop,
-      computedValue: propValues.get(propName),
-      propName,
-      readOnly,
-      deletable: deletable ?? false,
-      onDelete: () => {
-        if (prop) {
-          logic.handleDelete(prop);
-        }
-      },
-      onChange: (propValue, asset) => {
-        logic.handleChange({ prop, propName }, propValue);
+) =>
+  renderControl({
+    key: propName,
+    instanceId,
+    meta,
+    prop,
+    computedValue: propValues.get(propName),
+    propName,
+    readOnly,
+    deletable: deletable ?? false,
+    onDelete: () => {
+      if (prop) {
+        logic.handleDelete(prop);
+      }
+    },
+    onChange: (propValue, asset) => {
+      logic.handleChange({ prop, propName }, propValue);
 
-        // @todo: better way to do this?
-        if (
-          component === "Image" &&
-          propName === "src" &&
-          asset &&
-          "width" in asset.meta &&
-          "height" in asset.meta
-        ) {
-          logic.handleChangeByPropName("width", {
-            value: asset.meta.width,
-            type: "number",
-          });
-          logic.handleChangeByPropName("height", {
-            value: asset.meta.height,
-            type: "number",
-          });
+      // @todo: better way to do this?
+      if (
+        component === "Image" &&
+        propName === "src" &&
+        asset &&
+        "width" in asset.meta &&
+        "height" in asset.meta
+      ) {
+        logic.handleChangeByPropName("width", {
+          value: asset.meta.width,
+          type: "number",
+        });
+        logic.handleChangeByPropName("height", {
+          value: asset.meta.height,
+          type: "number",
+        });
 
-          setCssProperty("height")({
-            type: "keyword",
-            value: "fit-content",
-          });
-        }
-      },
-    })}
-  </Fragment>
-);
+        setCssProperty("height")({
+          type: "keyword",
+          value: "fit-content",
+        });
+      }
+    },
+  });
 
 const AddPropertyForm = ({
   availableProps,

apps/builder/app/builder/features/settings-panel/shared.tsx

@@ -52,16 +52,12 @@ type PropMetaByControl<Control> = Control extends string
   ? Extract<PropMeta, { control: Control }>
   : never;
 
-type PropByType<Type> = Type extends string
-  ? Extract<Prop, { type: Type }>
-  : never;
-
-export type ControlProps<Control, PropType> = {
+export type ControlProps<Control> = {
   instanceId: string;
   meta: PropMetaByControl<Control>;
   // prop is optional because we don't have it when an intial prop is not set
   // and we don't want to show user something like a 0 for number when it's in fact not set to any value
-  prop: PropByType<PropType> | undefined;
+  prop: Prop | undefined;
   propName: string;
   computedValue: unknown;
   deletable: boolean;

apps/builder/app/builder/shared/binding-popover.tsx

@@ -1,4 +1,10 @@
-import { forwardRef, useMemo, useRef, useState } from "react";
+import {
+  forwardRef,
+  useMemo,
+  useRef,
+  useState,
+  type ButtonHTMLAttributes,
+} from "react";
 import { isFeatureEnabled } from "@webstudio-is/feature-flags";
 import { DotIcon, PlusIcon, TrashIcon } from "@webstudio-is/icons";
 import {
@@ -75,25 +81,26 @@ const getUsedIdentifiers = (expression: string) => {
 const BindingPanel = ({
   scope,
   aliases,
-  error,
+  valueError,
   value,
   onChange,
-  onPotentialErrorChange,
-  onValidate,
+  onSave,
 }: {
   scope: Record<string, unknown>;
   aliases: Map<string, string>;
-  error: undefined | string;
+  valueError?: string;
   value: string;
-  onChange: (newValue: string) => void;
-  onPotentialErrorChange: (error: string) => void;
-  onValidate: () => void;
+  onChange: () => void;
+  onSave: (value: string, invalid: boolean) => void;
 }) => {
   const [expression, setExpression] = useState(value);
   const usedIdentifiers = useMemo(() => getUsedIdentifiers(value), [value]);
+  const [error, setError] = useState<undefined | string>();
+  const [touched, setTouched] = useState(false);
 
   const updateExpression = (newExpression: string) => {
     setExpression(newExpression);
+    onChange();
     try {
       if (newExpression.trim().length === 0) {
         throw Error("Cannot use empty expression");
@@ -107,9 +114,9 @@ const BindingPanel = ({
           return identifier;
         },
       });
-      onChange(newExpression);
+      setError(undefined);
     } catch (error) {
-      onPotentialErrorChange((error as Error).message);
+      setError((error as Error).message);
     }
   };
 
@@ -139,8 +146,8 @@ const BindingPanel = ({
                 active={usedIdentifiers.has(identifier)}
                 // convert variable to expression
                 onClick={() => {
-                  setExpression(identifier);
-                  onChange(identifier);
+                  updateExpression(identifier);
+                  onSave(identifier, false);
                 }}
               />
             );
@@ -148,15 +155,30 @@ const BindingPanel = ({
         </CssValueListArrowFocus>
       </Box>
       <Box css={{ padding: `0 ${theme.spacing[9]} ${theme.spacing[9]}` }}>
-        <InputErrorsTooltip errors={error ? [error] : undefined}>
+        <InputErrorsTooltip
+          errors={
+            touched && error ? [error] : valueError ? [valueError] : undefined
+          }
+        >
           <div>
             <ExpressionEditor
               scope={scope}
               aliases={aliases}
+              color={
+                error !== undefined || valueError !== undefined
+                  ? "error"
+                  : undefined
+              }
               autoFocus={true}
               value={expression}
-              onChange={updateExpression}
-              onBlur={onValidate}
+              onChange={(value) => {
+                updateExpression(value);
+                setTouched(false);
+              }}
+              onBlur={() => {
+                onSave(expression, error !== undefined);
+                setTouched(true);
+              }}
             />
           </div>
         </InputErrorsTooltip>
@@ -165,107 +187,137 @@ const BindingPanel = ({
   );
 };
 
-const BindingButton = forwardRef<HTMLButtonElement>((props, ref) => (
-  <SmallIconButton
-    ref={ref}
-    css={{
-      position: "absolute",
-      top: 0,
-      left: 0,
-      boxSizing: "border-box",
-      padding: 2,
-      transform: "translate(-50%, -50%) scale(1)",
-      transition: "transform 60ms",
-      "--dot-display": "block",
-      "--plus-display": "none",
-      "&:hover, &[aria-expanded=true]": {
-        transform: `translate(-50%, -50%) scale(1.5)`,
-        "--dot-display": "none",
-        "--plus-display": "block",
-      },
-    }}
-    {...props}
-    icon={
-      <Box
+const BindingButton = forwardRef<
+  HTMLButtonElement,
+  ButtonHTMLAttributes<HTMLButtonElement> & { error?: string }
+>(({ error, ...props }, ref) => {
+  const expanded = props["aria-expanded"];
+  return (
+    // prevent giving content to tooltip when popover is open
+    // to avoid button remounting and popover flickering
+    // when switch between valid and error value
+    <Tooltip content={expanded ? undefined : error} delayDuration={0}>
+      <SmallIconButton
+        ref={ref}
         css={{
-          width: 12,
-          height: 12,
-          borderRadius: "50%",
-          backgroundColor: "#834DF4",
-          display: "flex",
-          justifyContent: "center",
-          alignItems: "center",
+          position: "absolute",
+          top: 0,
+          left: 0,
+          boxSizing: "border-box",
+          padding: 2,
+          transform: "translate(-50%, -50%) scale(1)",
+          transition: "transform 60ms",
+          "--dot-display": "block",
+          "--plus-display": "none",
+          "&:hover, &[aria-expanded=true]": {
+            transform: `translate(-50%, -50%) scale(1.5)`,
+            "--dot-display": "none",
+            "--plus-display": "block",
+          },
         }}
-      >
-        <DotIcon
-          size={14}
-          fill="white"
-          style={{ display: `var(--dot-display)` }}
-        />
-        <PlusIcon
-          size={10}
-          fill="white"
-          style={{ display: `var(--plus-display)` }}
-        />
-      </Box>
-    }
-  />
-));
+        {...props}
+        icon={
+          <Box
+            css={{
+              width: 12,
+              height: 12,
+              borderRadius: "50%",
+              backgroundColor: "#834DF4",
+              display: "flex",
+              justifyContent: "center",
+              alignItems: "center",
+              "&[data-variant=error]": {
+                backgroundColor: theme.colors.backgroundDestructiveMain,
+              },
+            }}
+            data-variant={error ? "error" : "default"}
+          >
+            <DotIcon
+              size={14}
+              fill="white"
+              style={{ display: `var(--dot-display)` }}
+            />
+            <PlusIcon
+              size={10}
+              fill="white"
+              style={{ display: `var(--plus-display)` }}
+            />
+          </Box>
+        }
+      />
+    </Tooltip>
+  );
+});
 BindingButton.displayName = "BindingButton";
 
 export const BindingPopover = ({
   scope,
   aliases,
+  validate,
   value,
   onChange,
   onRemove,
 }: {
   scope: Record<string, unknown>;
   aliases: Map<string, string>;
-  acceptableType?: "text" | "any";
+  validate?: (value: unknown) => undefined | string;
   value: string;
   onChange: (newValue: string) => void;
   onRemove: (evaluatedValue: unknown) => void;
 }) => {
   const [open, onOpenChange] = useState(false);
-  const [error, setError] = useState<undefined | string>();
-  const potentialError = useRef<undefined | string>();
+  const hasUnsavedChange = useRef<boolean>(false);
+  const preventedClosing = useRef<boolean>(false);
 
   if (isFeatureEnabled("bindings") === false) {
     return;
   }
+  const valueError = validate?.(evaluateExpressionWithinScope(value, scope));
   return (
     <FloatingPanelPopover
       modal
       open={open}
       onOpenChange={(newOpen) => {
-        if (potentialError.current) {
-          setError(potentialError.current);
-        } else {
-          onOpenChange(newOpen);
+        // handle special case for popover close
+        if (newOpen === false) {
+          // prevent saving when changes are not saved or validated
+          if (hasUnsavedChange.current) {
+            // schedule closing after saving
+            preventedClosing.current = true;
+            return;
+          }
+          preventedClosing.current = false;
         }
+        onOpenChange(newOpen);
       }}
     >
       <FloatingPanelPopoverTrigger asChild>
-        <BindingButton />
+        <BindingButton error={valueError} />
       </FloatingPanelPopoverTrigger>
       <FloatingPanelPopoverContent side="top" align="start">
         <BindingPanel
           scope={scope}
           aliases={aliases}
-          error={error}
+          valueError={valueError}
           value={value}
-          onChange={(newValue) => {
-            potentialError.current = undefined;
-            setError(undefined);
-            onChange(newValue);
+          onChange={() => {
+            hasUnsavedChange.current = true;
           }}
-          onPotentialErrorChange={(error) => {
-            potentialError.current = error;
-          }}
-          onValidate={() => {
-            if (potentialError.current) {
-              setError(potentialError.current);
+          onSave={(value, invalid) => {
+            // avoid saving without changes
+            if (hasUnsavedChange.current === false) {
+              return;
+            }
+            // let user see the error and let close popover after
+            hasUnsavedChange.current = false;
+            if (invalid) {
+              return;
+            }
+            // save value and close popover
+            onChange(value);
+            if (preventedClosing.current) {
+              preventedClosing.current = false;
+              onOpenChange(false);
             }
           }}
         />
@@ -279,15 +331,16 @@ export const BindingPopover = ({
                   aria-label="Reset binding"
                   prefix={<TrashIcon />}
                   color="ghost"
-                  onClick={() => {
-                    // reset error, inline variables and close dialog
-                    potentialError.current = undefined;
-                    setError(undefined);
+                  onClick={(event) => {
+                    event.preventDefault();
+                    // inline variables and close dialog
                     const evaluatedValue = evaluateExpressionWithinScope(
                       value,
                       scope
                     );
                     onRemove(evaluatedValue);
+                    preventedClosing.current = false;
+                    hasUnsavedChange.current = false;
                     onOpenChange(false);
                   }}
                 />

apps/builder/app/builder/shared/code-editor.tsx

@@ -38,6 +38,7 @@ const rootStyle = css({
 
 export const CodeEditor = ({
   extensions,
+  className,
   readOnly = false,
   autoFocus = false,
   value,
@@ -45,6 +46,7 @@ export const CodeEditor = ({
   onBlur,
 }: {
   extensions: Extension[];
+  className?: string;
   readOnly?: boolean;
   autoFocus?: boolean;
   value: string;
@@ -130,5 +132,9 @@ export const CodeEditor = ({
     });
   }, [value]);
 
-  return <div className={rootStyle.toString()} ref={editorRef}></div>;
+  let rootClassName = rootStyle.toString();
+  if (className) {
+    rootClassName += ` ${className}`;
+  }
+  return <div className={rootClassName} ref={editorRef}></div>;
 };

apps/builder/app/builder/shared/expression-editor.tsx

@@ -212,9 +212,14 @@ const autocompletionStyle = css({
 const emptyScope: Scope = {};
 const emptyAliases: Aliases = new Map();
 
+const errorStyle = css({
+  borderColor: theme.colors.borderDestructiveMain,
+});
+
 export const ExpressionEditor = ({
   scope = emptyScope,
   aliases = emptyAliases,
+  color,
   autoFocus = false,
   readOnly = false,
   value,
@@ -229,6 +234,7 @@ export const ExpressionEditor = ({
    * variable aliases to show instead of $ws$dataSource$id
    */
   aliases?: Aliases;
+  color?: "error";
   autoFocus?: boolean;
   readOnly?: boolean;
   value: string;
@@ -275,6 +281,7 @@ export const ExpressionEditor = ({
   return (
     <CodeEditor
       extensions={extensions}
+      className={color === "error" ? errorStyle.toString() : undefined}
       readOnly={readOnly}
       autoFocus={autoFocus}
       value={value}