Skip to content

fix(SqlLab): South pane visual changes #35601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
msyavuz merged 11 commits into from
Oct 27, 2025
Merged

fix(SqlLab): South pane visual changes #35601

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
50e046e
fix(SqlLab): South pane visuals
msyavuz Oct 10, 2025
d972a95
fix: paddings for other tabs
msyavuz Oct 10, 2025
e14a28b
test(ActionButton): add test for the component
msyavuz Oct 10, 2025
96e697f
fix: smaller icons and spacing
msyavuz Oct 10, 2025
f1b312f
fix(TablePreview): update failing test
msyavuz Oct 10, 2025
232c427
fix: sanitize tooltip id
msyavuz Oct 13, 2025
cae826c
Merge branch 'master' into msyavuz/fix/south-pane-visuals
msyavuz Oct 27, 2025
2f50822
fix: attend pr review
msyavuz Oct 27, 2025
df2ecf8
chore: move to core packages
msyavuz Oct 27, 2025
fc603f5
fix: localization
msyavuz Oct 27, 2025
9f1542b
fix: ci
msyavuz Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion superset-frontend/packages/superset-ui-core/src/components/Tabs/Tabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,18 @@ import { css, styled, useTheme } from '@superset-ui/core';
// eslint-disable-next-line no-restricted-imports
import { Tabs as AntdTabs, TabsProps as AntdTabsProps } from 'antd';
import { Icons } from '@superset-ui/core/components/Icons';
import type { SerializedStyles } from '@emotion/react';

export interface TabsProps extends AntdTabsProps {
allowOverflow?: boolean;
contentStyle?: SerializedStyles;
}

const StyledTabs = ({
animated = false,
allowOverflow = true,
tabBarStyle,
contentStyle,
...props
}: TabsProps) => {
const theme = useTheme();
Expand All @@ -46,6 +49,7 @@ const StyledTabs = ({

.ant-tabs-content-holder {
overflow: ${allowOverflow ? 'visible' : 'auto'};
${contentStyle}

@korbit-ai korbit-ai Bot Oct 10, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing null check for contentStyle CSS interpolation category Functionality

Tell me more
What is the issue?

The contentStyle prop is being interpolated directly into CSS without null/undefined checks, which could cause rendering issues if the prop is not provided.

Why this matters

When contentStyle is undefined or null, the CSS interpolation will render 'undefined' or 'null' as literal text in the CSS, potentially breaking styles or causing console warnings.

Suggested change ∙ Feature Preview

Add a null check before interpolating contentStyle:

${contentStyle || ''}

Or use optional chaining with nullish coalescing:

${contentStyle ?? ''}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

}
.ant-tabs-tab {
flex: 1 1 auto;
Expand Down Expand Up @@ -85,9 +89,10 @@ const Tabs = Object.assign(StyledTabs, {
});

const StyledEditableTabs = styled(StyledTabs)`
${({ theme }) => `
${({ theme, contentStyle }) => `
.ant-tabs-content-holder {
background: ${theme.colorBgContainer};
${contentStyle}
}

& > .ant-tabs-nav {
Expand Down
10 changes: 8 additions & 2 deletions superset-frontend/src/SqlLab/components/QueryHistory/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
css,
FeatureFlag,
isFeatureEnabled,
useTheme,
} from '@superset-ui/core';
import QueryTable from 'src/SqlLab/components/QueryTable';
import { SqlLabRootState } from 'src/SqlLab/types';
Expand Down Expand Up @@ -67,6 +68,7 @@ const QueryHistory = ({
const { id, tabViewId } = useQueryEditor(String(queryEditorId), [
'tabViewId',
]);
const theme = useTheme();
const editorId = tabViewId ?? id;
const [ref, hasReachedBottom] = useInView({ threshold: 0 });
const [pageIndex, setPageIndex] = useState(0);
Expand Down Expand Up @@ -118,7 +120,11 @@ const QueryHistory = ({
}

return editorQueries.length > 0 ? (
<>
<div
css={css`
padding-left: ${theme.sizeUnit * 4}px;
`}
>
<QueryTable
columns={[
'state',
Expand All @@ -144,7 +150,7 @@ const QueryHistory = ({
/>
)}
{isFetching && <Skeleton active />}
</>
</div>
) : (
<StyledEmptyStateWrapper>
<EmptyState
Expand Down
3 changes: 3 additions & 0 deletions superset-frontend/src/SqlLab/components/ResultSet/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ const ReturnedRows = styled.div`
const ResultSetControls = styled.div`
display: flex;
justify-content: space-between;
padding-left: ${({ theme }) => theme.sizeUnit * 4}px;
`;

const ResultSetButtons = styled.div`
Expand Down Expand Up @@ -661,6 +662,7 @@ const ResultSet = ({
css={css`
display: flex;
justify-content: space-between;
padding-left: ${theme.sizeUnit * 4}px;
align-items: center;
gap: ${GAP}px;
`}
Expand Down Expand Up @@ -696,6 +698,7 @@ const ResultSet = ({
<div
css={css`
flex: 1 1 auto;
padding-left: ${theme.sizeUnit * 4}px;
`}
>
<AutoSizer disableWidth>
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ const StyledSqlEditor = styled.div`

.queryPane {
padding: ${theme.sizeUnit * 2}px;
padding-left: 0px;
+ .ant-splitter-bar .ant-splitter-bar-dragger {
&::before {
background: transparent;
Expand Down
14 changes: 6 additions & 8 deletions superset-frontend/src/SqlLab/components/TablePreview/TablePreview.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,32 +138,30 @@ test('renders preview', async () => {
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
describe('table actions', () => {
test('refreshes table metadata when triggered', async () => {
const { getByRole, getByText } = render(<TablePreview {...mockedProps} />, {
const { getByRole } = render(<TablePreview {...mockedProps} />, {
useRedux: true,
initialState,
});
await waitFor(() =>
expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(1),
);
const menuButton = getByRole('button', { name: /Table actions/i });
fireEvent.click(menuButton);
fireEvent.click(getByText('Refresh table schema'));
const refreshButton = getByRole('button', { name: 'sync' });
fireEvent.click(refreshButton);
await waitFor(() =>
expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(2),
);
});

test('shows CREATE VIEW statement', async () => {
const { getByRole, getByText } = render(<TablePreview {...mockedProps} />, {
const { getByRole } = render(<TablePreview {...mockedProps} />, {
useRedux: true,
initialState,
});
await waitFor(() =>
expect(fetchMock.calls(getTableMetadataEndpoint)).toHaveLength(1),
);
const menuButton = getByRole('button', { name: /Table actions/i });
fireEvent.click(menuButton);
fireEvent.click(getByText('Show CREATE VIEW statement'));
const viewButton = getByRole('button', { name: 'eye' });
fireEvent.click(viewButton);
await waitFor(() =>
expect(
screen.queryByRole('dialog', { name: 'CREATE VIEW statement' }),
Expand Down
109 changes: 51 additions & 58 deletions superset-frontend/src/SqlLab/components/TablePreview/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ import {
getExtensionsRegistry,
styled,
t,
useTheme,
} from '@superset-ui/core';
import {
SafeMarkdown,
Alert,
Breadcrumb,
Button,
Card,
Dropdown,
Skeleton,
Flex,
} from '@superset-ui/core/components';
import AutoSizer from 'react-virtualized-auto-sizer';
import { Icons } from '@superset-ui/core/components/Icons';
Expand All @@ -47,7 +47,7 @@ import {
useTableMetadataQuery,
} from 'src/hooks/apiResources';
import { runTablePreviewQuery } from 'src/SqlLab/actions/sqlLab';
import { Menu } from '@superset-ui/core/components/Menu';
import { ActionButton } from 'src/components/ActionButton';
import ResultSet from '../ResultSet';
import ShowSQL from '../ShowSQL';

Expand All @@ -68,23 +68,6 @@ const TABS_KEYS = {
INDEXES: 'indexes',
SAMPLE: 'sample',
};
const MENUS = [
{
key: 'refresh-table',
label: t('Refresh table schema'),
icon: <Icons.SyncOutlined iconSize="s" aria-hidden />,
},
{
key: 'copy-select-statement',
label: t('Copy SELECT statement'),
icon: <Icons.CopyOutlined iconSize="s" aria-hidden />,
},
{
key: 'show-create-view-statement',
label: t('Show CREATE VIEW statement'),
icon: <Icons.EyeOutlined iconSize="s" aria-hidden />,
},
];
const TAB_HEADER_HEIGHT = 80;
const PREVIEW_QUERY_LIMIT = 100;

Expand All @@ -96,6 +79,8 @@ const Title = styled.div`
column-gap: ${theme.sizeUnit}px;
font-size: ${theme.fontSizeLG}px;
font-weight: ${theme.fontWeightStrong};
padding-top: ${theme.sizeUnit * 2}px;
padding-left: ${theme.sizeUnit * 4}px;
`}
`;
const renderWell = (partitions: TableMetaData['partitions']) => {
Expand Down Expand Up @@ -133,6 +118,7 @@ const renderWell = (partitions: TableMetaData['partitions']) => {

const TablePreview: FC<Props> = ({ dbId, catalog, schema, tableName }) => {
const dispatch = useDispatch();
const theme = useTheme();
const [databaseName, backend, disableDataPreview] = useSelector<
SqlLabRootState,
string[]
Expand Down Expand Up @@ -240,16 +226,40 @@ const TablePreview: FC<Props> = ({ dbId, catalog, schema, tableName }) => {
],
);

const dropdownMenu = useMemo(() => {
let menus = [...MENUS];
if (!tableData.selectStar) {
menus = menus.filter(({ key }) => key !== 'copy-select-statement');
}
if (!tableData.view) {
menus = menus.filter(({ key }) => key !== 'show-create-view-statement');
}
return menus;
}, [tableData.view, tableData.selectStar]);
const titleActions = () => (
<Flex
align="center"
css={css`
padding-left: ${theme.sizeUnit * 2}px;
`}
>
<ActionButton
label="Refresh table schema"
Comment thread
msyavuz marked this conversation as resolved.
Outdated
tooltip={t('Refresh table schema')}
icon="SyncOutlined"
iconSize="m"
onClick={refreshTableMetadata}
/>
{tableData.selectStar && (
<ActionButton
label="Copy SELECT statement"
icon="CopyOutlined"
Comment thread
msyavuz marked this conversation as resolved.
Outdated
iconSize="m"
tooltip={t('Copy SELECT statement')}
onClick={() => copyStatementActionRef.current?.click()}
/>
)}
{tableData.view && (
<ActionButton
label="Show CREATE VIEW statement"
icon="EyeOutlined"
iconSize="m"
tooltip={t('Show CREATE VIEW statement')}
onClick={() => showViewStatementActionRef.current?.click()}
/>
)}
</Flex>
);
Comment on lines +229 to +259

@korbit-ai korbit-ai Bot Oct 10, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unmemoized Component Function category Design

Tell me more
What is the issue?

The titleActions function is defined inside the component, causing it to be recreated on every render. It should be memoized or moved outside the component since it depends on stable props and callbacks.

Why this matters

Recreating the function on every render can lead to unnecessary re-renders of child components and impact performance, especially in larger applications.

Suggested change ∙ Feature Preview

Use useMemo to memoize the titleActions function:

const titleActions = useMemo(
  () => (
    <Flex>...
    </Flex>
  ),
  [theme, tableData.selectStar, tableData.view, refreshTableMetadata]
);
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


if (isMetadataLoading) {
return <Skeleton active />;
Expand Down Expand Up @@ -282,7 +292,12 @@ const TablePreview: FC<Props> = ({ dbId, catalog, schema, tableName }) => {
flex-direction: column;
`}
>
<Breadcrumb separator=">">
<Breadcrumb
separator=">"
css={css`
padding-left: ${theme.sizeUnit * 4}px;
`}
>
<Breadcrumb.Item>{backend}</Breadcrumb.Item>
<Breadcrumb.Item>{databaseName}</Breadcrumb.Item>
{catalog && <Breadcrumb.Item>{catalog}</Breadcrumb.Item>}
Expand Down Expand Up @@ -315,33 +330,7 @@ const TablePreview: FC<Props> = ({ dbId, catalog, schema, tableName }) => {
<Title>
<Icons.InsertRowAboveOutlined iconSize="l" />
{tableName}
<Dropdown
popupRender={() => (
<Menu
onClick={({ key }) => {
if (key === 'refresh-table') {
refreshTableMetadata();
}
if (key === 'copy-select-statement') {
copyStatementActionRef.current?.click();
}
if (key === 'show-create-view-statement') {
showViewStatementActionRef.current?.click();
}
}}
items={dropdownMenu}
/>
)}
trigger={['click']}
>
<Button buttonSize="xsmall" buttonStyle="link">
<Icons.DownSquareOutlined
iconSize="m"
style={{ marginTop: 2, marginLeft: 4 }}
aria-label={t('Table actions')}
/>
</Button>
</Dropdown>
{titleActions()}
</Title>
{isMetadataRefreshing ? (
<Skeleton active />
Expand Down Expand Up @@ -440,7 +429,11 @@ const TablePreview: FC<Props> = ({ dbId, catalog, schema, tableName }) => {
css={css`
height: ${height}px;
`}
tabBarStyle={{ paddingLeft: theme.sizeUnit * 4 }}
items={tabItems}
contentStyle={css`
padding-left: ${theme.sizeUnit * 4}px;
`}
/>
);
}}
Expand Down
Loading
Loading