From 6b20e7444205498077014c0382dcccb485c49bf2 Mon Sep 17 00:00:00 2001 From: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com> Date: Tue, 20 Dec 2022 10:05:40 -0600 Subject: [PATCH] fix: Create dataset polish/bug fix (#22262) --- .../components/SqlEditorLeftBar/index.tsx | 21 +----- .../src/components/EmptyState/index.tsx | 26 ++++++- .../dataset/AddDataset/AddDataset.test.tsx | 2 +- .../DatasetPanel/DatasetPanel.test.tsx | 2 +- .../AddDataset/DatasetPanel/DatasetPanel.tsx | 67 +++++++++++++------ .../dataset/AddDataset/Header/Header.test.tsx | 16 +---- .../data/dataset/AddDataset/Header/index.tsx | 35 ++++++---- .../dataset/AddDataset/LeftPanel/index.tsx | 38 +++++++++-- .../DatasetLayout/DatasetLayout.test.tsx | 2 +- .../src/views/CRUD/data/dataset/styles.ts | 46 +++++++++---- 10 files changed, 166 insertions(+), 89 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx index 4b73cffff986..cc62022a123d 100644 --- 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 = ( - - {t('Manage your databases')}{' '} - {t('here')} -

- } - /> - ); - const handleSchemaChange = useCallback((schema: string) => { if (queryEditor) { dispatch(queryEditorSetSchema(queryEditor, schema)); @@ -248,7 +231,7 @@ const SqlEditorLeftBar = ({
); + +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) => ( + + {TRANSLATIONS.MANAGE_YOUR_DATABASES_TEXT}{' '} + {TRANSLATIONS.HERE_TEXT} +

+ } + /> +); 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 index 25e7e9cd289a..39fb2b295b30 100644 --- 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 index 4b066eb30b29..b5a29638c61b 100644 --- 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 index b8d0a469411f..8d579b79b8b9 100644 --- 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` - ${({ 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 = [ @@ -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 = ( <> {COLUMN_TITLE} - - - + {tableWithDataset ? ( + + + + + + ) : ( + + +
+ + + )} ); } else { @@ -299,9 +330,7 @@ const DatasetPanel = ({ {tableName && ( <> {datasetNames?.includes(tableName) && - renderExistingDatasetAlert( - datasets?.find(dataset => dataset.table_name === tableName), - )} + renderExistingDatasetAlert(tableWithDataset)} { 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'); }); 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 index 7c1e4f51e6d2..044221c3fe47 100644 --- 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; 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 ( - {} }} - titlePanelAdditionalItems={<>} - rightPanelAdditionalItems={renderDisabledSaveButton()} - additionalActionsMenu={renderOverlay()} - menuDropdownProps={{ - disabled: true, - }} - tooltipProps={tooltipProps} - /> + {editing ? ( + {} }} + titlePanelAdditionalItems={<>} + rightPanelAdditionalItems={renderDisabledSaveButton()} + additionalActionsMenu={renderOverlay()} + menuDropdownProps={{ + disabled: true, + }} + tooltipProps={tooltipProps} + /> + ) : ( + + {title || DEFAULT_TITLE} + + )} ); } diff --git a/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx b/superset-frontend/src/views/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx index 05ec47b4c0c7..4f7dfca196f4 100644 --- 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 (

@@ -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) => (

)} diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/DatasetLayout.test.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetLayout/DatasetLayout.test.tsx index 78ac80fb508e..35375a52b4fb 100644 --- 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 () => { diff --git a/superset-frontend/src/views/CRUD/data/dataset/styles.ts b/superset-frontend/src/views/CRUD/data/dataset/styles.ts index 3bc60b72e9cf..45082dea7d26 100644 --- 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`