rusackas commented on code in PR #41285:
URL: https://github.com/apache/superset/pull/41285#discussion_r3475073463
##########
superset-frontend/src/extensions/ExtensionsStartup.tsx:
##########
@@ -67,9 +70,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:
This is intentional — one broken extension shouldn't blank out the others,
so `initializeExtension` logs each failure individually and keeps going. The
toast is reserved for a top-level failure (the API fetch), which does reject
and surface. A per-extension failure summary is a fair future enhancement but
out of scope here.
##########
superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx:
##########
@@ -98,6 +101,101 @@ 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 activate = () => {
+ const primaryItems =
+ menus.getMenu(ViewLocations.sqllab.newTab)?.primary ?? [];
+ if (primaryItems.length === 0) {
+ onAddSqlEditor();
+ } else {
+ setOpen(prev => !prev);
+ }
+ };
+
+ 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();
+ activate();
+ };
+
+ const handleKeyDown = (e: React.KeyboardEvent) => {
+ // The same wrapper button activates on Enter/Space; intercept those keys
so
+ // keyboard users reach the extension new-tab dropdown rather than antd's
+ // default add-tab path.
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ e.stopPropagation();
+ activate();
+ }
+ };
+
+ return (
+ <Tooltip id="add-tab" placement="left" title={newTabTooltip}>
+ <Dropdown
+ open={open}
+ onOpenChange={setOpen}
+ menu={{ items: dropdownItems }}
+ trigger={[]}
+ >
+ <span
+ role="button"
+ tabIndex={0}
+ onClick={handleClick}
+ onKeyDown={handleKeyDown}
+ >
Review Comment:
Both paths are already wired on the focusable inner element: `tabIndex={0}`
+ `onKeyDown` handles Enter/Space for keyboard users, and `onClick` calls
`stopPropagation()` before antd's `onEdit("add")` can fire. This is the
conventional antd `addIcon` pattern, so the dropdown owns activation.
##########
superset-frontend/src/core/sqlLab/models.ts:
##########
@@ -62,6 +65,7 @@ export class Tab implements sqlLabType.Tab {
this.schema = schema;
this.editorGetter = editorGetter;
this.panels = panels;
+ this.backendId = backendId;
Review Comment:
Intentional — `backendId` mirrors `tabViewId` only, and the `getTabs leaves
backendId undefined when the editor has no tabViewId` test documents that.
Deriving it from `id` for bootstrapped tabs conflates two different
identifiers; I'd rather populate `tabViewId` at bootstrap than overload
`backendId` here.
--
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]
