rusackas commented on code in PR #41285:
URL: https://github.com/apache/superset/pull/41285#discussion_r3454399459


##########
superset-frontend/packages/superset-core/src/sqlLab/index.ts:
##########
@@ -62,6 +62,14 @@ export interface Tab {
    */
   id: string;
 
+  /**
+   * The stable backend-assigned identifier for this tab. Exposed as an opaque
+   * string so the public extension API does not leak the backend's internal
+   * numeric tab id. Set once the tab has been persisted to the backend;
+   * undefined for new tabs before the first backend sync.
+   */
+  backendId?: string;

Review Comment:
   Good catch — wired `tabViewId` through `makeTab`/`getTab` so `backendId` 
actually gets set on the runtime `Tab` now.



##########
superset-frontend/src/SqlLab/components/SqlEditor/index.tsx:
##########
@@ -271,6 +279,50 @@ const SqlEditor: FC<Props> = ({
 
   const logAction = useLogAction({ queryEditorId: queryEditor.id });
   const isActive = currentQueryEditorId === queryEditor.id;
+
+  // Re-renders when an extension registers a northPane view after async load.
+  const northPaneViews = useViews(ViewLocations.sqllab.northPane) || [];
+
+  // Resolve the per-tab localStorage key the same way every other SQL Lab
+  // consumer does (`tabViewId ?? id`), so the value written, read back, and
+  // observed via the `storage` event all agree once a tab is 
backend-persisted.
+  const northPaneStorageId = queryEditor.tabViewId ?? queryEditor.id;
+
+  // ID of the northPane view active for this tab, or null for the default
+  // SQL editor layout.  Set by an extension via PENDING_NORTH_PANE_VIEW_KEY
+  // before calling createTab(); persisted per-tab in localStorage.
+  const [northPaneViewId, setNorthPaneViewId] = useState<string | null>(() => {
+    const pendingViewId = localStorage.getItem(PENDING_NORTH_PANE_VIEW_KEY);
+    if (pendingViewId) {
+      localStorage.removeItem(PENDING_NORTH_PANE_VIEW_KEY);
+      localStorage.setItem(
+        NORTH_PANE_VIEW_KEY(northPaneStorageId),
+        pendingViewId,
+      );
+      return pendingViewId;
+    }
+    return localStorage.getItem(NORTH_PANE_VIEW_KEY(northPaneStorageId));
+  });
+
+  useEffect(() => {
+    const persistKey = NORTH_PANE_VIEW_KEY(northPaneStorageId);
+    if (northPaneViewId) {
+      localStorage.setItem(persistKey, northPaneViewId);
+    } else {
+      localStorage.removeItem(persistKey);
+    }
+  }, [northPaneStorageId, northPaneViewId]);

Review Comment:
   Fair point — wrapped the northPane `localStorage` reads/writes in try/catch 
so a storage-restricted browser can't crash the editor mount.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to