Skip to content
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.

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

fix: Create dataset polish/bug fix #22262

Merged
merged 8 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 2 additions & 19 deletions superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import { TableSelectorMultiple } from 'src/components/TableSelector';
import { IconTooltip } from 'src/components/IconTooltip';
import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor';
import { DatabaseObject } from 'src/components/DatabaseSelector';
import { EmptyStateSmall } from 'src/components/EmptyState';
import { emptyStateComponent } from 'src/components/EmptyState';
import {
getItem,
LocalStorageKeys,
Expand Down Expand Up @@ -197,23 +197,6 @@ const SqlEditorLeftBar = ({
const shouldShowReset = window.location.search === '?reset=1';
const tableMetaDataHeight = height - 130; // 130 is the height of the selects above

const emptyStateComponent = (
<EmptyStateSmall
image="empty.svg"
title={
emptyResultsWithSearch
? t('No databases match your search')
: t('There are no databases available')
}
description={
<p>
{t('Manage your databases')}{' '}
<a href="/databaseview/list">{t('here')}</a>
</p>
}
/>
);

const handleSchemaChange = useCallback((schema: string) => {
if (queryEditor) {
dispatch(queryEditorSetSchema(queryEditor, schema));
Expand Down Expand Up @@ -248,7 +231,7 @@ const SqlEditorLeftBar = ({
<div data-test="sql-editor-left-bar" className="SqlEditorLeftBar">
<TableSelectorMultiple
onEmptyResults={onEmptyResults}
emptyState={emptyStateComponent}
emptyState={emptyStateComponent(emptyResultsWithSearch)}
database={userSelectedDb}
getDbList={handleDbList}
handleError={handleError}
Expand Down
26 changes: 25 additions & 1 deletion superset-frontend/src/components/EmptyState/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import React, { ReactNode, SyntheticEvent } from 'react';
import { styled, css, SupersetTheme } from '@superset-ui/core';
import { styled, css, SupersetTheme, t } from '@superset-ui/core';
import { Empty } from 'src/components';
import Button from 'src/components/Button';

Expand Down Expand Up @@ -229,3 +229,27 @@ export const EmptyStateSmall = ({
</TextContainer>
</EmptyStateContainer>
);

const TRANSLATIONS = {
NO_DATABASES_MATCH_TITLE: t('No databases match your search'),
NO_DATABASES_AVAILABLE_TITLE: t('There are no databases available'),
MANAGE_YOUR_DATABASES_TEXT: t('Manage your databases'),
HERE_TEXT: t('here'),
};

export const emptyStateComponent = (emptyResultsWithSearch: boolean) => (
<EmptyStateSmall
image="empty.svg"
title={
emptyResultsWithSearch
? TRANSLATIONS.NO_DATABASES_MATCH_TITLE
: TRANSLATIONS.NO_DATABASES_AVAILABLE_TITLE
}
description={
<p>
{TRANSLATIONS.MANAGE_YOUR_DATABASES_TEXT}{' '}
<a href="/databaseview/list">{TRANSLATIONS.HERE_TEXT}</a>
</p>
}
/>
);
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('AddDataset', () => {
const blankeStateImgs = screen.getAllByRole('img', { name: /empty/i });

// Header
expect(await screen.findByTestId('editable-title')).toBeVisible();
expect(await screen.findByText(/new dataset/i)).toBeVisible();
// Left panel
expect(blankeStateImgs[0]).toBeVisible();
// Footer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import DatasetPanel, {
ALT_LOADING,
tableColumnDefinition,
COLUMN_TITLE,
} from './DatasetPanel';
} from 'src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel';
import { exampleColumns, exampleDataset } from './fixtures';
import {
SELECT_MESSAGE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ const HALF = 0.5;
const MARGIN_MULTIPLIER = 3;

const StyledHeader = styled.div<StyledHeaderProps>`
${({ theme }) => `
position: ${(props: StyledHeaderProps) => props.position};
${({ theme, position }) => `
position: ${position};
margin: ${theme.gridUnit * (MARGIN_MULTIPLIER + 1)}px
${theme.gridUnit * MARGIN_MULTIPLIER}px
${theme.gridUnit * MARGIN_MULTIPLIER}px
Expand Down Expand Up @@ -124,17 +124,27 @@ const StyledLoader = styled.div`
`}
`;

const TableContainer = styled.div`
const TableContainerWithBanner = styled.div`
${({ theme }) => `
position: relative;
margin: ${theme.gridUnit * MARGIN_MULTIPLIER}px;
margin-left: ${theme.gridUnit * (MARGIN_MULTIPLIER + 3)}px;
height: calc(100% - ${theme.gridUnit * 60}px);
overflow: auto;
`}
`;

const TableContainerWithoutBanner = styled.div`
${({ theme }) => `
position: relative;
margin: ${theme.gridUnit * MARGIN_MULTIPLIER}px;
margin-left: ${theme.gridUnit * (MARGIN_MULTIPLIER + 3)}px;
overflow: scroll;
height: calc(100% - ${theme.gridUnit * 36}px);
height: calc(100% - ${theme.gridUnit * 30}px);
overflow: auto;
`}
`;

const StyledTable = styled(Table)`
const TableScrollContainer = styled.div`
position: absolute;
left: 0;
top: 0;
Expand Down Expand Up @@ -167,6 +177,7 @@ export const COLUMN_TITLE = t('Table columns');
export const ALT_LOADING = t('Loading');

const pageSizeOptions = ['5', '10', '15', '25'];
const DEFAULT_PAGE_SIZE = 25;

// Define the columns for Table instance
export const tableColumnDefinition: ColumnsType<ITableColumn> = [
Expand Down Expand Up @@ -253,6 +264,9 @@ const DatasetPanel = ({
const theme = useTheme();
const hasColumns = columnList?.length > 0 ?? false;
const datasetNames = datasets?.map(dataset => dataset.table_name);
const tableWithDataset = datasets?.find(
dataset => dataset.table_name === tableName,
);

let component;
let loader;
Expand All @@ -271,16 +285,33 @@ const DatasetPanel = ({
component = (
<>
<StyledTitle>{COLUMN_TITLE}</StyledTitle>
<TableContainer>
<StyledTable
loading={loading}
size={TableSize.SMALL}
columns={tableColumnDefinition}
data={columnList}
pageSizeOptions={pageSizeOptions}
defaultPageSize={10}
/>
</TableContainer>
{tableWithDataset ? (
<TableContainerWithBanner>
<TableScrollContainer>
<Table
loading={loading}
size={TableSize.SMALL}
columns={tableColumnDefinition}
data={columnList}
pageSizeOptions={pageSizeOptions}
defaultPageSize={DEFAULT_PAGE_SIZE}
/>
</TableScrollContainer>
</TableContainerWithBanner>
) : (
<TableContainerWithoutBanner>
<TableScrollContainer>
<Table
loading={loading}
size={TableSize.SMALL}
columns={tableColumnDefinition}
data={columnList}
pageSizeOptions={pageSizeOptions}
defaultPageSize={DEFAULT_PAGE_SIZE}
/>
</TableScrollContainer>
</TableContainerWithoutBanner>
)}
</>
);
} else {
Expand All @@ -299,9 +330,7 @@ const DatasetPanel = ({
{tableName && (
<>
{datasetNames?.includes(tableName) &&
renderExistingDatasetAlert(
datasets?.find(dataset => dataset.table_name === tableName),
)}
renderExistingDatasetAlert(tableWithDataset)}
<StyledHeader
position={
!loading && hasColumns ? EPosition.RELATIVE : EPosition.ABSOLUTE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,33 +31,23 @@ describe('Header', () => {
test('renders a blank state Header', async () => {
await waitForRender();

const datasetName = screen.getByTestId('editable-title');
const saveButton = screen.getByRole('button', {
name: /save save/i,
});
const menuButton = screen.getByRole('button', {
name: /menu actions trigger/i,
});
const datasetName = screen.getByText(/new dataset/i);

expect(datasetName).toBeVisible();
expect(saveButton).toBeVisible();
expect(saveButton).toBeDisabled();
expect(menuButton).toBeVisible();
expect(menuButton).toBeDisabled();
});

test('displays "New dataset" when a table is not selected', async () => {
await waitForRender();

const datasetName = screen.getByTestId('editable-title');
const datasetName = screen.getByText(/new dataset/i);
expect(datasetName.innerHTML).toBe(DEFAULT_TITLE);
});

test('displays table name when a table is selected', async () => {
// The schema and table name are passed in through props once selected
await waitForRender({ schema: 'testSchema', title: 'testTable' });

const datasetName = screen.getByTestId('editable-title');
const datasetName = screen.getByText(/testtable/i);

expect(datasetName.innerHTML).toBe('testTable');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { TooltipPlacement } from 'src/components/Tooltip';
import {
HeaderComponentStyles,
disabledSaveBtnStyles,
StyledCreateDatasetTitle,
} from 'src/views/CRUD/data/dataset/styles';
import {
DatasetActionType,
Expand Down Expand Up @@ -62,10 +63,12 @@ const renderOverlay = () => (
export default function Header({
setDataset,
title = DEFAULT_TITLE,
editing = false,
}: {
setDataset: React.Dispatch<DSReducerActionType>;
title?: string | null | undefined;
schema?: string | null | undefined;
editing?: boolean;
}) {
const editableTitleProps = {
title: title ?? DEFAULT_TITLE,
Expand All @@ -82,19 +85,25 @@ export default function Header({

return (
<HeaderComponentStyles>
<PageHeaderWithActions
editableTitleProps={editableTitleProps}
showTitlePanelItems={false}
showFaveStar={false}
faveStarProps={{ itemId: 1, saveFaveStar: () => {} }}
titlePanelAdditionalItems={<></>}
rightPanelAdditionalItems={renderDisabledSaveButton()}
additionalActionsMenu={renderOverlay()}
menuDropdownProps={{
disabled: true,
}}
tooltipProps={tooltipProps}
/>
{editing ? (
<PageHeaderWithActions
editableTitleProps={editableTitleProps}
showTitlePanelItems={false}
showFaveStar={false}
faveStarProps={{ itemId: 1, saveFaveStar: () => {} }}
titlePanelAdditionalItems={<></>}
rightPanelAdditionalItems={renderDisabledSaveButton()}
additionalActionsMenu={renderOverlay()}
menuDropdownProps={{
disabled: true,
}}
tooltipProps={tooltipProps}
/>
) : (
<StyledCreateDatasetTitle>
{title || DEFAULT_TITLE}
</StyledCreateDatasetTitle>
)}
</HeaderComponentStyles>
);
}