feat: select the page when opening the page settings (#4375)

Closes #4372

Editing another page settings is confusing to users and complicates the
logic. Win-win

Author: TrySound

apps/builder/app/builder/features/pages/page-utils.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, test } from "@jest/globals";
+import { setEnv } from "@webstudio-is/feature-flags";
 import { createDefaultPages } from "@webstudio-is/project-build";
 import {
   isRootFolder,
@@ -20,13 +21,13 @@ import {
 import {
   $dataSourceVariables,
   $dataSources,
-  $editingPageId,
   $pages,
   $resourceValues,
   $selectedPageId,
 } from "~/shared/nano-states";
 import { registerContainers } from "~/shared/sync";
 
+setEnv("*");
 registerContainers();
 
 const createPages = () => {
@@ -396,7 +397,7 @@ describe("deleteFolderWithChildrenMutable", () => {
   });
 });
 
-test("page root scope should rely on editing page", () => {
+test("page root scope should rely on selected page", () => {
   const pages = createDefaultPages({
     rootInstanceId: "homeRootId",
     homePageId: "homePageId",
@@ -412,8 +413,7 @@ test("page root scope should rely on editing page", () => {
     systemDataSourceId: "system",
   });
   $pages.set(pages);
-  $selectedPageId.set("homePageId");
-  $editingPageId.set("pageId");
+  $selectedPageId.set("pageId");
   $dataSources.set(
     toMap([
       {
@@ -447,7 +447,7 @@ test("page root scope should use variable and resource values", () => {
       systemDataSourceId: "system",
     })
   );
-  $editingPageId.set("homePageId");
+  $selectedPageId.set("homePageId");
   $dataSources.set(
     toMap([
       {
@@ -494,7 +494,7 @@ test("page root scope should prefill default system variable value", () => {
       systemDataSourceId: "systemId",
     })
   );
-  $editingPageId.set("homePageId");
+  $selectedPageId.set("homePageId");
   $dataSources.set(
     toMap([
       {

apps/builder/app/builder/features/pages/page-utils.ts

@@ -4,7 +4,6 @@ import {
   type Page,
   type Folder,
   type WebstudioData,
-  type System,
   Pages,
   findPageByIdOrPath,
   getPagePath,
@@ -19,16 +18,12 @@ import {
   updateWebstudioData,
 } from "~/shared/instance-utils";
 import {
-  $dataSourceVariables,
   $dataSources,
-  $editingPageId,
   $pages,
-  $publishedOrigin,
-  $resourceValues,
   $selectedInstanceSelector,
+  $selectedPage,
   $selectedPageId,
-  getPageDefaultSystem,
-  mergeSystem,
+  $variableValuesByInstanceSelector,
 } from "~/shared/nano-states";
 import { insertPageCopyMutable } from "~/shared/page-utils";
 
@@ -244,75 +239,20 @@ export const deleteFolderWithChildrenMutable = (
   };
 };
 
-const $editingPage = computed(
-  [$editingPageId, $pages],
-  (editingPageId, pages) => {
-    if (editingPageId === undefined || pages === undefined) {
-      return;
-    }
-    return findPageByIdOrPath(editingPageId, pages);
-  }
-);
-
-const $editingPagePath = computed($editingPage, (page) => page?.path);
-
-const $editingPageHistory = computed($editingPage, (page) => page?.history);
-
-const $editingPageDefaultSystem = computed(
-  [$publishedOrigin, $editingPagePath, $editingPageHistory],
-  (origin, path, history) => getPageDefaultSystem({ origin, path, history })
-);
-
-const $pageRootVariableValues = computed(
-  [
-    $editingPage,
-    $dataSources,
-    $dataSourceVariables,
-    $resourceValues,
-    $editingPageDefaultSystem,
-  ],
-  (page, dataSources, dataSourceVariables, resourceValues, defaultSystem) => {
-    const variableValues = new Map<string, unknown>();
-    if (page === undefined) {
-      return variableValues;
-    }
-    for (const variable of dataSources.values()) {
-      if (variable.scopeInstanceId !== page.rootInstanceId) {
-        continue;
-      }
-      if (variable.type === "variable") {
-        const value = dataSourceVariables.get(variable.id);
-        variableValues.set(variable.id, value ?? variable.value.value);
-      }
-      if (variable.type === "parameter") {
-        const value = dataSourceVariables.get(variable.id);
-        variableValues.set(variable.id, value);
-        if (variable.id === page.systemDataSourceId) {
-          variableValues.set(
-            variable.id,
-            mergeSystem(defaultSystem, value as undefined | System)
-          );
-        }
-      }
-      if (variable.type === "resource") {
-        const value = resourceValues.get(variable.resourceId);
-        variableValues.set(variable.id, value);
-      }
-    }
-    return variableValues;
-  }
-);
-
 export const $pageRootScope = computed(
-  [$editingPage, $pageRootVariableValues, $dataSources],
-  (editingPage, pageRootVariableValues, dataSources) => {
+  [$selectedPage, $variableValuesByInstanceSelector, $dataSources],
+  (page, variableValuesByInstanceSelector, dataSources) => {
     const scope: Record<string, unknown> = {};
     const aliases = new Map<string, string>();
     const defaultValues = new Map<string, unknown>();
-    if (editingPage === undefined) {
+    if (page === undefined) {
       return { variableValues: defaultValues, scope, aliases };
     }
-    for (const [dataSourceId, value] of pageRootVariableValues) {
+    const values =
+      variableValuesByInstanceSelector.get(
+        JSON.stringify([page.rootInstanceId])
+      ) ?? new Map<string, unknown>();
+    for (const [dataSourceId, value] of values) {
       const dataSource = dataSources.get(dataSourceId);
       if (dataSource === undefined) {
         continue;
@@ -321,7 +261,7 @@ export const $pageRootScope = computed(
       scope[name] = value;
       aliases.set(name, dataSource.name);
     }
-    return { variableValues: pageRootVariableValues, scope, aliases };
+    return { variableValues: values, scope, aliases };
   }
 );
 

apps/builder/app/builder/features/pages/pages.tsx

@@ -434,19 +434,19 @@ const newPageId = "new-page";
 
 const PageEditor = ({
   editingPageId,
-  setEditingPageId,
+  onClose,
 }: {
   editingPageId: string;
-  setEditingPageId: (pageId?: string) => void;
+  onClose: () => void;
 }) => {
   const currentPageId = useStore($selectedPageId);
 
   if (editingPageId === newPageId) {
     return (
       <NewPageSettings
-        onClose={() => setEditingPageId(undefined)}
+        onClose={onClose}
         onSuccess={(pageId) => {
-          setEditingPageId(undefined);
+          onClose();
           switchPage(pageId);
         }}
       />
@@ -455,9 +455,9 @@ const PageEditor = ({
 
   return (
     <PageSettings
-      onClose={() => setEditingPageId(undefined)}
+      onClose={onClose}
       onDelete={() => {
-        setEditingPageId(undefined);
+        onClose();
         // switch to home page when deleted currently selected page
         if (editingPageId === currentPageId) {
           const pages = $pages.get();
@@ -467,7 +467,7 @@ const PageEditor = ({
         }
       }}
       onDuplicate={(newPageId) => {
-        setEditingPageId(undefined);
+        onClose();
         switchPage(newPageId);
       }}
       pageId={editingPageId}
@@ -478,29 +478,25 @@ const PageEditor = ({
 
 const FolderEditor = ({
   editingFolderId,
-  setEditingFolderId,
+  onClose,
 }: {
   editingFolderId: string;
-  setEditingFolderId: (pageId?: string) => void;
+  onClose: () => void;
 }) => {
   if (editingFolderId === newFolderId) {
     return (
       <NewFolderSettings
-        onClose={() => setEditingFolderId(undefined)}
-        onSuccess={() => {
-          setEditingFolderId(undefined);
-        }}
         key={newFolderId}
+        onClose={onClose}
+        onSuccess={onClose}
       />
     );
   }
 
   return (
     <FolderSettings
-      onClose={() => setEditingFolderId(undefined)}
-      onDelete={() => {
-        setEditingFolderId(undefined);
-      }}
+      onClose={onClose}
+      onDelete={onClose}
       folderId={editingFolderId}
       key={editingFolderId}
     />
@@ -567,7 +563,13 @@ export const PagesPanel = ({ onClose }: { onClose: () => void }) => {
           onClose();
         }}
         editingItemId={editingItemId}
-        onEdit={$editingPageId.set}
+        onEdit={(itemId) => {
+          // always select page when edit its settings
+          if (itemId && isFolder(itemId, pages.folders) === false) {
+            $selectedPageId.set(itemId);
+          }
+          $editingPageId.set(itemId);
+        }}
       />
 
       <ExtendedPanel isOpen={editingItemId !== undefined}>
@@ -576,12 +578,12 @@ export const PagesPanel = ({ onClose }: { onClose: () => void }) => {
             {isFolder(editingItemId, pages.folders) ? (
               <FolderEditor
                 editingFolderId={editingItemId}
-                setEditingFolderId={$editingPageId.set}
+                onClose={() => $editingPageId.set(undefined)}
               />
             ) : (
               <PageEditor
                 editingPageId={editingItemId}
-                setEditingPageId={$editingPageId.set}
+                onClose={() => $editingPageId.set(undefined)}
               />
             )}
           </>

apps/builder/app/shared/nano-states/props.ts

@@ -497,9 +497,6 @@ export const $variableValuesByInstanceSelector = computed(
     ) => {
       const [instanceId] = instanceSelector;
       const instance = instances.get(instanceId);
-      if (instance === undefined) {
-        return;
-      }
 
       let variableValues = new Map<string, unknown>(parentVariableValues);
       variableValuesByInstanceSelector.set(
@@ -563,6 +560,10 @@ export const $variableValuesByInstanceSelector = computed(
         }
       }
 
+      if (instance === undefined) {
+        return;
+      }
+
       if (instance.component === collectionComponent) {
         const data = propValues.get("data");
         const itemVariableId = parameters.get("item");

apps/builder/app/shared/nano-states/variables.ts

@@ -13,45 +13,35 @@ export const $dataSourceVariables = atom<Map<DataSource["id"], unknown>>(
 
 export const $resourceValues = atom(new Map<Resource["id"], unknown>());
 
-export const getPageDefaultSystem = ({
-  origin,
-  path,
-  history,
-}: {
-  origin: string;
-  path?: string;
-  history?: string[];
-}) => {
-  const defaultSystem: System = {
-    params: {},
-    search: {},
-    origin,
-  };
-  if (path) {
-    const tokens = tokenizePathnamePattern(path);
-    // try to match the first item in history to let user
-    // see the page without manually entering params
-    // or selecting them in address bar
-    const matchedParams = history
-      ? matchPathnamePattern(path, history[0])
-      : undefined;
-    for (const token of tokens) {
-      if (token.type === "param") {
-        defaultSystem.params[token.name] =
-          matchedParams?.[token.name] ?? undefined;
-      }
-    }
-  }
-  return defaultSystem;
-};
-
 const $selectedPagePath = computed($selectedPage, (page) => page?.path);
 
 const $selectedPageHistory = computed($selectedPage, (page) => page?.history);
 
 export const $selectedPageDefaultSystem = computed(
   [$publishedOrigin, $selectedPagePath, $selectedPageHistory],
-  (origin, path, history) => getPageDefaultSystem({ origin, path, history })
+  (origin, path, history) => {
+    const defaultSystem: System = {
+      params: {},
+      search: {},
+      origin,
+    };
+    if (path) {
+      const tokens = tokenizePathnamePattern(path);
+      // try to match the first item in history to let user
+      // see the page without manually entering params
+      // or selecting them in address bar
+      const matchedParams = history
+        ? matchPathnamePattern(path, history[0])
+        : undefined;
+      for (const token of tokens) {
+        if (token.type === "param") {
+          defaultSystem.params[token.name] =
+            matchedParams?.[token.name] ?? undefined;
+        }
+      }
+    }
+    return defaultSystem;
+  }
 );
 
 export const mergeSystem = (left: System, right?: System): System => {