rusackas commented on code in PR #41285:
URL: https://github.com/apache/superset/pull/41285#discussion_r3453927090
##########
superset-frontend/packages/superset-core/src/sqlLab/index.ts:
##########
@@ -62,6 +62,13 @@ export interface Tab {
*/
id: string;
+ /**
+ * The stable backend-assigned ID for this tab (the tabstateview integer ID).
+ * Set once the tab has been persisted to the backend. Undefined for new tabs
+ * before the first backend sync.
+ */
+ backendId?: string;
Review Comment:
Kept `backendId` as an opaque string deliberately so the public extension
API doesn't leak the internal numeric tab id — clarified the docstring to say
as much rather than calling it an integer id.
##########
superset-frontend/packages/superset-core/src/sqlLab/index.ts:
##########
@@ -62,6 +62,13 @@ export interface Tab {
*/
id: string;
+ /**
+ * The stable backend-assigned ID for this tab (the tabstateview integer ID).
+ * Set once the tab has been persisted to the backend. Undefined for new tabs
+ * before the first backend sync.
+ */
+ backendId?: string;
Review Comment:
Leaving this as a string on purpose — we don't want the public extension API
exposing the backend's internal numeric id. Fixed the misleading docstring that
called it an integer.
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.tsx:
##########
@@ -271,6 +279,44 @@ 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) || [];
+
+ // 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(queryEditor.id), pendingViewId);
+ return pendingViewId;
+ }
+ return localStorage.getItem(NORTH_PANE_VIEW_KEY(queryEditor.id));
+ });
+
+ useEffect(() => {
+ const persistKey = NORTH_PANE_VIEW_KEY(
+ queryEditor.tabViewId ?? queryEditor.id,
+ );
+ if (northPaneViewId) {
+ localStorage.setItem(persistKey, northPaneViewId);
+ } else {
+ localStorage.removeItem(persistKey);
+ }
+ }, [queryEditor.tabViewId, queryEditor.id, northPaneViewId]);
+
+ useEffect(() => {
+ const handler = (e: StorageEvent) => {
+ if (e.key === NORTH_PANE_VIEW_KEY(queryEditor.id)) {
+ setNorthPaneViewId(e.newValue || null);
+ }
+ };
+ window.addEventListener('storage', handler);
+ return () => window.removeEventListener('storage', handler);
+ }, [queryEditor.id]);
Review Comment:
Fixed the storage-listener key mismatch — read/write/listener all resolve to
`tabViewId ?? id` now. Also added coverage for the extension load-failure path.
--
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]
