chore: Add portal to the DropdownMenuContent by default (#4828)

## Description We should never use those menus without a portal, enforcing it by default now: - added portal to DropdownMenuContent - removed usages of the portal in consumers ## Steps for reproduction 1. click button 2. expect xyz ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 0000) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file
webstudio-is · Feb 5, 2025 · 609fdb4 · 609fdb4
1 parent 196a3d3
commit 609fdb4
Show file tree

Hide file tree

Showing 11 changed files with 454 additions and 485 deletions.
diff --git a/apps/builder/app/builder/features/settings-panel/variables-section.tsx b/apps/builder/app/builder/features/settings-panel/variables-section.tsx
@@ -9,7 +9,6 @@ import {
   DropdownMenu,
   DropdownMenuContent,
   DropdownMenuItem,
-  DropdownMenuPortal,
   DropdownMenuTrigger,
   Flex,
   Label,
@@ -257,25 +256,23 @@ const VariablesItem = ({
                   onClick={() => {}}
                 />
               </DropdownMenuTrigger>
-              <DropdownMenuPortal>
-                <DropdownMenuContent
-                  css={{ width: theme.spacing[28] }}
-                  onCloseAutoFocus={(event) => event.preventDefault()}
-                >
-                  <DropdownMenuItem onSelect={() => setInspectDialogOpen(true)}>
-                    Inspect
+              <DropdownMenuContent
+                css={{ width: theme.spacing[28] }}
+                onCloseAutoFocus={(event) => event.preventDefault()}
+              >
+                <DropdownMenuItem onSelect={() => setInspectDialogOpen(true)}>
+                  Inspect
+                </DropdownMenuItem>
+                {source === "local" && (
+                  <DropdownMenuItem
+                    // allow to delete only unused variables
+                    disabled={variable.type === "parameter" || usageCount > 0}
+                    onSelect={() => deleteVariable(variable.id)}
+                  >
+                    Delete {usageCount > 0 && `(${usageCount} bindings)`}
                   </DropdownMenuItem>
-                  {source === "local" && (
-                    <DropdownMenuItem
-                      // allow to delete only unused variables
-                      disabled={variable.type === "parameter" || usageCount > 0}
-                      onSelect={() => deleteVariable(variable.id)}
-                    >
-                      Delete {usageCount > 0 && `(${usageCount} bindings)`}
-                    </DropdownMenuItem>
-                  )}
-                </DropdownMenuContent>
-              </DropdownMenuPortal>
+                )}
+              </DropdownMenuContent>
             </DropdownMenu>
           </>
         }

diff --git a/apps/builder/app/builder/features/style-panel/controls/menu/menu-control.tsx b/apps/builder/app/builder/features/style-panel/controls/menu/menu-control.tsx
@@ -6,7 +6,6 @@ import {
   DropdownMenu,
   DropdownMenuContent,
   DropdownMenuItem,
-  DropdownMenuPortal,
   DropdownMenuRadioGroup,
   DropdownMenuRadioItem,
   DropdownMenuSeparator,
@@ -74,57 +73,55 @@ export const MenuControl = ({
           </IconButton>
         </DropdownMenuTrigger>
       </PropertyValueTooltip>
-      <DropdownMenuPortal>
-        <DropdownMenuContent sideOffset={4} collisionPadding={16} side="bottom">
-          <DropdownMenuRadioGroup
-            value={currentValue}
-            onValueChange={(value) => setValue({ type: "keyword", value })}
-          >
-            {items.map(({ name, label, icon: Icon }) => {
-              return (
-                <DropdownMenuRadioItem
-                  text="sentence"
-                  key={name}
-                  value={name}
-                  icon={<MenuCheckedIcon />}
-                  onFocus={() => {
-                    setValue(
-                      { type: "keyword", value: name },
-                      { isEphemeral: true }
-                    );
-                    setDescriptionValue(name);
-                  }}
-                  onBlur={() => {
-                    setValue(
-                      { type: "keyword", value: currentValue },
-                      { isEphemeral: true }
-                    );
-                    setDescriptionValue(undefined);
-                  }}
-                >
-                  <Flex gap="1">
-                    <Flex
-                      css={{
-                        width: theme.spacing[9],
-                        height: theme.spacing[9],
-                      }}
-                      align="center"
-                      justify="center"
-                    >
-                      <Icon />
-                    </Flex>
-                    {label}
+      <DropdownMenuContent sideOffset={4} collisionPadding={16} side="bottom">
+        <DropdownMenuRadioGroup
+          value={currentValue}
+          onValueChange={(value) => setValue({ type: "keyword", value })}
+        >
+          {items.map(({ name, label, icon: Icon }) => {
+            return (
+              <DropdownMenuRadioItem
+                text="sentence"
+                key={name}
+                value={name}
+                icon={<MenuCheckedIcon />}
+                onFocus={() => {
+                  setValue(
+                    { type: "keyword", value: name },
+                    { isEphemeral: true }
+                  );
+                  setDescriptionValue(name);
+                }}
+                onBlur={() => {
+                  setValue(
+                    { type: "keyword", value: currentValue },
+                    { isEphemeral: true }
+                  );
+                  setDescriptionValue(undefined);
+                }}
+              >
+                <Flex gap="1">
+                  <Flex
+                    css={{
+                      width: theme.spacing[9],
+                      height: theme.spacing[9],
+                    }}
+                    align="center"
+                    justify="center"
+                  >
+                    <Icon />
                   </Flex>
-                </DropdownMenuRadioItem>
-              );
-            })}
-          </DropdownMenuRadioGroup>
-          <DropdownMenuSeparator />
-          <DropdownMenuItem hint>
-            <Box css={{ width: theme.spacing[25] }}>{description}</Box>
-          </DropdownMenuItem>
-        </DropdownMenuContent>
-      </DropdownMenuPortal>
+                  {label}
+                </Flex>
+              </DropdownMenuRadioItem>
+            );
+          })}
+        </DropdownMenuRadioGroup>
+        <DropdownMenuSeparator />
+        <DropdownMenuItem hint>
+          <Box css={{ width: theme.spacing[25] }}>{description}</Box>
+        </DropdownMenuItem>
+      </DropdownMenuContent>
     </DropdownMenu>
   );
 };
diff --git a/apps/builder/app/builder/features/style-panel/sections/transforms/transforms.tsx b/apps/builder/app/builder/features/style-panel/sections/transforms/transforms.tsx
@@ -12,7 +12,6 @@ import {
   DropdownMenu,
   DropdownMenuContent,
   DropdownMenuItem,
-  DropdownMenuPortal,
   DropdownMenuTrigger,
   Flex,
   Grid,
@@ -170,31 +169,29 @@ export const Section = () => {
                     prefix={<PlusIcon />}
                   ></SectionTitleButton>
                 </DropdownMenuTrigger>
-                <DropdownMenuPortal>
-                  <DropdownMenuContent
-                    collisionPadding={16}
-                    css={{ width: theme.spacing[24] }}
-                  >
-                    {transformPanels.map((panel) => (
-                      <DropdownMenuItem
-                        disabled={isTransformPanelPropertyUsed({
+                <DropdownMenuContent
+                  collisionPadding={16}
+                  css={{ width: theme.spacing[24] }}
+                >
+                  {transformPanels.map((panel) => (
+                    <DropdownMenuItem
+                      disabled={isTransformPanelPropertyUsed({
+                        panel,
+                        styles,
+                      })}
+                      key={panel}
+                      onSelect={() => {
+                        addDefaultsForTransormSection({
                           panel,
                           styles,
-                        })}
-                        key={panel}
-                        onSelect={() => {
-                          addDefaultsForTransormSection({
-                            panel,
-                            styles,
-                          });
-                          setIsOpen(true);
-                        }}
-                      >
-                        {humanizeString(panel)}
-                      </DropdownMenuItem>
-                    ))}
-                  </DropdownMenuContent>
-                </DropdownMenuPortal>
+                        });
+                        setIsOpen(true);
+                      }}
+                    >
+                      {humanizeString(panel)}
+                    </DropdownMenuItem>
+                  ))}
+                </DropdownMenuContent>
               </DropdownMenu>
             </Flex>
           }

diff --git a/apps/builder/app/builder/features/style-panel/style-source/style-source-control.tsx b/apps/builder/app/builder/features/style-panel/style-source/style-source-control.tsx
@@ -1,7 +1,6 @@
 import {
   DropdownMenu,
   DropdownMenuContent,
-  DropdownMenuPortal,
   DropdownMenuTrigger,
   Text,
   styled,
@@ -97,14 +96,12 @@ const Menu = (props: MenuProps) => {
           <ChevronDownIcon style={{ position: "relative" }} />
         </MenuTrigger>
       </DropdownMenuTrigger>
-      <DropdownMenuPortal>
-        <DropdownMenuContent
-          onCloseAutoFocus={(event) => event.preventDefault()}
-          css={{ maxWidth: theme.spacing[24] }}
-        >
-          {props.children}
-        </DropdownMenuContent>
-      </DropdownMenuPortal>
+      <DropdownMenuContent
+        onCloseAutoFocus={(event) => event.preventDefault()}
+        css={{ maxWidth: theme.spacing[24] }}
+      >
+        {props.children}
+      </DropdownMenuContent>
     </DropdownMenu>
   );
 };

diff --git a/apps/builder/app/builder/features/topbar/builder-mode.tsx b/apps/builder/app/builder/features/topbar/builder-mode.tsx
@@ -20,11 +20,8 @@ import {
   ToolbarToggleItem,
   Tooltip,
   Text,
-} from "@webstudio-is/design-system";
-import {
   DropdownMenu,
   DropdownMenuContent,
-  DropdownMenuPortal,
   DropdownMenuTrigger,
 } from "@webstudio-is/design-system";
 import {
@@ -107,53 +104,51 @@ export const BuilderModeDropDown = () => {
             </ToolbarToggleItem>
           </DropdownMenuTrigger>
         </Tooltip>
-        <DropdownMenuPortal>
-          <DropdownMenuContent
-            sideOffset={4}
-            collisionPadding={16}
-            side="bottom"
-            loop
+        <DropdownMenuContent
+          sideOffset={4}
+          collisionPadding={16}
+          side="bottom"
+          loop
+        >
+          <DropdownMenuRadioGroup
+            value={builderMode}
+            onValueChange={(value) => {
+              if (isBuilderMode(value)) {
+                setBuilderMode(value);
+              }
+            }}
           >
-            <DropdownMenuRadioGroup
-              value={builderMode}
-              onValueChange={(value) => {
-                if (isBuilderMode(value)) {
-                  setBuilderMode(value);
-                }
-              }}
-            >
-              {Object.entries(menuItems)
-                .filter(([_, { enabled }]) => enabled)
-                .map(([mode, { icon, title, shortcut }]) => (
-                  <DropdownMenuRadioItem
-                    key={mode}
-                    value={mode}
-                    onFocus={handleFocus(mode as keyof typeof menuItems)}
-                    onBlur={handleBlur}
-                    icon={<MenuCheckedIcon />}
-                  >
-                    <Flex css={{ px: theme.spacing[3] }} gap={2}>
-                      {icon}
-                      <Box>{title}</Box>
-                    </Flex>
-                    <DropdownMenuItemRightSlot>
-                      <Kbd value={shortcut} />
-                    </DropdownMenuItemRightSlot>
-                    &nbsp;
-                  </DropdownMenuRadioItem>
-                ))}
-            </DropdownMenuRadioGroup>
-            <DropdownMenuSeparator />
+            {Object.entries(menuItems)
+              .filter(([_, { enabled }]) => enabled)
+              .map(([mode, { icon, title, shortcut }]) => (
+                <DropdownMenuRadioItem
+                  key={mode}
+                  value={mode}
+                  onFocus={handleFocus(mode as keyof typeof menuItems)}
+                  onBlur={handleBlur}
+                  icon={<MenuCheckedIcon />}
+                >
+                  <Flex css={{ px: theme.spacing[3] }} gap={2}>
+                    {icon}
+                    <Box>{title}</Box>
+                  </Flex>
+                  <DropdownMenuItemRightSlot>
+                    <Kbd value={shortcut} />
+                  </DropdownMenuItemRightSlot>
+                  &nbsp;
+                </DropdownMenuRadioItem>
+              ))}
+          </DropdownMenuRadioGroup>
+          <DropdownMenuSeparator />
 
-            <div className={menuItemCss({ hint: true })}>
-              <Box css={{ width: theme.spacing[25] }}>
-                {activeMode
-                  ? menuItems[activeMode].description
-                  : "Select Design or Content mode"}
-              </Box>
-            </div>
-          </DropdownMenuContent>
-        </DropdownMenuPortal>
+          <div className={menuItemCss({ hint: true })}>
+            <Box css={{ width: theme.spacing[25] }}>
+              {activeMode
+                ? menuItems[activeMode].description
+                : "Select Design or Content mode"}
+            </Box>
+          </div>
+        </DropdownMenuContent>
       </DropdownMenu>
     </Flex>
   );