From ff665fa5a7552d0e00df213adaf5f039f0ab6b23 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Tue, 20 Apr 2021 15:21:07 -0700 Subject: [PATCH] feat: restyle database modal (#14092) * restyle database modal * change name of tab to Basic * update test with RTL better RTL render statement * change color and position of required asterisk * refactor db logic --- .../CRUD/data/database/DatabaseModal_spec.jsx | 112 +-- .../CRUD/data/database/DatabaseModal.tsx | 803 ++++++++---------- .../src/views/CRUD/data/database/styles.ts | 203 +++++ 3 files changed, 626 insertions(+), 492 deletions(-) create mode 100644 superset-frontend/src/views/CRUD/data/database/styles.ts diff --git a/superset-frontend/spec/javascripts/views/CRUD/data/database/DatabaseModal_spec.jsx b/superset-frontend/spec/javascripts/views/CRUD/data/database/DatabaseModal_spec.jsx index 9e200a59f..89116a50b 100644 --- a/superset-frontend/spec/javascripts/views/CRUD/data/database/DatabaseModal_spec.jsx +++ b/superset-frontend/spec/javascripts/views/CRUD/data/database/DatabaseModal_spec.jsx @@ -23,7 +23,6 @@ import * as redux from 'react-redux'; import { styledMount as mount } from 'spec/helpers/theming'; import { render, screen } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; -import { supersetTheme, ThemeProvider } from '@superset-ui/core'; import { Provider } from 'react-redux'; import DatabaseModal from 'src/views/CRUD/data/database/DatabaseModal'; import Modal from 'src/components/Modal'; @@ -87,9 +86,9 @@ describe('DatabaseModal', () => { it('renders a Tabs menu', () => { expect(wrapper.find(Tabs)).toExist(); }); - it('renders five TabPanes', () => { - expect(wrapper.find(Tabs.TabPane)).toExist(); - expect(wrapper.find(Tabs.TabPane)).toHaveLength(5); + it('renders two TabPanes', () => { + expect(wrapper.find('.ant-tabs-tab')).toExist(); + expect(wrapper.find('.ant-tabs-tab')).toHaveLength(2); }); it('renders input elements for Connection section', () => { expect(wrapper.find('input[name="database_name"]')).toExist(); @@ -101,22 +100,24 @@ describe('DatabaseModal', () => { describe('initial load', () => { it('hides the forms from the db when not selected', () => { render( - - - - - , + , + { useRedux: true }, ); - // Select SQL Lab settings tab + // Select Advanced tab + const advancedTab = screen.getByRole('tab', { + name: /advanced/i, + }); + userEvent.click(advancedTab); + // Select SQL Lab tab const sqlLabSettingsTab = screen.getByRole('tab', { - name: /sql lab settings/i, + name: /sql lab/i, }); userEvent.click(sqlLabSettingsTab); @@ -129,21 +130,22 @@ describe('DatabaseModal', () => { }); }); it('renders all settings when "Expose in SQL Lab" is checked', () => { - render( - - - - - , - ); + render(, { useRedux: true }); - // Select SQL Lab settings tab + // Select Advanced tab + const advancedTab = screen.getByRole('tab', { + name: /advanced/i, + }); + userEvent.click(advancedTab); + + // Select SQL Lab tab const sqlLabSettingsTab = screen.getByRole('tab', { - name: /sql lab settings/i, + name: /sql lab/i, }); userEvent.click(sqlLabSettingsTab); - // Grab all SQL Lab Settings by their labels + + // Grab all SQL Lab settings by their labels // const exposeInSqlLab = screen.getByText('Expose in SQL Lab'); const exposeInSqlLab = screen.getByRole('checkbox', { name: /expose in sql lab/i, @@ -165,17 +167,17 @@ describe('DatabaseModal', () => { }); it('renders the schema field when allowCTAS is checked', () => { - render( - - - - - , - ); + render(, { useRedux: true }); - // Select SQL Lab settings tab + // Select Advanced tab + const advancedTab = screen.getByRole('tab', { + name: /advanced/i, + }); + userEvent.click(advancedTab); + + // Select SQL Lab tab const sqlLabSettingsTab = screen.getByRole('tab', { - name: /sql lab settings/i, + name: /sql lab/i, }); userEvent.click(sqlLabSettingsTab); // Grab CTAS & schema field by their labels @@ -195,17 +197,17 @@ describe('DatabaseModal', () => { }); it('renders the schema field when allowCVAS is checked', () => { - render( - - - - - , - ); + render(, { useRedux: true }); - // Select SQL Lab settings tab + // Select Advanced tab + const advancedTab = screen.getByRole('tab', { + name: /advanced/i, + }); + userEvent.click(advancedTab); + + // Select SQL Lab tab const sqlLabSettingsTab = screen.getByRole('tab', { - name: /sql lab settings/i, + name: /sql lab/i, }); userEvent.click(sqlLabSettingsTab); // Grab CVAS by it's label & schema field @@ -225,17 +227,17 @@ describe('DatabaseModal', () => { }); it('renders the schema field when both allowCTAS and allowCVAS are checked', () => { - render( - - - - - , - ); + render(, { useRedux: true }); - // Select SQL Lab settings tab + // Select Advanced tab + const advancedTab = screen.getByRole('tab', { + name: /advanced/i, + }); + userEvent.click(advancedTab); + + // Select SQL Lab tab const sqlLabSettingsTab = screen.getByRole('tab', { - name: /sql lab settings/i, + name: /sql lab/i, }); userEvent.click(sqlLabSettingsTab); // Grab CTAS and CVAS by their labels, & schema field diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseModal.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseModal.tsx index 105b77e53..693b87f5c 100644 --- a/superset-frontend/src/views/CRUD/data/database/DatabaseModal.tsx +++ b/superset-frontend/src/views/CRUD/data/database/DatabaseModal.tsx @@ -17,21 +17,27 @@ * under the License. */ import React, { FunctionComponent, useState, useEffect } from 'react'; -import { styled, t } from '@superset-ui/core'; +import cx from 'classnames'; import InfoTooltip from 'src/components/InfoTooltip'; +import { t, supersetTheme } from '@superset-ui/core'; import { useSingleViewResource, testDatabaseConnection, } from 'src/views/CRUD/hooks'; import withToasts from 'src/messageToasts/enhancers/withToasts'; -import Icon from 'src/components/Icon'; -import Modal from 'src/components/Modal'; import Tabs from 'src/common/components/Tabs'; import Button from 'src/components/Button'; import IndeterminateCheckbox from 'src/components/IndeterminateCheckbox'; -import { JsonEditor } from 'src/components/AsyncAceEditor'; +import Collapse from 'src/components/Collapse'; import { DatabaseObject } from './types'; import { useCommonConf } from './state'; +import { + StyledModal, + StyledInputContainer, + StyledJsonEditor, + StyledExpandableForm, + StyledRequiredTab, +} from './styles'; interface DatabaseModalProps { addDangerToast: (msg: string) => void; @@ -43,118 +49,6 @@ interface DatabaseModalProps { } const DEFAULT_TAB_KEY = '1'; -const EXPOSE_SQLLAB_FORM_HEIGHT = '270px'; -const CTAS_CVAS_SCHEMA_FORM_HEIGHT = '94px'; - -const StyledIcon = styled(Icon)` - margin: auto ${({ theme }) => theme.gridUnit * 2}px auto 0; -`; - -const StyledInputContainer = styled.div` - margin-bottom: ${({ theme }) => theme.gridUnit * 2}px; - - &.extra-container { - padding-top: 8px; - } - - &.expandable { - height: 0; - overflow: hidden; - transition: height 0.25s; - margin-left: ${({ theme }) => theme.gridUnit * 8}px; - padding: 0; - &.open { - height: ${CTAS_CVAS_SCHEMA_FORM_HEIGHT}; - } - } - - .helper { - display: block; - padding: ${({ theme }) => theme.gridUnit}px 0; - color: ${({ theme }) => theme.colors.grayscale.base}; - font-size: ${({ theme }) => theme.typography.sizes.s - 1}px; - text-align: left; - - .required { - margin-left: ${({ theme }) => theme.gridUnit / 2}px; - color: ${({ theme }) => theme.colors.error.base}; - } - } - - .input-container { - display: flex; - align-items: top; - - label { - display: flex; - margin-left: ${({ theme }) => theme.gridUnit * 2}px; - margin-top: ${({ theme }) => theme.gridUnit * 0.75}px; - font-family: ${({ theme }) => theme.typography.families.sansSerif}; - font-size: ${({ theme }) => theme.typography.sizes.m}px; - } - - i { - margin: 0 ${({ theme }) => theme.gridUnit}px; - } - } - - input, - textarea { - flex: 1 1 auto; - } - - textarea { - height: 160px; - resize: none; - } - - input::placeholder, - textarea::placeholder { - color: ${({ theme }) => theme.colors.grayscale.light1}; - } - - textarea, - input[type='text'], - input[type='number'] { - padding: ${({ theme }) => theme.gridUnit * 1.5}px - ${({ theme }) => theme.gridUnit * 2}px; - border-style: none; - border: 1px solid ${({ theme }) => theme.colors.grayscale.light2}; - border-radius: ${({ theme }) => theme.gridUnit}px; - - &[name='name'] { - flex: 0 1 auto; - width: 40%; - } - - &[name='sqlalchemy_uri'] { - margin-right: ${({ theme }) => theme.gridUnit * 3}px; - } - } -`; - -const StyledJsonEditor = styled(JsonEditor)` - flex: 1 1 auto; - border: 1px solid ${({ theme }) => theme.colors.grayscale.light2}; - border-radius: ${({ theme }) => theme.gridUnit}px; -`; - -const StyledExpandableForm = styled.div` - padding-top: ${({ theme }) => theme.gridUnit}px; - .input-container { - padding-top: ${({ theme }) => theme.gridUnit}px; - padding-bottom: ${({ theme }) => theme.gridUnit}px; - } - &.expandable { - height: 0; - overflow: hidden; - transition: height 0.25s; - margin-left: ${({ theme }) => theme.gridUnit * 7}px; - &.open { - height: ${EXPOSE_SQLLAB_FORM_HEIGHT}; - } - } -`; const DatabaseModal: FunctionComponent = ({ addDangerToast, @@ -196,14 +90,11 @@ const DatabaseModal: FunctionComponent = ({ const connection = { sqlalchemy_uri: db ? db.sqlalchemy_uri : '', - database_name: - db && db.database_name.trim().length - ? db.database_name.trim() - : undefined, - impersonate_user: db ? db.impersonate_user || undefined : undefined, - extra: db && db.extra && db.extra.length ? db.extra : undefined, - encrypted_extra: db ? db.encrypted_extra || undefined : undefined, - server_cert: db ? db.server_cert || undefined : undefined, + database_name: db?.database_name?.trim() || undefined, + impersonate_user: db?.impersonate_user || undefined, + extra: db?.extra || undefined, + encrypted_extra: db?.encrypted_extra || undefined, + server_cert: db?.server_cert || undefined, }; testDatabaseConnection(connection, addDangerToast, addSuccessToast); @@ -219,8 +110,8 @@ const DatabaseModal: FunctionComponent = ({ if (isEditMode) { // Edit const update: DatabaseObject = { - database_name: db ? db.database_name.trim() : '', - sqlalchemy_uri: db ? db.sqlalchemy_uri : '', + database_name: db?.database_name.trim() || '', + sqlalchemy_uri: db?.sqlalchemy_uri || '', ...db, }; @@ -296,12 +187,7 @@ const DatabaseModal: FunctionComponent = ({ }; const validate = () => { - if ( - db && - db.database_name.trim().length && - db.sqlalchemy_uri && - db.sqlalchemy_uri.length - ) { + if (db?.database_name?.trim() && db?.sqlalchemy_uri) { setDisableSave(false); } else { setDisableSave(true); @@ -356,39 +242,27 @@ const DatabaseModal: FunctionComponent = ({ const createAsOpen = !!(db?.allow_ctas || db?.allow_cvas); return ( - - - {isEditMode ? t('Edit database') : t('Add database')} - - } + title={

{isEditMode ? t('Edit database') : t('Add database')}

} > - - {t('Connection')} - * - - } - key="1" - > + {t('Basic')}} key="1">
- {t('Database name')} + {t('Display Name')} *
@@ -400,6 +274,9 @@ const DatabaseModal: FunctionComponent = ({ onChange={onInputChange} />
+
+ {t('Pick a name to help you identify this database.')} +
@@ -417,9 +294,6 @@ const DatabaseModal: FunctionComponent = ({ )} onChange={onInputChange} /> -
{t('Refer to the ')} @@ -433,300 +307,355 @@ const DatabaseModal: FunctionComponent = ({ {t(' for more information on how to structure your URI.')}
- - {t('Performance')}} key="2"> - -
{t('Chart cache timeout')}
-
- -
-
- {t( - 'Duration (in seconds) of the caching timeout for charts of this database.' + - ' A timeout of 0 indicates that the cache never expires.' + - ' Note this defaults to the global timeout if undefined.', - )} -
- - -
- - -
- - - {t('SQL Lab settings')}} key="3"> - - -
- - -
- - -
- - -
- - -
- - -
- -
- {t('CTAS & CVAS SCHEMA')} -
+ + + {t('Advanced')}} key="2"> + + +

SQL Lab

+

+ Configure how this database will function in SQL Lab. +

+ + } + key="1" + > + +
+ + +
+ +
- +
-
- {t( - 'When allowing CREATE TABLE AS option in SQL Lab, this option ' + - 'forces the table to be created in this schema.', - )} + + +
+ + +
+ +
+ {t('CTAS & CVAS SCHEMA')} +
+
+ +
+
+ {t( + 'When allowing CREATE TABLE AS option in SQL Lab, this option ' + + 'forces the table to be created in this schema.', + )} +
+ + + +
+ +
- - -
- - + +
+ + +
+ + + + + +

Performance

+

+ Adjust settings that will impact the performance of this + database. +

+
+ } + key="2" + > + +
{t('Chart cache timeout')}
+
+ +
+
+ {t( + 'Duration (in seconds) of the caching timeout for charts of this database.' + + ' A timeout of 0 indicates that the cache never expires.' + + ' Note this defaults to the global timeout if undefined.', + )} +
+ + +
+ + +
+ + + +

Security

+

+ Add connection information for other systems. +

+
+ } + key="3" + > + +
{t('Secure extra')}
+
+ + onEditorChange(json, 'encrypted_extra') + } + width="100%" + height="160px" + /> +
+
+
+ {t( + 'JSON string containing additional connection configuration.', + )}
- - -
- - +
+ {t( + 'This is used to provide connection information for systems like Hive, ' + + 'Presto, and BigQuery, which do not conform to the username:password syntax ' + + 'normally used by SQLAlchemy.', + )}
- - - - - - {t('Security')}} key="4"> - -
{t('Secure extra')}
-
- - onEditorChange(json, 'encrypted_extra') - } - width="100%" - height="160px" - /> -
-
-
- {t( - 'JSON string containing additional connection configuration.', - )} -
-
- {t( - 'This is used to provide connection information for systems like Hive, ' + - 'Presto, and BigQuery, which do not conform to the username:password syntax ' + - 'normally used by SQLAlchemy.', - )} -
-
- - -
{t('Root certificate')}
-
-