From 68673a2d1741d6abaf328f79299c91b6d30a46d5 Mon Sep 17 00:00:00 2001
From: Dhiraj Kumar <sumankumaribarnwal@gmail.com>
Date: Sat, 4 Jan 2025 00:28:05 +0530
Subject: [PATCH] Review

---
 .../canvas/components/charts/types.ts         |   2 +-
 .../features/canvas/components/charts/util.ts |   6 +-
 .../canvas/inspector/ChartTypeSelector.svelte |   4 +-
 .../StateManagersProvider.svelte              |  12 +-
 .../canvas/stores/CanvasStateProvider.svelte  |  15 ---
 .../features/canvas/stores/syncCanvasState.ts |   5 +-
 .../workspaces/CanvasWorkspace.svelte         | 105 +++++++++---------
 .../routes/(viz)/canvas/[name]/+page.svelte   |  17 ++-
 8 files changed, 76 insertions(+), 90 deletions(-)
 delete mode 100644 web-common/src/features/canvas/stores/CanvasStateProvider.svelte

diff --git a/web-common/src/features/canvas/components/charts/types.ts b/web-common/src/features/canvas/components/charts/types.ts
index 84e54474cbe..4ab51c96bb2 100644
--- a/web-common/src/features/canvas/components/charts/types.ts
+++ b/web-common/src/features/canvas/components/charts/types.ts
@@ -21,7 +21,7 @@ export interface ChartConfig {
 export type ChartType = "line_chart" | "bar_chart" | "stacked_bar";
 
 export interface ChartMetadata {
-  id: ChartType;
+  type: ChartType;
   icon: ComponentType<SvelteComponent>;
   title: string;
 }
diff --git a/web-common/src/features/canvas/components/charts/util.ts b/web-common/src/features/canvas/components/charts/util.ts
index 1e8f5e69595..b4f9e0b06d8 100644
--- a/web-common/src/features/canvas/components/charts/util.ts
+++ b/web-common/src/features/canvas/components/charts/util.ts
@@ -25,7 +25,7 @@ export function generateSpec(
 }
 
 export const chartMetadata: ChartMetadata[] = [
-  { id: "line_chart", title: "Line", icon: LineChart },
-  { id: "bar_chart", title: "Bar", icon: BarChart },
-  { id: "stacked_bar", title: "Stacked Bar", icon: StackedBar },
+  { type: "line_chart", title: "Line", icon: LineChart },
+  { type: "bar_chart", title: "Bar", icon: BarChart },
+  { type: "stacked_bar", title: "Stacked Bar", icon: StackedBar },
 ];
diff --git a/web-common/src/features/canvas/inspector/ChartTypeSelector.svelte b/web-common/src/features/canvas/inspector/ChartTypeSelector.svelte
index 9cf4b78ce4b..81ad27ece35 100644
--- a/web-common/src/features/canvas/inspector/ChartTypeSelector.svelte
+++ b/web-common/src/features/canvas/inspector/ChartTypeSelector.svelte
@@ -13,7 +13,7 @@
   export let component: CanvasComponentObj;
 
   async function selectChartType(chartType: ChartMetadata) {
-    component.updateChartType(chartType.id);
+    component.updateChartType(chartType.type);
   }
 </script>
 
@@ -26,7 +26,7 @@
           square
           small
           type="secondary"
-          selected={componentType === chart.id}
+          selected={componentType === chart.type}
           on:click={() => selectChartType(chart)}
         >
           <svelte:component this={chart.icon} size="20px" />
diff --git a/web-common/src/features/canvas/state-managers/StateManagersProvider.svelte b/web-common/src/features/canvas/state-managers/StateManagersProvider.svelte
index fa4f11ce50f..df368c764c2 100644
--- a/web-common/src/features/canvas/state-managers/StateManagersProvider.svelte
+++ b/web-common/src/features/canvas/state-managers/StateManagersProvider.svelte
@@ -1,4 +1,6 @@
 <script lang="ts">
+  import { createCanvasStateSync } from "@rilldata/web-common/features/canvas/stores/syncCanvasState";
+  import DelayedSpinner from "@rilldata/web-common/features/entity-management/DelayedSpinner.svelte";
   import { useQueryClient } from "@tanstack/svelte-query";
   import { setContext } from "svelte";
   import { createStateManagers, DEFAULT_STORE_KEY } from "./state-managers";
@@ -11,7 +13,15 @@
     canvasName,
   });
 
+  const canvasStoreReady = createCanvasStateSync(canvasName);
+
   setContext(DEFAULT_STORE_KEY, stateManagers);
 </script>
 
-<slot />
+{#if canvasStoreReady.isFetching}
+  <div class="grid place-items-center size-full">
+    <DelayedSpinner isLoading={canvasStoreReady.isFetching} size="40px" />
+  </div>
+{:else}
+  <slot />
+{/if}
diff --git a/web-common/src/features/canvas/stores/CanvasStateProvider.svelte b/web-common/src/features/canvas/stores/CanvasStateProvider.svelte
deleted file mode 100644
index 534aefe2dfb..00000000000
--- a/web-common/src/features/canvas/stores/CanvasStateProvider.svelte
+++ /dev/null
@@ -1,15 +0,0 @@
-<script lang="ts">
-  import { createCanvasStateSync } from "@rilldata/web-common/features/canvas/stores/syncCanvasState";
-  import DelayedSpinner from "@rilldata/web-common/features/entity-management/DelayedSpinner.svelte";
-  import { getCanvasStateManagers } from "../state-managers/state-managers";
-
-  const canvasStoreReady = createCanvasStateSync(getCanvasStateManagers());
-</script>
-
-{#if canvasStoreReady.isFetching}
-  <div class="grid place-items-center size-full">
-    <DelayedSpinner isLoading={canvasStoreReady.isFetching} size="40px" />
-  </div>
-{:else}
-  <slot />
-{/if}
diff --git a/web-common/src/features/canvas/stores/syncCanvasState.ts b/web-common/src/features/canvas/stores/syncCanvasState.ts
index ed55ba08ce0..28e47e17b5f 100644
--- a/web-common/src/features/canvas/stores/syncCanvasState.ts
+++ b/web-common/src/features/canvas/stores/syncCanvasState.ts
@@ -1,9 +1,6 @@
-import type { StateManagers } from "@rilldata/web-common/features/canvas/state-managers/state-managers";
 import { canvasEntities } from "@rilldata/web-common/features/canvas/stores/canvas-entities";
-import { get } from "svelte/store";
 
-export function createCanvasStateSync(ctx: StateManagers) {
-  const canvasName = get(ctx.canvasName);
+export function createCanvasStateSync(canvasName: string) {
   if (canvasEntities.hasCanvas(canvasName)) {
     // TODO: Add sync method if required
     return { isFetching: false, error: false };
diff --git a/web-common/src/features/workspaces/CanvasWorkspace.svelte b/web-common/src/features/workspaces/CanvasWorkspace.svelte
index 5ab7782aa48..ccb80a45e28 100644
--- a/web-common/src/features/workspaces/CanvasWorkspace.svelte
+++ b/web-common/src/features/workspaces/CanvasWorkspace.svelte
@@ -10,7 +10,6 @@
   import VisualCanvasEditing from "@rilldata/web-common/features/canvas/inspector/VisualCanvasEditing.svelte";
   import { useDefaultMetrics } from "@rilldata/web-common/features/canvas/selector";
   import StateManagersProvider from "@rilldata/web-common/features/canvas/state-managers/StateManagersProvider.svelte";
-  import CanvasStateProvider from "@rilldata/web-common/features/canvas/stores/CanvasStateProvider.svelte";
   import DelayedSpinner from "@rilldata/web-common/features/entity-management/DelayedSpinner.svelte";
   import { getNameFromFile } from "@rilldata/web-common/features/entity-management/entity-mappers";
   import type { FileArtifact } from "@rilldata/web-common/features/entity-management/file-artifact";
@@ -127,62 +126,60 @@
 {#if canvasResource && fileArtifact}
   {#key canvasName}
     <StateManagersProvider {canvasName}>
-      <CanvasStateProvider>
-        <CanvasThemeProvider>
-          <WorkspaceContainer>
-            <WorkspaceHeader
-              slot="header"
-              {filePath}
-              hasUnsavedChanges={$hasUnsavedChanges}
-              titleInput={fileName}
-              onTitleChange={onChangeCallback}
-              resourceKind={ResourceKind.Canvas}
-            >
-              <div class="flex gap-x-2" slot="cta">
-                <PreviewButton
-                  href="/canvas/{canvasName}"
-                  disabled={allErrors.length > 0 || resourceIsReconciling}
-                  reconciling={resourceIsReconciling}
+      <CanvasThemeProvider>
+        <WorkspaceContainer>
+          <WorkspaceHeader
+            slot="header"
+            {filePath}
+            hasUnsavedChanges={$hasUnsavedChanges}
+            titleInput={fileName}
+            onTitleChange={onChangeCallback}
+            resourceKind={ResourceKind.Canvas}
+          >
+            <div class="flex gap-x-2" slot="cta">
+              <PreviewButton
+                href="/canvas/{canvasName}"
+                disabled={allErrors.length > 0 || resourceIsReconciling}
+                reconciling={resourceIsReconciling}
+              />
+
+              <AddComponentMenu {addComponent} />
+              <ViewSelector
+                allowSplit={false}
+                bind:selectedView={$selectedViewStore}
+              />
+            </div>
+          </WorkspaceHeader>
+
+          <WorkspaceEditorContainer
+            slot="body"
+            error={mainError}
+            showError={!!$remoteContent && selectedView === "code"}
+          >
+            {#if selectedView === "code"}
+              <CanvasEditor
+                bind:autoSave={$autoSave}
+                {canvasName}
+                {fileArtifact}
+                {lineBasedRuntimeErrors}
+              />
+            {:else if selectedView === "viz"}
+              {#if mainError}
+                <ErrorPage
+                  body={mainError.message}
+                  fatal
+                  header="Unable to load dashboard preview"
+                  statusCode={404}
                 />
-
-                <AddComponentMenu {addComponent} />
-                <ViewSelector
-                  allowSplit={false}
-                  bind:selectedView={$selectedViewStore}
-                />
-              </div>
-            </WorkspaceHeader>
-
-            <WorkspaceEditorContainer
-              slot="body"
-              error={mainError}
-              showError={!!$remoteContent && selectedView === "code"}
-            >
-              {#if selectedView === "code"}
-                <CanvasEditor
-                  bind:autoSave={$autoSave}
-                  {canvasName}
-                  {fileArtifact}
-                  {lineBasedRuntimeErrors}
-                />
-              {:else if selectedView === "viz"}
-                {#if mainError}
-                  <ErrorPage
-                    body={mainError.message}
-                    fatal
-                    header="Unable to load dashboard preview"
-                    statusCode={404}
-                  />
-                {:else if canvasResource}
-                  <Canvas {fileArtifact} />
-                {/if}
+              {:else if canvasResource}
+                <Canvas {fileArtifact} />
               {/if}
-            </WorkspaceEditorContainer>
+            {/if}
+          </WorkspaceEditorContainer>
 
-            <VisualCanvasEditing {fileArtifact} slot="inspector" />
-          </WorkspaceContainer>
-        </CanvasThemeProvider>
-      </CanvasStateProvider>
+          <VisualCanvasEditing {fileArtifact} slot="inspector" />
+        </WorkspaceContainer>
+      </CanvasThemeProvider>
     </StateManagersProvider>
   {/key}
 {:else if allErrors.length}
diff --git a/web-local/src/routes/(viz)/canvas/[name]/+page.svelte b/web-local/src/routes/(viz)/canvas/[name]/+page.svelte
index bd5418bb05e..c02204b58ce 100644
--- a/web-local/src/routes/(viz)/canvas/[name]/+page.svelte
+++ b/web-local/src/routes/(viz)/canvas/[name]/+page.svelte
@@ -2,7 +2,6 @@
   import CanvasDashboardEmbed from "@rilldata/web-common/features/canvas/CanvasDashboardEmbed.svelte";
   import CanvasThemeProvider from "@rilldata/web-common/features/canvas/CanvasThemeProvider.svelte";
   import StateManagersProvider from "@rilldata/web-common/features/canvas/state-managers/StateManagersProvider.svelte";
-  import CanvasStateProvider from "@rilldata/web-common/features/canvas/stores/CanvasStateProvider.svelte";
   import type { PageData } from "./$types";
 
   export let data: PageData;
@@ -13,13 +12,11 @@
 </script>
 
 <StateManagersProvider {canvasName}>
-  <CanvasStateProvider>
-    <CanvasThemeProvider>
-      <CanvasDashboardEmbed
-        items={data.dashboard.spec?.items || []}
-        {columns}
-        {gap}
-      />
-    </CanvasThemeProvider>
-  </CanvasStateProvider>
+  <CanvasThemeProvider>
+    <CanvasDashboardEmbed
+      items={data.dashboard.spec?.items || []}
+      {columns}
+      {gap}
+    />
+  </CanvasThemeProvider>
 </StateManagersProvider>