bito-code-review[bot] commented on code in PR #41285:
URL: https://github.com/apache/superset/pull/41285#discussion_r3453928259
##########
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:
<!-- Bito Reply -->
The suggestion to change the type from `string` to `number` is appropriate
because it aligns the frontend type definition with the backend API's actual
return type, as noted in the review. This change helps prevent potential type
mismatches and runtime errors when handling the backend-assigned ID.
**superset-frontend/packages/superset-core/src/sqlLab/index.ts**
```
70: - backendId?: string;
70: + backendId?: number;
```
##########
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:
<!-- Bito Reply -->
The suggestion to align the storage listener key with the persistence key is
correct and necessary for consistent cross-tab state synchronization. By
ensuring the listener watches `NORTH_PANE_VIEW_KEY(queryEditor.tabViewId ??
queryEditor.id)`, you prevent the synchronization issues that occur when
`tabViewId` and `id` differ.
--
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]
