rusackas commented on code in PR #41285:
URL: https://github.com/apache/superset/pull/41285#discussion_r4078342368
##########
superset-frontend/src/SqlLab/contributions.ts:
##########
@@ -46,5 +46,27 @@ export const ViewLocations = {
statusBar: 'sqllab.statusBar',
results: 'sqllab.results',
queryHistory: 'sqllab.queryHistory',
+ // Extensions can register a full-pane replacement here. SqlEditor renders
+ // the registered view instead of the default editor+SouthPane split when
+ // a tab was opened in that mode.
+ northPane: 'sqllab.northPane',
+ // Extensions register tab-type commands here. When any are present the
+ // "+" new-tab button becomes a dropdown listing all registered tab types
+ // plus the built-in SQL Editor option.
+ newTab: 'sqllab.newTab',
},
} as const;
+
+/**
+ * localStorage key an extension sets before calling createTab() to declare
+ * which northPane view the new tab should open with. The value must be the
+ * view ID passed to views.registerView() (e.g. "my-ext.northPane"). SqlEditor
+ * consumes and removes this key during initialization, then persists the
chosen
+ * view ID under a per-tab key so the mode survives page reloads.
+ *
+ * @example
+ * // In an extension's newTab command handler:
+ * localStorage.setItem(PENDING_NORTH_PANE_VIEW_KEY, 'my-ext.northPane');
+ * sqlLab.createTab({ title: 'My View' });
+ */
+export const PENDING_NORTH_PANE_VIEW_KEY = 'sqllab.pendingNorthPaneView';
Review Comment:
Good call, that shared mailbox was a footgun. I dropped
`PENDING_NORTH_PANE_VIEW_KEY` entirely and moved it onto tab-creation state:
`createTab({ northPaneViewId })` stamps the view onto the new query editor
itself, and SqlEditor reads it from there (still persisting the per-tab
localStorage entry so it survives a reload that rehydrates from the backend
without the field).
##########
superset-frontend/src/core/sqlLab/index.ts:
##########
@@ -80,6 +80,19 @@ const findQueryEditor = (editorId: string) => {
return editor;
};
+/**
+ * Resolves the backend-assigned id for a query editor, if it has one. A tab
+ * created locally and later synced carries it in `tabViewId`; a tab hydrated
+ * from the backend on page load uses that id directly as its `queryEditor.id`
+ * (with `inLocalStorage` unset). Only a tab that still lives solely in local
+ * storage has no backend id yet.
+ */
+const resolveBackendId = (
Review Comment:
Good catch. `extractBaseData()` resolves the editor by `id` or `tabViewId`
(since a synced tab's query carries the backend id as `sqlEditorId`) and runs
it through the same `resolveBackendId()` as the other paths, with tests for
both shapes.
##########
superset-frontend/src/SqlLab/contributions.ts:
##########
@@ -46,5 +46,27 @@ export const ViewLocations = {
statusBar: 'sqllab.statusBar',
results: 'sqllab.results',
queryHistory: 'sqllab.queryHistory',
+ // Extensions can register a full-pane replacement here. SqlEditor renders
+ // the registered view instead of the default editor+SouthPane split when
+ // a tab was opened in that mode.
+ northPane: 'sqllab.northPane',
Review Comment:
Yep, missed the public union. Added `northPane` and `newTab` to
`SqlLabLocation` in `superset-core`.
##########
superset-frontend/src/extensions/ExtensionsStartup.tsx:
##########
@@ -67,9 +70,30 @@ const ExtensionsStartup: React.FC<{ children?:
React.ReactNode }> = ({
views,
};
+ // Load extensions without blocking the initial render (see #40915);
+ // surface any load failure as a warning toast instead of failing silently.
+ // ExtensionsLoader already logs the details of each failure, so only the
+ // user-facing toast is raised here.
if (isFeatureEnabled(FeatureFlag.EnableExtensions)) {
- ExtensionsLoader.getInstance().initializeExtensions();
+ ExtensionsLoader.getInstance()
+ .initializeExtensions()
+ .then(failed => {
+ if (failed.length > 0) {
+ dispatch(
+ addWarningToast(
+ t('Some extensions failed to load: %s', failed.join(', ')),
+ ),
+ );
+ }
+ })
+ .catch((error: unknown) => {
+ dispatch(
+ addWarningToast(t('Extensions failed to load: %s', String(error))),
Review Comment:
Agreed, `[object Response]` is useless in a toast. Swapped in
`getClientErrorObject()` so the message/status from the failed fetch is what
gets shown, with a test covering a 403 Response rejection.
--
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]
