fix: make all radix components uncontrolled (#5227)

Here improved handling radix states by making all components
uncontrolled yet still synchronizing external state with local one.

Author: TrySound

apps/builder/app/shared/content-model.ts

@@ -424,7 +424,7 @@ export const richTextPlaceholders: Map<undefined | string, string> = new Map([
   ["blockquote", "Blockquote"],
   ["li", "List item"],
   ["a", "Link"],
-  ["span", "Span"],
+  ["span", ""],
 ]);
 
 const findContentTags = ({

packages/sdk-components-react-radix/package.json

@@ -58,7 +58,6 @@
     "@radix-ui/react-switch": "^1.2.2",
     "@radix-ui/react-tabs": "^1.1.9",
     "@radix-ui/react-tooltip": "^1.2.4",
-    "@radix-ui/react-use-controllable-state": "^1.2.2",
     "@webstudio-is/css-engine": "workspace:*",
     "@webstudio-is/icons": "workspace:*",
     "@webstudio-is/react-sdk": "workspace:*",

packages/sdk-components-react-radix/src/__generated__/tabs.props.ts

@@ -98,7 +98,7 @@ export const meta: TemplateMeta = {
     "A vertically stacked set of interactive headings that each reveal an associated section of content. Clicking on the heading will open the item and close other items.",
   order: 3,
   template: (
-    <radix.Accordion collapsible={true} defaultValue="0">
+    <radix.Accordion collapsible={true} value="0">
       {createAccordionItem(
         "Is it accessible?",
         "Yes. It adheres to the WAI-ARIA design pattern."

packages/sdk-components-react-radix/src/accordion.template.tsx

@@ -4,6 +4,8 @@ import {
   forwardRef,
   type ComponentProps,
   type RefAttributes,
+  useState,
+  useEffect,
 } from "react";
 import {
   Root,
@@ -21,8 +23,20 @@ export const Accordion = forwardRef<
     Extract<ComponentPropsWithoutRef<typeof Root>, { type: "single" }>,
     "type" | "asChild"
   >
->((props, ref) => {
-  return <Root ref={ref} type="single" {...props} />;
+>(({ defaultValue, ...props }, ref) => {
+  const currentValue = props.value ?? defaultValue ?? "";
+  const [value, setValue] = useState(currentValue);
+  // synchronize external value with local one when changed
+  useEffect(() => setValue(currentValue), [currentValue]);
+  return (
+    <Root
+      {...props}
+      ref={ref}
+      type="single"
+      value={value}
+      onValueChange={setValue}
+    />
+  );
 });
 
 export const AccordionItem = forwardRef<

packages/sdk-components-react-radix/src/accordion.tsx

@@ -3,9 +3,10 @@ import {
   type ComponentPropsWithRef,
   forwardRef,
   type ComponentProps,
+  useState,
+  useEffect,
 } from "react";
 import { Root, Indicator } from "@radix-ui/react-checkbox";
-import { useControllableState } from "@radix-ui/react-use-controllable-state";
 
 export const Checkbox = forwardRef<
   HTMLButtonElement,
@@ -16,16 +17,16 @@ export const Checkbox = forwardRef<
     defaultChecked?: boolean;
   }
 >(({ defaultChecked, ...props }, ref) => {
-  const [checked, onCheckedChange] = useControllableState({
-    prop: props.checked ?? defaultChecked ?? false,
-    defaultProp: false,
-  });
+  const currentChecked = props.checked ?? defaultChecked ?? false;
+  const [checked, setChecked] = useState(currentChecked);
+  // synchronize external value with local one when changed
+  useEffect(() => setChecked(currentChecked), [currentChecked]);
   return (
     <Root
       {...props}
       ref={ref}
       checked={checked}
-      onCheckedChange={(open) => onCheckedChange(open === true)}
+      onCheckedChange={(open) => setChecked(open === true)}
     />
   );
 });

packages/sdk-components-react-radix/src/checkbox.tsx

@@ -5,14 +5,22 @@ import {
   Children,
   type ComponentProps,
   type RefAttributes,
+  useState,
+  useEffect,
 } from "react";
 import { Root, Trigger, Content } from "@radix-ui/react-collapsible";
 import { type Hook, getClosestInstance } from "@webstudio-is/react-sdk/runtime";
 
-export const Collapsible: ForwardRefExoticComponent<
-  Omit<ComponentProps<typeof Root>, "defaultOpen" | "asChild"> &
-    RefAttributes<HTMLDivElement>
-> = Root;
+export const Collapsible = forwardRef<
+  HTMLDivElement,
+  Omit<ComponentProps<typeof Root>, "defaultOpen" | "asChild">
+>((props, ref) => {
+  const currentOpen = props.open ?? false;
+  const [open, setOpen] = useState(currentOpen);
+  // synchronize external value with local one when changed
+  useEffect(() => setOpen(currentOpen), [currentOpen]);
+  return <Root {...props} ref={ref} open={open} onOpenChange={setOpen} />;
+});
 
 /**
  * We're not exposing the 'asChild' property for the Trigger.

packages/sdk-components-react-radix/src/collapsible.tsx

@@ -1,22 +1,21 @@
+import interactionResponse from "await-interaction-response";
 import {
-  type ComponentPropsWithoutRef,
   type ReactNode,
+  type ComponentProps,
   forwardRef,
   Children,
-  type ComponentProps,
   useEffect,
   useRef,
   useContext,
   useCallback,
+  useState,
 } from "react";
 import * as DialogPrimitive from "@radix-ui/react-dialog";
 import {
   ReactSdkContext,
   getClosestInstance,
   type Hook,
 } from "@webstudio-is/react-sdk/runtime";
-import { useControllableState } from "@radix-ui/react-use-controllable-state";
-import interactionResponse from "await-interaction-response";
 
 /**
  * Naive heuristic to determine if a click event will cause navigate
@@ -50,23 +49,19 @@ const willNavigate = (event: MouseEvent) => {
 // wrap in forwardRef because Root is functional component without ref
 export const Dialog = forwardRef<
   HTMLDivElement,
-  Omit<ComponentPropsWithoutRef<typeof DialogPrimitive.Root>, "defaultOpen">
+  Omit<ComponentProps<typeof DialogPrimitive.Root>, "defaultOpen">
 >((props, _ref) => {
   const { renderer } = useContext(ReactSdkContext);
 
-  const [open, onOpenChange] = useControllableState({
-    prop: props.open,
-    defaultProp: false,
-    onChange: props.onOpenChange,
-  });
-
-  const onOpenChangeHandler = useCallback(
-    async (open: boolean) => {
-      await interactionResponse();
-      onOpenChange(open);
-    },
-    [onOpenChange]
-  );
+  const currentOpen = props.open ?? false;
+  const [open, setOpen] = useState(currentOpen);
+  // synchronize external value with local one when changed
+  useEffect(() => setOpen(currentOpen), [currentOpen]);
+
+  const onOpenChangeHandler = useCallback(async (open: boolean) => {
+    await interactionResponse();
+    setOpen(open);
+  }, []);
 
   /**
    * Close the dialog when a navigable link within it is clicked.
@@ -130,7 +125,7 @@ export const DialogTrigger = forwardRef<
 
 export const DialogOverlay = forwardRef<
   HTMLDivElement,
-  ComponentPropsWithoutRef<typeof DialogPrimitive.Overlay>
+  ComponentProps<typeof DialogPrimitive.Overlay>
 >((props, ref) => {
   return (
     <DialogPrimitive.DialogPortal>
@@ -141,7 +136,7 @@ export const DialogOverlay = forwardRef<
 
 export const DialogContent = forwardRef<
   HTMLDivElement,
-  ComponentPropsWithoutRef<typeof DialogPrimitive.Content>
+  ComponentProps<typeof DialogPrimitive.Content>
 >((props, ref) => {
   const preventAutoFocusOnClose = useRef(false);
   const { renderer } = useContext(ReactSdkContext);

packages/sdk-components-react-radix/src/dialog.tsx

@@ -98,5 +98,6 @@ export const metaDialog: WsComponentMeta = {
     children: ["instance"],
     descendants: [radix.DialogTrigger, radix.DialogOverlay],
   },
+  initialProps: ["open"],
   props: propsDialog,
 };

packages/sdk-components-react-radix/src/dialog.ws.ts

@@ -3,6 +3,8 @@ import {
   type ReactNode,
   forwardRef,
   Children,
+  useState,
+  useEffect,
 } from "react";
 import * as PopoverPrimitive from "@radix-ui/react-popover";
 import { getClosestInstance, type Hook } from "@webstudio-is/react-sdk/runtime";
@@ -12,7 +14,13 @@ export const Popover = forwardRef<
   HTMLDivElement,
   Omit<ComponentPropsWithoutRef<typeof PopoverPrimitive.Root>, "defaultOpen">
 >((props, _ref) => {
-  return <PopoverPrimitive.Root {...props} />;
+  const currentOpen = props.open ?? false;
+  const [open, setOpen] = useState(currentOpen);
+  // synchronize external value with local one when changed
+  useEffect(() => setOpen(currentOpen), [currentOpen]);
+  return (
+    <PopoverPrimitive.Root {...props} open={open} onOpenChange={setOpen} />
+  );
 });
 
 /**