fix: Create dataset polish/bug fix (#22262)

apache · Dec 20, 2022 · 6b20e74 · 6b20e74
1 parent 7f4e522
commit 6b20e74
Show file tree

Hide file tree

Showing 10 changed files with 166 additions and 89 deletions.
diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
@@ -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,
@@ -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));
@@ -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}

diff --git a/superset-frontend/src/components/EmptyState/index.tsx b/superset-frontend/src/components/EmptyState/index.tsx
@@ -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';
 
@@ -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>
+    }
+  />
+);
diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/AddDataset.test.tsx
@@ -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

diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.test.tsx
@@ -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,

diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx
@@ -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
@@ -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;
@@ -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> = [
@@ -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;
@@ -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 {
@@ -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

diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/Header.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/Header.test.tsx
@@ -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');
   });

diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/index.tsx
@@ -26,6 +26,7 @@ import { TooltipPlacement } from 'src/components/Tooltip';
 import {
   HeaderComponentStyles,
   disabledSaveBtnStyles,
+  StyledCreateDatasetTitle,
 } from 'src/views/CRUD/data/dataset/styles';
 import {
   DatasetActionType,
@@ -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,
@@ -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>
   );
 }