Fix missing home bookmark while siwtching using breadcrumbs

rilldata · Dec 13, 2024 · 61fc74a · 61fc74a
1 parent c9b0fe9
commit 61fc74a
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 52 deletions.
diff --git a/web-admin/src/routes/[organization]/[project]/-/share/[token]/+page.svelte b/web-admin/src/routes/[organization]/[project]/-/share/[token]/+page.svelte
@@ -61,6 +61,7 @@
       <DashboardURLStateSync
         metricsViewName={explore.metricsView.meta.name.name}
         exploreName={resourceName}
+        extraKeyPrefix={`${organization}__${project}__`}
         {defaultExplorePreset}
         {initExploreState}
         {exploreStateFromYAMLConfig}

diff --git a/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.svelte b/web-admin/src/routes/[organization]/[project]/explore/[dashboard]/+page.svelte
@@ -1,5 +1,5 @@
 <script lang="ts">
-  import { afterNavigate, onNavigate } from "$app/navigation";
+  import { onNavigate } from "$app/navigation";
   import { page } from "$app/stores";
   import DashboardBuilding from "@rilldata/web-admin/features/dashboards/DashboardBuilding.svelte";
   import DashboardErrored from "@rilldata/web-admin/features/dashboards/DashboardErrored.svelte";
@@ -12,7 +12,6 @@
   import { useExplore } from "@rilldata/web-common/features/explores/selectors";
   import { eventBus } from "@rilldata/web-common/lib/event-bus/event-bus";
   import { runtime } from "@rilldata/web-common/runtime-client/runtime-store";
-  import type { AfterNavigate } from "@sveltejs/kit";
   import type { PageData } from "./$types";
 
   const PollIntervalWhenDashboardFirstReconciling = 1000;
@@ -83,12 +82,6 @@
     });
   }
 
-  let previousNavigationType: AfterNavigate["type"];
-  afterNavigate(({ from, type }) => {
-    if (from !== null && !from.url) return;
-    previousNavigationType = type;
-  });
-
   onNavigate(() => {
     // Temporary: clear the mocked user when navigating away.
     // In the future, we should be able to handle the mocked user on all project pages.
@@ -112,12 +105,12 @@
         <DashboardURLStateSync
           {metricsViewName}
           {exploreName}
+          extraKeyPrefix={`${orgName}__${projectName}__`}
           {defaultExplorePreset}
           {initExploreState}
           {exploreStateFromYAMLConfig}
           {partialExploreStateFromUrl}
           {exploreStateFromSessionStorage}
-          {previousNavigationType}
         >
           <DashboardThemeProvider>
             <Dashboard {metricsViewName} {exploreName} />

diff --git a/web-common/src/features/dashboards/state-managers/StateManagersProvider.svelte b/web-common/src/features/dashboards/state-managers/StateManagersProvider.svelte
@@ -1,6 +1,6 @@
 <script lang="ts">
   import { page } from "$app/stores";
-  import { onMount, setContext } from "svelte";
+  import { setContext } from "svelte";
   import { useQueryClient } from "@tanstack/svelte-query";
   import { createStateManagers, DEFAULT_STORE_KEY } from "./state-managers";
 
@@ -21,8 +21,6 @@
   });
   setContext(DEFAULT_STORE_KEY, stateManagers);
 
-  onMount(() => () => stateManagers.cleanup());
-
   // Our state management was not built around the ability to arbitrarily change the explore or metrics view name
   // This needs to change, but this is a workaround for now
   $: if (visualEditing) {

diff --git a/web-common/src/features/dashboards/state-managers/state-managers.ts b/web-common/src/features/dashboards/state-managers/state-managers.ts
@@ -4,7 +4,6 @@ import {
   contextColWidthDefaults,
 } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity";
 import { createPersistentDashboardStore } from "@rilldata/web-common/features/dashboards/stores/persistent-dashboard-state";
-import { updateExploreSessionStore } from "@rilldata/web-common/features/dashboards/url-state/explore-web-view-store";
 import { getDefaultExplorePreset } from "@rilldata/web-common/features/dashboards/url-state/getDefaultExplorePreset";
 import { initLocalUserPreferenceStore } from "@rilldata/web-common/features/dashboards/user-preferences";
 import {
@@ -71,8 +70,6 @@ export type StateManagers = {
    */
   contextColumnWidths: Writable<ContextColWidths>;
   defaultExploreState: Readable<V1ExplorePreset>;
-
-  cleanup: () => void;
 };
 
 export const DEFAULT_STORE_KEY = Symbol("state-managers");
@@ -155,17 +152,6 @@ export function createStateManagers({
       );
     },
   );
-  const unsub = dashboardStore.subscribe((dashState) => {
-    const exploreState = get(validSpecStore).data?.explore;
-    if (!dashState || !exploreState || dashState.name !== get(exploreNameStore))
-      return;
-    updateExploreSessionStore(
-      get(exploreNameStore),
-      extraKeyPrefix,
-      dashState,
-      exploreState,
-    );
-  });
 
   const persistentDashboardStore = createPersistentDashboardStore(
     (extraKeyPrefix || "") + exploreName,
@@ -201,9 +187,5 @@ export function createStateManagers({
     }),
     contextColumnWidths,
     defaultExploreState,
-
-    cleanup: () => {
-      unsub();
-    },
   };
 }
diff --git a/web-common/src/features/dashboards/url-state/DashboardURLStateSync.svelte b/web-common/src/features/dashboards/url-state/DashboardURLStateSync.svelte
@@ -1,23 +1,24 @@
 <script lang="ts">
-  import { afterNavigate, goto } from "$app/navigation";
+  import { afterNavigate, goto, replaceState } from "$app/navigation";
   import { page } from "$app/stores";
   import ErrorPage from "@rilldata/web-common/components/ErrorPage.svelte";
   import { getStateManagers } from "@rilldata/web-common/features/dashboards/state-managers/state-managers";
   import { metricsExplorerStore } from "@rilldata/web-common/features/dashboards/stores/dashboard-stores";
   import type { MetricsExplorerEntity } from "@rilldata/web-common/features/dashboards/stores/metrics-explorer-entity";
   import { convertExploreStateToURLSearchParams } from "@rilldata/web-common/features/dashboards/url-state/convertExploreStateToURLSearchParams";
+  import { updateExploreSessionStore } from "@rilldata/web-common/features/dashboards/url-state/explore-web-view-store";
   import { getUpdatedUrlForExploreState } from "@rilldata/web-common/features/dashboards/url-state/getUpdatedUrlForExploreState";
   import {
     createQueryServiceMetricsViewSchema,
     type V1ExplorePreset,
   } from "@rilldata/web-common/runtime-client";
   import type { HTTPError } from "@rilldata/web-common/runtime-client/fetchWrapper";
   import { runtime } from "@rilldata/web-common/runtime-client/runtime-store";
-  import type { AfterNavigate } from "@sveltejs/kit";
   import { onMount } from "svelte";
 
   export let metricsViewName: string;
   export let exploreName: string;
+  export let extraKeyPrefix: string | undefined = undefined;
   export let defaultExplorePreset: V1ExplorePreset;
   export let exploreStateFromYAMLConfig: Partial<MetricsExplorerEntity>;
   export let initExploreState: Partial<MetricsExplorerEntity> | undefined =
@@ -26,7 +27,6 @@
   export let exploreStateFromSessionStorage:
     | Partial<MetricsExplorerEntity>
     | undefined;
-  export let previousNavigationType: AfterNavigate["type"];
 
   const { dashboardStore, validSpecStore, timeRangeSummaryStore } =
     getStateManagers();
@@ -46,9 +46,9 @@
       // null checks
       !metricsSpec ||
       !exploreSpec ||
+      !to ||
       // seems like a sveltekit bug where an additional afterNavigate is triggered with invalid fields
-      (from !== null && !from.url) ||
-      !to
+      (from !== null && !from.url)
     ) {
       return;
     }
@@ -61,7 +61,7 @@
 
     let partialExplore = partialExploreStateFromUrl;
     let shouldUpdateUrl = false;
-    if (exploreStateFromSessionStorage && type !== "popstate") {
+    if (exploreStateFromSessionStorage) {
       partialExplore = exploreStateFromSessionStorage;
       shouldUpdateUrl = true;
     }
@@ -80,14 +80,11 @@
     );
     prevUrl = redirectUrl.toString();
 
-    if (!shouldUpdateUrl || redirectUrl.search === $page.url.search) {
+    if (!shouldUpdateUrl || redirectUrl.search === to.url.toString()) {
       return;
     }
 
-    void goto(redirectUrl, {
-      replaceState: true,
-      state: $page.state,
-    });
+    replaceState(redirectUrl, $page.state);
   });
 
   let prevUrl = "";
@@ -130,10 +127,7 @@
       return;
     }
 
-    void goto(redirectUrl, {
-      replaceState: true,
-      state: $page.state,
-    });
+    replaceState(redirectUrl, $page.state);
   }
 
   function gotoNewState() {
@@ -148,9 +142,15 @@
       defaultExplorePreset,
     );
     const newUrl = u.toString();
-    if (prevUrl !== newUrl) {
-      void goto(newUrl);
-    }
+    if (prevUrl === newUrl) return;
+
+    void goto(newUrl);
+    updateExploreSessionStore(
+      exploreName,
+      extraKeyPrefix,
+      $dashboardStore,
+      exploreSpec,
+    );
   }
 
   // reactive to only dashboardStore
@@ -160,10 +160,11 @@
   }
 
   onMount(() => {
-    // safeguard to make sure we initialize the explore state in case afterNavigate is missed
-    if (!$dashboardStore) {
-      handleExploreInit(previousNavigationType === "enter");
-    }
+    setTimeout(() => {
+      if (!$dashboardStore) {
+        handleExploreInit(true);
+      }
+    });
   });
 </script>