rusackas commented on code in PR #41285:
URL: https://github.com/apache/superset/pull/41285#discussion_r3453925845
##########
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));
Review Comment:
Good catch — fixed. The read and the storage listener now resolve the key
the same way as the write (`tabViewId ?? id`), so persisted tabs restore
correctly.
##########
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);
Review Comment:
Yep — the listener now keys off the same resolved `tabViewId ?? id` as the
write, so cross-window sync holds for persisted tabs.
##########
superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx:
##########
@@ -98,6 +101,82 @@ const AddTabIconWrapper = styled.span`
// Get the user's OS
const userOS = detectOS();
+const newTabTooltip =
+ userOS === 'Windows' ? t('New tab (Ctrl + q)') : t('New tab (Ctrl + t)');
+
+const PlusIcon = (
+ <AddTabIconWrapper>
+ <Icons.PlusOutlined iconSize="l" data-test="add-tab-icon" />
+ </AddTabIconWrapper>
+);
+
+function NewTabButton({ onAddSqlEditor }: { onAddSqlEditor: () => void }) {
+ const [open, setOpen] = useState(false);
+
+ const dropdownItems = useMemo<MenuItemType[]>(() => {
+ if (!open) return [];
+ const primaryItems =
+ menus.getMenu(ViewLocations.sqllab.newTab)?.primary ?? [];
+ return [
+ {
+ key: 'sql-editor',
+ label: t('SQL Editor'),
+ icon: <Icons.TableOutlined iconSize="m" />,
+ onClick: () => {
+ setOpen(false);
+ onAddSqlEditor();
+ },
+ },
+ ...primaryItems.map(item => {
+ const command = commands.getCommand(item.command);
+ const Icon = command?.icon
+ ? ((Icons as Record<string, typeof Icons.FileOutlined>)[
+ command.icon
+ ] ?? Icons.FileOutlined)
+ : Icons.FileOutlined;
+ return {
+ key: command?.id ?? item.command,
+ label: command?.title ?? item.command,
+ icon: <Icon iconSize="m" />,
+ onClick: () => {
+ setOpen(false);
+ commands.executeCommand(item.command);
+ },
+ } as MenuItemType;
+ }),
+ ];
+ }, [open, onAddSqlEditor]);
+
+ const handleClick = (e: React.MouseEvent) => {
+ // Antd's Tabs wraps addIcon in its own <button onClick={() =>
onEdit('add')}>.
+ // Stop propagation so antd doesn't also call newQueryEditor() while we
handle it.
+ e.stopPropagation();
+ const primaryItems =
+ menus.getMenu(ViewLocations.sqllab.newTab)?.primary ?? [];
+ if (primaryItems.length === 0) {
+ onAddSqlEditor();
+ } else {
+ setOpen(prev => !prev);
+ }
+ };
+
+ return (
+ <Tooltip id="add-tab" placement="left" title={newTabTooltip}>
+ <Dropdown
+ open={open}
+ onOpenChange={setOpen}
+ menu={{ items: dropdownItems }}
+ trigger={[]}
+ >
+ {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events */}
+ <span role="button" tabIndex={0} onClick={handleClick}>
+ {PlusIcon}
Review Comment:
Fixed — added an `onKeyDown` so Enter/Space hit the same path as the click
and open the extension dropdown instead of antd's default add-tab.
##########
superset-frontend/src/extensions/ExtensionsStartup.tsx:
##########
@@ -72,9 +75,28 @@ 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.
if (isFeatureEnabled(FeatureFlag.EnableExtensions)) {
- ExtensionsLoader.getInstance().initializeExtensions();
+ ExtensionsLoader.getInstance()
+ .initializeExtensions()
+ .then(() =>
+ supersetCore.utils.logging.info(
+ 'Extensions initialized successfully.',
+ ),
+ )
+ .catch(error => {
+ supersetCore.utils.logging.error(
+ 'Error setting up extensions:',
+ error,
+ );
+ dispatch(
+ addWarningToast(t('Extensions failed to load: %s', String(error))),
+ );
+ });
Review Comment:
Right — the loader was swallowing its own failure. Made
`initializeExtensions` rethrow (and reset the promise so a retry can happen),
so the `.catch` here actually fires the warning toast now.
--
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]
