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

2022-12-20 10:05:40 -06:00 · 2022-12-20 10:05:40 -06:00 · 6b20e74442
parent 7f4e522f1a
commit 6b20e74442
10 changed files with 166 additions and 89 deletions
--- 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}
--- 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>
+    }
+  />
+);
--- 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
--- 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,
--- 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;
-  overflow: scroll;
-  height: calc(100% - ${theme.gridUnit * 36}px);
+  height: calc(100% - ${theme.gridUnit * 60}px);
+  overflow: auto;
  `}
 `;

-const StyledTable = styled(Table)`
+const TableContainerWithoutBanner = styled.div`
+  ${({ theme }) => `
+  position: relative;
+  margin: ${theme.gridUnit * MARGIN_MULTIPLIER}px;
+  margin-left: ${theme.gridUnit * (MARGIN_MULTIPLIER + 3)}px;
+  height: calc(100% - ${theme.gridUnit * 30}px);
+  overflow: auto;
+  `}
+`;
+
+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
--- 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,25 +31,15 @@ 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);
  });

@ -57,7 +47,7 @@ describe('Header', () => {
    // 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');
  });
--- 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>
  );
 }
--- a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx
+++ b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx
@ -35,7 +35,10 @@ import Loading from 'src/components/Loading';
 import DatabaseSelector, {
  DatabaseObject,
 } from 'src/components/DatabaseSelector';
-import { EmptyStateMedium } from 'src/components/EmptyState';
+import {
+  EmptyStateMedium,
+  emptyStateComponent,
+} from 'src/components/EmptyState';
 import { useToasts } from 'src/components/MessageToasts/withToasts';
 import { DatasetActionType } from '../types';

@ -63,7 +66,7 @@ const LeftPanelStyle = styled.div`
    }
    .refresh {
      position: absolute;
-      top: ${theme.gridUnit * 37.25}px;
+      top: ${theme.gridUnit * 38.75}px;
      left: ${theme.gridUnit * 16.75}px;
      span[role="button"]{
        font-size: ${theme.gridUnit * 4.25}px;
@ -87,6 +90,10 @@ const LeftPanelStyle = styled.div`
      left: ${theme.gridUnit * 3.25}px;
      right: 0;

+      .no-scrollbar {
+        margin-right: ${theme.gridUnit * 4}px;
+      }
+
      .options {
        cursor: pointer;
        padding: ${theme.gridUnit * 1.75}px;
@ -112,7 +119,7 @@ const LeftPanelStyle = styled.div`
    }
    form > span[aria-label="refresh"] {
      position: absolute;
-      top: ${theme.gridUnit * 67.5}px;
+      top: ${theme.gridUnit * 69}px;
      left: ${theme.gridUnit * 42.75}px;
      font-size: ${theme.gridUnit * 4.25}px;
    }
@ -121,13 +128,13 @@ const LeftPanelStyle = styled.div`
    }
    .loading-container {
      position: absolute;
-      top: 359px;
+      top: ${theme.gridUnit * 89.75}px;
      left: 0;
      right: 0;
      text-align: center;
      img {
        width: ${theme.gridUnit * 20}px;
-        margin-bottom: 10px;
+        margin-bottom: ${theme.gridUnit * 2.5}px;
      }
      p {
        color: ${theme.colors.grayscale.light1};
@ -240,6 +247,15 @@ export default function LeftPanel({
  const REFRESH_TABLES_TEXT = t('Refresh tables');
  const SEARCH_TABLES_PLACEHOLDER_TEXT = t('Search tables');

+  const optionsList = document.getElementsByClassName('options-list');
+  const scrollableOptionsList =
+    optionsList[0]?.scrollHeight > optionsList[0]?.clientHeight;
+  const [emptyResultsWithSearch, setEmptyResultsWithSearch] = useState(false);
+
+  const onEmptyResults = (searchText?: string) => {
+    setEmptyResultsWithSearch(!!searchText);
+  };
+
  return (
    <LeftPanelStyle>
      <p className="section-title db-schema">
@ -249,6 +265,8 @@ export default function LeftPanel({
        handleError={addDangerToast}
        onDbChange={setDatabase}
        onSchemaChange={setSchema}
+        emptyState={emptyStateComponent(emptyResultsWithSearch)}
+        onEmptyResults={onEmptyResults}
      />
      {loadTables && !refresh && Loader(TABLE_LOADING_TEXT)}
      {schema && !loadTables && !tableOptions.length && !searchVal && (
@ -291,7 +309,13 @@ export default function LeftPanel({
              filteredOptions.map((option, i) => (
                <div
                  className={
-                    selectedTable === i ? 'options-highlighted' : 'options'
+                    selectedTable === i
+                      ? scrollableOptionsList
+                        ? 'options-highlighted'
+                        : 'options-highlighted no-scrollbar'
+                      : scrollableOptionsList
+                      ? 'options'
+                      : 'options no-scrollbar'
                  }
                  key={i}
                  role="button"
@ -308,7 +332,7 @@ export default function LeftPanel({
                      }
                      iconSize="m"
                      css={css`
-                        margin-right: ${theme.gridUnit * 6}px;
+                        margin-right: ${theme.gridUnit * 2}px;
                      `}
                    />
                  )}
--- a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/DatasetLayout.test.tsx
+++ b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/DatasetLayout.test.tsx
@ -41,7 +41,7 @@ describe('DatasetLayout', () => {
  it('renders a Header when passed in', async () => {
    await waitForRender();

-    expect(screen.getByTestId('editable-title')).toBeVisible();
+    expect(screen.getByText(/new dataset/i)).toBeVisible();
  });

  it('renders a LeftPanel when passed in', async () => {
--- a/superset-frontend/src/views/CRUD/data/dataset/styles.ts
+++ b/superset-frontend/src/views/CRUD/data/dataset/styles.ts
@ -65,19 +65,31 @@ export const FooterRow = styled(Row)`
 `;

 export const StyledLayoutHeader = styled.div`
+  ${({ theme }) => `
  flex: 0 0 auto;
-  height: ${({ theme }) => theme.gridUnit * 16}px;
-  border-bottom: 2px solid ${({ theme }) => theme.colors.grayscale.light2};
+  height: ${theme.gridUnit * 16}px;
+  border-bottom: 2px solid ${theme.colors.grayscale.light2};

  .header-with-actions {
-    height: ${({ theme }) => theme.gridUnit * 15.5}px;
+    height: ${theme.gridUnit * 15.5}px;
  }
+  `}
+`;
+
+export const StyledCreateDatasetTitle = styled.div`
+  ${({ theme }) => `
+  margin: ${theme.gridUnit * 4}px;
+  font-size: ${theme.typography.sizes.xl}px;
+  font-weight: ${theme.typography.weights.bold};
+  `}
 `;

 export const StyledLayoutLeftPanel = styled.div`
-  width: ${({ theme }) => theme.gridUnit * 80}px;
+  ${({ theme }) => `
+  width: ${theme.gridUnit * 80}px;
  height: 100%;
-  border-right: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
+  border-right: 1px solid ${theme.colors.grayscale.light2};
+  `}
 `;

 export const StyledLayoutDatasetPanel = styled.div`
@ -86,21 +98,27 @@ export const StyledLayoutDatasetPanel = styled.div`
 `;

 export const StyledLayoutRightPanel = styled.div`
-  border-left: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
-  color: ${({ theme }) => theme.colors.success.base};
+  ${({ theme }) => `
+  border-left: 1px solid ${theme.colors.grayscale.light2};
+  color: ${theme.colors.success.base};
+  `}
 `;

 export const StyledLayoutFooter = styled.div`
-  height: ${({ theme }) => theme.gridUnit * 16}px;
+  ${({ theme }) => `
+  height: ${theme.gridUnit * 16}px;
  width: 100%;
-  border-top: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
-  border-bottom: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
-  color: ${({ theme }) => theme.colors.info.base};
-  border-top: ${({ theme }) => theme.gridUnit / 4}px solid
-    ${({ theme }) => theme.colors.grayscale.light2};
-  padding: ${({ theme }) => theme.gridUnit * 4}px;
+  border-top: 1px solid ${theme.colors.grayscale.light2};
+  border-bottom: 1px solid ${theme.colors.grayscale.light2};
+  color: ${theme.colors.info.base};
+  border-top: ${theme.gridUnit / 4}px solid
+    ${theme.colors.grayscale.light2};
+  padding: ${theme.gridUnit * 4}px;
  display: flex;
  justify-content: flex-end;
+  background-color: ${theme.colors.grayscale.light5};
+  z-index: ${theme.zIndex.max}
+  `}
 `;

 export const HeaderComponentStyles = styled.div`