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.

Sign up for GitHub

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

chore(explore): Visual updates to explore datasource panel #20317

Closed
wants to merge 5 commits into from
Closed

chore(explore): Visual updates to explore datasource panel #20317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bd3b6a9
Visual updates to explore datasource panel
lyndsiWilliams Jun 8, 2022
64f4181
Temporarily make Query icon visible
lyndsiWilliams Jun 8, 2022
7c07534
Remove Query icon visibility
lyndsiWilliams Jun 8, 2022
6fab57f
Removed isValidDatasourceType check
lyndsiWilliams Jun 8, 2022
31dfdd2
Adjusts conditional logic approach to be extensible for additional ty…
eric-briscoe Jun 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions superset-frontend/src/explore/components/DatasourcePanel/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,7 @@ export default function DataSourcePanel({
return true;
};

const isValidDatasourceType =
datasource.type === DatasourceType.Dataset ||
datasource.type === DatasourceType.SlTable ||
datasource.type === DatasourceType.SavedQuery ||
datasource.type === DatasourceType.Query;
const dataSourceIsQuery = datasource?.type === DatasourceType.Query;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we duck type this instead?


const mainBody = useMemo(
() => (
Expand All @@ -327,7 +323,7 @@ export default function DataSourcePanel({
placeholder={t('Search Metrics & Columns')}
/>
<div className="field-selections">
{isValidDatasourceType && showInfoboxCheck() && (
{dataSourceIsQuery && showInfoboxCheck() && (
<StyledInfoboxWrapper>
<Alert
closable
Expand Down Expand Up @@ -440,19 +436,22 @@ export default function DataSourcePanel({
search,
showAllColumns,
showAllMetrics,
dataSourceIsQuery,
shouldForceUpdate,
],
);

return (
<DatasourceContainer>
<SaveDatasetModal
visible={showSaveDatasetModal}
onHide={() => setShowSaveDatasetModal(false)}
buttonTextOnSave={t('Save')}
buttonTextOnOverwrite={t('Overwrite')}
datasource={datasource}
/>
{dataSourceIsQuery && (
<SaveDatasetModal
visible={showSaveDatasetModal}
onHide={() => setShowSaveDatasetModal(false)}
buttonTextOnSave={t('Save')}
buttonTextOnOverwrite={t('Overwrite')}
datasource={datasource}
/>
)}
<Control {...datasourceControl} name="datasource" actions={actions} />
{datasource.id != null && mainBody}
</DatasourceContainer>
Expand Down
13 changes: 4 additions & 9 deletions superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ const ExplorePanelContainer = styled.div`
position: relative;
display: flex;
flex-direction: row;
padding: 0 ${theme.gridUnit * 4}px;
padding: 0 ${theme.gridUnit * 2}px 0 ${theme.gridUnit * 4}px;
justify-content: space-between;
.horizontal-text {
font-size: ${theme.typography.sizes.s}px;
font-size: ${theme.typography.sizes.m}px;
}
}
.no-show {
Expand All @@ -145,7 +145,7 @@ const ExplorePanelContainer = styled.div`
padding: ${theme.gridUnit * 2}px;
width: ${theme.gridUnit * 8}px;
}
.callpase-icon > svg {
.collapse-icon > svg {
color: ${theme.colors.primary.base};
}
`};
Expand Down Expand Up @@ -603,7 +603,7 @@ function ExploreViewContainer(props) {
}
>
<div className="title-container">
<span className="horizontal-text">{t('Dataset')}</span>
<span className="horizontal-text">{t('Chart Source')}</span>
<span
role="button"
tabIndex={0}
Expand Down Expand Up @@ -642,11 +642,6 @@ function ExploreViewContainer(props) {
/>
</Tooltip>
</span>
<Icons.DatasetPhysical
css={{ marginTop: theme.gridUnit * 2 }}
iconSize="l"
iconColor={theme.colors.grayscale.base}
/>
</div>
) : null}
<Resizable
Expand Down
30 changes: 28 additions & 2 deletions ...set-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ import React from 'react';
import sinon from 'sinon';
import configureStore from 'redux-mock-store';
import { mount, shallow } from 'enzyme';
import { supersetTheme, ThemeProvider } from '@superset-ui/core';
import { supersetTheme, ThemeProvider, DatasourceType } from '@superset-ui/core';
import { Menu } from 'src/components/Menu';
import {
DatasourceModal,
ChangeDatasourceModal,
} from 'src/components/Datasource';
import DatasourceControl from 'src/explore/components/controls/DatasourceControl';
import DatasourceControl, { getDatasourceTitle } from 'src/explore/components/controls/DatasourceControl';
import Icons from 'src/components/Icons';
import { Tooltip } from 'src/components/Tooltip';

Expand Down Expand Up @@ -143,4 +143,30 @@ describe('DatasourceControl', () => {
defaultProps.datasource.health_check_message,
);
});

it('Gets Datasource Title', () => {
const sql = 'This is the sql';
const name = 'this is a name';
const emptyResult = '';
const queryDatasource1 = {type: DatasourceType.Query, sql: sql};
let displayText = getDatasourceTitle(queryDatasource1);
expect(displayText).toBe(sql);
const queryDatasource2 = {type: DatasourceType.Query, sql: null};
displayText = getDatasourceTitle(queryDatasource2);
expect(displayText).toBe(emptyResult);
const queryDatasource3 = {type: 'random type', name: name};
displayText = getDatasourceTitle(queryDatasource3);
expect(displayText).toBe(name);
const queryDatasource4 = {type: 'random type'};
displayText = getDatasourceTitle(queryDatasource4);
expect(displayText).toBe(emptyResult);
displayText = getDatasourceTitle();
expect(displayText).toBe(emptyResult);
displayText = getDatasourceTitle(null);
expect(displayText).toBe(emptyResult);
displayText = getDatasourceTitle('I should not be a string');
expect(displayText).toBe(emptyResult);
displayText = getDatasourceTitle([]);
expect(displayText).toBe(emptyResult);
})
});
72 changes: 52 additions & 20 deletions superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
*/
import React from 'react';
import PropTypes from 'prop-types';
import { t, styled, withTheme } from '@superset-ui/core';
import { t, styled, withTheme, DatasourceType } from '@superset-ui/core';
import { getUrlParam } from 'src/utils/urlUtils';

import { AntdDropdown } from 'src/components';
Expand Down Expand Up @@ -97,7 +97,7 @@ const Styles = styled.div`
white-space: nowrap;
overflow: hidden;
}
.dataset-svg {
.datasource-svg {
margin-right: ${({ theme }) => 2 * theme.gridUnit}px;
flex: none;
}
Expand All @@ -113,6 +113,47 @@ const CHANGE_DATASET = 'change_dataset';
const VIEW_IN_SQL_LAB = 'view_in_sql_lab';
const EDIT_DATASET = 'edit_dataset';

// If the string is longer than this value's number characters we add
// a tooltip for user can see the full name by hovering over the visually truncated string in UI
const VISIBLE_TITLE_LENGTH = 25;


// Assign icon for each DatasourceType. If no icon assingment is found in the lookup, no icon will render
export const datasourceIconLookup = {
[DatasourceType.Query]: <Icons.ConsoleSqlOutlined className="datasource-svg" />,
[DatasourceType.Table]: <Icons.DatasetPhysical className="datasource-svg" />,
}

// Render title for datasource with tooltip only if text is longer than VISIBLE_TITLE_LENGTH
export const renderDatasourceTitle = displayString =>
displayString.length > VISIBLE_TITLE_LENGTH ? (
// Add a tooltip only for long names that will be visually truncated
<Tooltip title={displayString}>
<span className="title-select">{displayString}</span>
</Tooltip>
) : (
<span title={displayString} className="title-select">
{displayString}
</span>
);

// Different data source types use different attributes for the display title
export const getDatasourceTitle = datasource => {
let text = '';
const dataSourceType = datasource?.type;
if (dataSourceType) {
switch (dataSourceType) {
case DatasourceType.Query:
text = datasource?.sql ?? '';
break;
default:
text = datasource?.name ?? '';
break;
}
}
return text;
};

class DatasourceControl extends React.PureComponent {
constructor(props) {
super(props);
Expand Down Expand Up @@ -186,7 +227,7 @@ class DatasourceControl extends React.PureComponent {
render() {
const { showChangeDatasourceModal, showEditDatasourceModal } = this.state;
const { datasource, onChange, theme } = this.props;
const isMissingDatasource = datasource.id == null;
const isMissingDatasource = datasource?.id?.length > 0 ? false : true;
let isMissingParams = false;
if (isMissingDatasource) {
const datasourceId = getUrlParam(URL_PARAMS.datasourceId);
Expand Down Expand Up @@ -241,20 +282,13 @@ class DatasourceControl extends React.PureComponent {
} catch {} // eslint-disable-line no-empty
}

const titleText = getDatasourceTitle(datasource) ;

return (
<Styles data-test="datasource-control" className="DatasourceControl">
<div className="data-container">
<Icons.DatasetPhysical className="dataset-svg" />
{/* Add a tooltip only for long dataset names */}
{!isMissingDatasource && datasource.name.length > 25 ? (
<Tooltip title={datasource.name}>
<span className="title-select">{datasource.name}</span>
</Tooltip>
) : (
<span title={datasource.name} className="title-select">
{datasource.name}
</span>
)}
{datasourceIconLookup[datasource?.type]}
{renderDatasourceTitle(titleText)}
{healthCheckMessage && (
<Tooltip title={healthCheckMessage}>
<Icons.AlertSolid iconColor={theme.colors.warning.base} />
Expand All @@ -268,12 +302,10 @@ class DatasourceControl extends React.PureComponent {
trigger={['click']}
data-test="datasource-menu"
>
<Tooltip title={t('More dataset related options')}>
<Icons.MoreVert
className="datasource-modal-trigger"
data-test="datasource-menu-trigger"
/>
</Tooltip>
<Icons.MoreVert
className="datasource-modal-trigger"
data-test="datasource-menu-trigger"
/>
</AntdDropdown>
</div>
{/* missing dataset */}
Expand Down