Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
691 changes: 331 additions & 360 deletions superset-frontend/package-lock.json

Large diffs are not rendered by default.

56 changes: 28 additions & 28 deletions superset-frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,34 +65,34 @@
"@babel/runtime-corejs3": "^7.12.5",
"@data-ui/sparkline": "^0.0.84",
"@emotion/core": "^10.0.35",
"@superset-ui/chart-controls": "^0.17.10",
"@superset-ui/core": "^0.17.10",
"@superset-ui/legacy-plugin-chart-calendar": "^0.17.10",
"@superset-ui/legacy-plugin-chart-chord": "^0.17.10",
"@superset-ui/legacy-plugin-chart-country-map": "^0.17.10",
"@superset-ui/legacy-plugin-chart-event-flow": "^0.17.10",
"@superset-ui/legacy-plugin-chart-force-directed": "^0.17.10",
"@superset-ui/legacy-plugin-chart-heatmap": "^0.17.10",
"@superset-ui/legacy-plugin-chart-histogram": "^0.17.10",
"@superset-ui/legacy-plugin-chart-horizon": "^0.17.10",
"@superset-ui/legacy-plugin-chart-map-box": "^0.17.10",
"@superset-ui/legacy-plugin-chart-paired-t-test": "^0.17.10",
"@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.17.10",
"@superset-ui/legacy-plugin-chart-partition": "^0.17.10",
"@superset-ui/legacy-plugin-chart-pivot-table": "^0.17.10",
"@superset-ui/legacy-plugin-chart-rose": "^0.17.10",
"@superset-ui/legacy-plugin-chart-sankey": "^0.17.10",
"@superset-ui/legacy-plugin-chart-sankey-loop": "^0.17.10",
"@superset-ui/legacy-plugin-chart-sunburst": "^0.17.10",
"@superset-ui/legacy-plugin-chart-treemap": "^0.17.10",
"@superset-ui/legacy-plugin-chart-world-map": "^0.17.10",
"@superset-ui/legacy-preset-chart-big-number": "^0.17.10",
"@superset-ui/legacy-preset-chart-deckgl": "^0.4.2",
"@superset-ui/legacy-preset-chart-nvd3": "^0.17.10",
"@superset-ui/plugin-chart-echarts": "^0.17.10",
"@superset-ui/plugin-chart-table": "^0.17.10",
"@superset-ui/plugin-chart-word-cloud": "^0.17.10",
"@superset-ui/preset-chart-xy": "^0.17.10",
"@superset-ui/chart-controls": "^0.17.12",
"@superset-ui/core": "^0.17.11",
"@superset-ui/legacy-plugin-chart-calendar": "^0.17.12",
"@superset-ui/legacy-plugin-chart-chord": "^0.17.12",
"@superset-ui/legacy-plugin-chart-country-map": "^0.17.12",
"@superset-ui/legacy-plugin-chart-event-flow": "^0.17.12",
"@superset-ui/legacy-plugin-chart-force-directed": "^0.17.12",
"@superset-ui/legacy-plugin-chart-heatmap": "^0.17.12",
"@superset-ui/legacy-plugin-chart-histogram": "^0.17.12",
"@superset-ui/legacy-plugin-chart-horizon": "^0.17.12",
"@superset-ui/legacy-plugin-chart-map-box": "^0.17.12",
"@superset-ui/legacy-plugin-chart-paired-t-test": "^0.17.12",
"@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.17.12",
"@superset-ui/legacy-plugin-chart-partition": "^0.17.12",
"@superset-ui/legacy-plugin-chart-pivot-table": "^0.17.12",
"@superset-ui/legacy-plugin-chart-rose": "^0.17.12",
"@superset-ui/legacy-plugin-chart-sankey": "^0.17.12",
"@superset-ui/legacy-plugin-chart-sankey-loop": "^0.17.12",
"@superset-ui/legacy-plugin-chart-sunburst": "^0.17.12",
"@superset-ui/legacy-plugin-chart-treemap": "^0.17.12",
"@superset-ui/legacy-plugin-chart-world-map": "^0.17.12",
"@superset-ui/legacy-preset-chart-big-number": "^0.17.12",
"@superset-ui/legacy-preset-chart-deckgl": "^0.4.6",
"@superset-ui/legacy-preset-chart-nvd3": "^0.17.12",
"@superset-ui/plugin-chart-echarts": "^0.17.12",
"@superset-ui/plugin-chart-table": "^0.17.12",
"@superset-ui/plugin-chart-word-cloud": "^0.17.12",
"@superset-ui/preset-chart-xy": "^0.17.12",
"@vx/responsive": "^0.0.195",
"abortcontroller-polyfill": "^1.1.9",
"antd": "^4.9.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
* under the License.
*/
import React from 'react';
import { HTML5Backend } from 'react-dnd-html5-backend';
import { DndProvider } from 'react-dnd';
import { render, screen } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import DatasourcePanel from 'src/explore/components/DatasourcePanel';
Expand Down Expand Up @@ -49,32 +51,38 @@ describe('datasourcepanel', () => {
actions: {},
};

const setup = props => (
<DndProvider backend={HTML5Backend}>
<DatasourcePanel {...props} />
</DndProvider>
);

function search(value, input) {
userEvent.clear(input);
userEvent.type(input, value);
}

it('should render', () => {
const { container } = render(<DatasourcePanel {...props} />);
const { container } = render(setup(props));
expect(container).toBeVisible();
});

it('should display items in controls', () => {
render(<DatasourcePanel {...props} />);
render(setup(props));
expect(screen.getByText('birth_names')).toBeTruthy();
expect(screen.getByText('Columns')).toBeTruthy();
expect(screen.getByText('Metrics')).toBeTruthy();
});

it('should render search results', () => {
const { container } = render(<DatasourcePanel {...props} />);
const { container } = render(setup(props));
const c = container.getElementsByClassName('option-label');

expect(c).toHaveLength(5);
});

it('should render 0 search results', () => {
const { container } = render(<DatasourcePanel {...props} />);
const { container } = render(setup(props));
const c = container.getElementsByClassName('option-label');
const searchInput = screen.getByPlaceholderText('Search Metrics & Columns');

Expand All @@ -85,7 +93,7 @@ describe('datasourcepanel', () => {
});

it('should render and sort search results', () => {
const { container } = render(<DatasourcePanel {...props} />);
const { container } = render(setup(props));
const c = container.getElementsByClassName('option-label');
const searchInput = screen.getByPlaceholderText('Search Metrics & Columns');

Expand Down
18 changes: 15 additions & 3 deletions superset-frontend/src/explore/components/DatasourcePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ import { debounce } from 'lodash';
import { matchSorter, rankings } from 'match-sorter';
import { FAST_DEBOUNCE } from 'src/constants';
import { ExploreActions } from 'src/explore/actions/exploreActions';
import Control from 'src/explore/components/Control';
import Control from './Control';
import DatasourcePanelDragWrapper from './DatasourcePanel/DatasourcePanelDragWrapper';
import { DatasourcePanelDndType } from './DatasourcePanel/types';

interface DatasourceControl extends ControlConfig {
datasource?: DatasourceMeta;
Expand Down Expand Up @@ -204,7 +206,12 @@ export default function DataSourcePanel({
</div>
{metricSlice.map(m => (
<LabelContainer key={m.metric_name} className="column">
<MetricOption metric={m} showType />
<DatasourcePanelDragWrapper
metricOrColumnName={m.metric_name}
type={DatasourcePanelDndType.METRIC}
>
<MetricOption metric={m} showType />
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about using a cursor: move instead of default here?

Copy link
Member Author

Choose a reason for hiding this comment

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

sorry, what is cursor: move on this?

Copy link
Member

Choose a reason for hiding this comment

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

I mean that here we could use cursor: move to indicate that the label is draggable
image

Copy link
Member Author

Choose a reason for hiding this comment

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

All right, I will add indicator style on this component and drop component.

Copy link
Member Author

Choose a reason for hiding this comment

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

The style pointer: move on the datasource panel looks strange. the UI behavior we can ask @mihir174

move.mp4

</DatasourcePanelDragWrapper>
</LabelContainer>
))}
</Collapse.Panel>
Expand All @@ -217,7 +224,12 @@ export default function DataSourcePanel({
</div>
{columnSlice.map(col => (
<LabelContainer key={col.column_name} className="column">
<ColumnOption column={col} showType />
<DatasourcePanelDragWrapper
metricOrColumnName={col.column_name}
type={DatasourcePanelDndType.COLUMN}
>
<ColumnOption column={col} showType />
</DatasourcePanelDragWrapper>
</LabelContainer>
))}
</Collapse.Panel>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import React, { ReactNode } from 'react';
import { useDrag } from 'react-dnd';
import { styled } from '@superset-ui/core';
import { DatasourcePanelDndItem } from './types';

const DatasourceItemContainer = styled.div`
display: flex;
align-items: center;
width: 100%;
height: ${({ theme }) => theme.gridUnit * 6}px;
cursor: pointer;

:hover {
background-color: ${({ theme }) => theme.colors.grayscale.light2};
}
`;

interface DatasourcePanelDragWrapperProps extends DatasourcePanelDndItem {
children?: ReactNode;
}

export default function DatasourcePanelDragWrapper(
props: DatasourcePanelDragWrapperProps,
) {
const [, drag] = useDrag({
item: {
metricOrColumnName: props.metricOrColumnName,
type: props.type,
},
});

return (
<DatasourceItemContainer ref={drag}>
{props.children}
</DatasourceItemContainer>
);
}
30 changes: 30 additions & 0 deletions superset-frontend/src/explore/components/DatasourcePanel/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
export enum DatasourcePanelDndType {
// todo: The new `metric` conflicts with the existing metric type
METRIC = 'datasource-panel-metric',
COLUMN = 'column',
}

export interface DatasourcePanelDndItem {
metricOrColumnName: string;
type:
| typeof DatasourcePanelDndType.METRIC
| typeof DatasourcePanelDndType.COLUMN;
Comment on lines +27 to +29
Copy link
Member

Choose a reason for hiding this comment

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

This should also work:

Suggested change
type:
| typeof DatasourcePanelDndType.METRIC
| typeof DatasourcePanelDndType.COLUMN;
type: DatasourcePanelDndType;

}
39 changes: 30 additions & 9 deletions superset-frontend/src/explore/components/OptionControls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ import { Tooltip } from 'src/common/components/Tooltip';
import Icon from 'src/components/Icon';
import { savedMetricType } from 'src/explore/components/controls/MetricControl/types';

const DragContainer = styled.div`
export const DragContainer = styled.div`
margin-bottom: ${({ theme }) => theme.gridUnit}px;
:last-child {
margin-bottom: 0;
}
`;

const OptionControlContainer = styled.div<{
export const OptionControlContainer = styled.div<{
isAdhoc?: boolean;
}>`
display: flex;
Expand All @@ -47,7 +47,7 @@ const OptionControlContainer = styled.div<{
cursor: ${({ isAdhoc }) => (isAdhoc ? 'pointer' : 'default')};
`;

const Label = styled.div`
export const Label = styled.div`
display: flex;
max-width: 100%;
overflow: hidden;
Expand All @@ -63,13 +63,13 @@ const Label = styled.div`
}
`;

const CaretContainer = styled.div`
export const CaretContainer = styled.div`
height: 100%;
border-left: solid 1px ${({ theme }) => theme.colors.grayscale.dark2}0C;
margin-left: auto;
`;

const CloseContainer = styled.div`
export const CloseContainer = styled.div`
height: 100%;
width: ${({ theme }) => theme.gridUnit * 6}px;
border-right: solid 1px ${({ theme }) => theme.colors.grayscale.dark2}0C;
Expand All @@ -92,7 +92,26 @@ export const LabelsContainer = styled.div`
border-radius: ${({ theme }) => theme.gridUnit}px;
`;

export const AddControlLabel = styled.div`
export const DndLabelsContainer = styled.div<{
canDrop?: boolean;
isOver?: boolean;
}>`
padding: ${({ theme }) => theme.gridUnit}px;
border: ${({ canDrop, isOver, theme }) => {
if (isOver && canDrop) {
return `dashed 1px ${theme.colors.info.dark1}`;
}
if (isOver && !canDrop) {
return `dashed 1px ${theme.colors.error.dark1}`;
}
Comment on lines +102 to +106
Copy link
Member

Choose a reason for hiding this comment

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

It feels like dashed 1px doesn't stick out quite enough. IMO 2px looked better, but then the default 1 px causes this to resize when hovering in/out, so that doesn't work quite so well without addditional tuning of the default borders.

Copy link
Member

Choose a reason for hiding this comment

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

I think that we should also add an indicator for canDrop && !isOver so that user knows where he can drag a column

Copy link
Member

Choose a reason for hiding this comment

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

I think that we should also add an indicator for canDrop && !isOver so that user knows where he can drag a column

Yeah I like that idea 👍

return `solid 1px ${theme.colors.grayscale.light2}`;
}};
border-radius: ${({ theme }) => theme.gridUnit}px;
`;

export const AddControlLabel = styled.div<{
cancelHover?: boolean;
}>`
display: flex;
align-items: center;
width: 100%;
Expand All @@ -102,14 +121,16 @@ export const AddControlLabel = styled.div`
color: ${({ theme }) => theme.colors.grayscale.light1};
border: dashed 1px ${({ theme }) => theme.colors.grayscale.light2};
border-radius: ${({ theme }) => theme.gridUnit}px;
cursor: pointer;
cursor: ${({ cancelHover }) => (cancelHover ? 'inherit' : 'pointer')};

:hover {
background-color: ${({ theme }) => theme.colors.grayscale.light4};
background-color: ${({ cancelHover, theme }) =>
cancelHover ? 'inherit' : theme.colors.grayscale.light4};
}

:active {
background-color: ${({ theme }) => theme.colors.grayscale.light3};
background-color: ${({ cancelHover, theme }) =>
cancelHover ? 'inherit' : theme.colors.grayscale.light3};
}
`;

Expand Down
Loading