Skip to content

Commit

Permalink
fix(dashboard): drag and drop indicator UX (apache#26699)
Browse files Browse the repository at this point in the history 
(cherry picked from commit 8e2b534)
  • Loading branch information
@justinpark @michael-s-molina
justinpark authored and michael-s-molina committed Feb 23, 2024
1 parent 5b928a4 commit 37fd859
Show file tree
Hide file tree
Showing 26 changed files with 585 additions and 335 deletions.
53 changes: 31 additions & 22 deletions superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import BuilderComponentPane from 'src/dashboard/components/BuilderComponentPane'
import DashboardHeader from 'src/dashboard/containers/DashboardHeader';
import Icons from 'src/components/Icons';
import IconButton from 'src/dashboard/components/IconButton';
import DragDroppable from 'src/dashboard/components/dnd/DragDroppable';
import { Droppable } from 'src/dashboard/components/dnd/DragDroppable';
import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
import WithPopoverMenu from 'src/dashboard/components/menu/WithPopoverMenu';
import getDirectPathToTabIndex from 'src/dashboard/util/getDirectPathToTabIndex';
Expand Down Expand Up @@ -81,6 +81,7 @@ import {
MAIN_HEADER_HEIGHT,
OPEN_FILTER_BAR_MAX_WIDTH,
OPEN_FILTER_BAR_WIDTH,
EMPTY_CONTAINER_Z_INDEX,
} from 'src/dashboard/constants';
import { getRootLevelTabsComponent, shouldFocusTabs } from './utils';
import DashboardContainer from './DashboardContainer';
Expand All @@ -107,12 +108,27 @@ const StickyPanel = styled.div<{ width: number }>`

// @z-index-above-dashboard-popovers (99) + 1 = 100
const StyledHeader = styled.div`
grid-column: 2;
grid-row: 1;
position: sticky;
top: 0;
z-index: 100;
max-width: 100vw;
${({ theme }) => css`
grid-column: 2;
grid-row: 1;
position: sticky;
top: 0;
z-index: 100;
max-width: 100vw;
.empty-droptarget:before {
position: absolute;
content: '';
display: none;
width: calc(100% - ${theme.gridUnit * 2}px);
height: calc(100% - ${theme.gridUnit * 2}px);
left: ${theme.gridUnit}px;
top: ${theme.gridUnit}px;
border: 1px dashed transparent;
border-radius: ${theme.gridUnit}px;
opacity: 0.5;
}
`}
`;

const StyledContent = styled.div<{
Expand Down Expand Up @@ -211,13 +227,9 @@ const DashboardContentWrapper = styled.div`
/* provide hit area in case row contents is edge to edge */
.dashboard-component-tabs-content {
.dragdroppable-row {
> .dragdroppable-row {
padding-top: ${theme.gridUnit * 4}px;
}
& > div:not(:last-child):not(.empty-droptarget) {
margin-bottom: ${theme.gridUnit * 4}px;
}
}
.dashboard-component-chart-holder {
Expand Down Expand Up @@ -250,25 +262,21 @@ const DashboardContentWrapper = styled.div`
}
& > .empty-droptarget {
z-index: ${EMPTY_CONTAINER_Z_INDEX};
position: absolute;
width: 100%;
}
& > .empty-droptarget:first-child:not(.empty-droptarget--full) {
height: ${theme.gridUnit * 4}px;
top: -2px;
z-index: 10;
top: 0;
}
& > .empty-droptarget:last-child {
height: ${theme.gridUnit * 3}px;
bottom: 0;
height: ${theme.gridUnit * 4}px;
bottom: ${-theme.gridUnit * 4}px;
}
}
.empty-droptarget:first-child .drop-indicator--bottom {
top: ${theme.gridUnit * 6}px;
}
`}
`;

Expand Down Expand Up @@ -616,8 +624,9 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
)}
<StyledHeader ref={headerRef}>
{/* @ts-ignore */}
<DragDroppable
<Droppable
data-test="top-level-tabs"
className={cx(!topLevelTabs && editMode && 'empty-droptarget')}
component={dashboardRoot}
parentComponent={null}
depth={DASHBOARD_ROOT_DEPTH}
Expand All @@ -630,7 +639,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
style={draggableStyle}
>
{renderDraggableContent}
</DragDroppable>
</Droppable>
</StyledHeader>
<StyledContent fullSizeChartId={fullSizeChartId}>
<Global
Expand Down
18 changes: 16 additions & 2 deletions superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,19 @@
* under the License.
*/
import React from 'react';
import { fireEvent, render } from 'spec/helpers/testing-library';
import { fireEvent, render, waitFor } from 'spec/helpers/testing-library';
import { OptionControlLabel } from 'src/explore/components/controls/OptionControls';

import DashboardWrapper from './DashboardWrapper';

beforeAll(() => {
jest.useFakeTimers();
});

afterAll(() => {
jest.useRealTimers();
});

test('should render children', () => {
const { getByTestId } = render(
<DashboardWrapper>
Expand All @@ -32,7 +40,7 @@ test('should render children', () => {
expect(getByTestId('mock-children')).toBeInTheDocument();
});

test('should update the style on dragging state', () => {
test('should update the style on dragging state', async () => {
const defaultProps = {
label: <span>Test label</span>,
tooltipTitle: 'This is a tooltip title',
Expand Down Expand Up @@ -69,7 +77,13 @@ test('should update the style on dragging state', () => {
container.getElementsByClassName('dragdroppable--dragging'),
).toHaveLength(0);
fireEvent.dragStart(getByText('Label 1'));
await waitFor(() => jest.runAllTimers());
expect(
container.getElementsByClassName('dragdroppable--dragging'),
).toHaveLength(1);
fireEvent.dragEnd(getByText('Label 1'));
// immediately discards dragging state after dragEnd
expect(
container.getElementsByClassName('dragdroppable--dragging'),
).toHaveLength(0);
});
33 changes: 27 additions & 6 deletions superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
* under the License.
*/
import React, { useEffect } from 'react';
import { css, styled } from '@superset-ui/core';
import { FAST_DEBOUNCE, css, styled } from '@superset-ui/core';
import { RootState } from 'src/dashboard/types';
import { useSelector } from 'react-redux';
import { useDragDropManager } from 'react-dnd';
import classNames from 'classnames';
import { debounce } from 'lodash';

const StyledDiv = styled.div`
${({ theme }) => css`
Expand All @@ -32,10 +33,20 @@ const StyledDiv = styled.div`
flex: 1;
/* Special cases */
&.dragdroppable--dragging
.dashboard-component-tabs-content
> .empty-droptarget.empty-droptarget--full {
height: 100%;
&.dragdroppable--dragging {
&
.dashboard-component-tabs-content
> .empty-droptarget.empty-droptarget--full {
height: 100%;
}
& .empty-droptarget:before {
display: block;
border-color: ${theme.colors.primary.light1};
background-color: ${theme.colors.primary.light3};
}
& .grid-row:after {
border-style: hidden;
}
}
/* A row within a column has inset hover menu */
Expand Down Expand Up @@ -106,12 +117,22 @@ const DashboardWrapper: React.FC<Props> = ({ children }) => {

useEffect(() => {
const monitor = dragDropManager.getMonitor();
const debouncedSetIsDragged = debounce(setIsDragged, FAST_DEBOUNCE);
const unsub = monitor.subscribeToStateChange(() => {
setIsDragged(monitor.isDragging());
const isDragging = monitor.isDragging();
if (isDragging) {
// set a debounced function to prevent HTML5 drag source
// from interfering with the drop zone highlighting
debouncedSetIsDragged(true);
} else {
debouncedSetIsDragged.cancel();
setIsDragged(false);
}
});

return () => {
unsub();
debouncedSetIsDragged.cancel();
};
}, [dragDropManager]);

Expand Down
115 changes: 64 additions & 51 deletions superset-frontend/src/dashboard/components/DashboardGrid.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { addAlpha, css, styled, t } from '@superset-ui/core';
import { EmptyStateBig } from 'src/components/EmptyState';
import { componentShape } from '../util/propShapes';
import DashboardComponent from '../containers/DashboardComponent';
import DragDroppable from './dnd/DragDroppable';
import { Droppable } from './dnd/DragDroppable';
import { GRID_GUTTER_SIZE, GRID_COLUMN_COUNT } from '../util/constants';
import { TAB_TYPE } from '../util/componentTypes';

Expand All @@ -41,15 +41,8 @@ const propTypes = {

const defaultProps = {};

const renderDraggableContentBottom = dropProps =>
dropProps.dropIndicatorProps && (
<div className="drop-indicator drop-indicator--bottom" />
);

const renderDraggableContentTop = dropProps =>
dropProps.dropIndicatorProps && (
<div className="drop-indicator drop-indicator--top" />
);
const renderDraggableContent = dropProps =>
dropProps.dropIndicatorProps && <div {...dropProps.dropIndicatorProps} />;

const DashboardEmptyStateContainer = styled.div`
position: absolute;
Expand All @@ -60,28 +53,42 @@ const DashboardEmptyStateContainer = styled.div`
`;

const GridContent = styled.div`
${({ theme }) => css`
${({ theme, editMode }) => css`
display: flex;
flex-direction: column;
/* gutters between rows */
& > div:not(:last-child):not(.empty-droptarget) {
margin-bottom: ${theme.gridUnit * 4}px;
${!editMode && `margin-bottom: ${theme.gridUnit * 4}px`};
}
& > .empty-droptarget {
.empty-droptarget {
width: 100%;
height: 100%;
height: ${theme.gridUnit * 4}px;
display: flex;
align-items: center;
justify-content: center;
border-radius: ${theme.gridUnit}px;
overflow: hidden;
&:before {
content: '';
display: block;
width: calc(100% - ${theme.gridUnit * 2}px);
height: calc(100% - ${theme.gridUnit * 2}px);
border: 1px dashed transparent;
border-radius: ${theme.gridUnit}px;
opacity: 0.5;
}
}
& > .empty-droptarget:first-child {
height: ${theme.gridUnit * 12}px;
margin-top: ${theme.gridUnit * -6}px;
height: ${theme.gridUnit * 4}px;
margin-top: ${theme.gridUnit * -4}px;
}
& > .empty-droptarget:last-child {
height: ${theme.gridUnit * 12}px;
margin-top: ${theme.gridUnit * -6}px;
height: ${theme.gridUnit * 24}px;
}
& > .empty-droptarget.empty-droptarget--full:only-child {
Expand Down Expand Up @@ -265,10 +272,14 @@ class DashboardGrid extends React.PureComponent {
</DashboardEmptyStateContainer>
)}
<div className="dashboard-grid" ref={this.setGridRef}>
<GridContent className="grid-content" data-test="grid-content">
<GridContent
className="grid-content"
data-test="grid-content"
editMode={editMode}
>
{/* make the area above components droppable */}
{editMode && (
<DragDroppable
<Droppable
component={gridComponent}
depth={depth}
parentComponent={null}
Expand All @@ -281,41 +292,43 @@ class DashboardGrid extends React.PureComponent {
gridComponent?.children?.length === 0,
})}
editMode
dropToChild={gridComponent?.children?.length === 0}
>
{renderDraggableContentTop}
</DragDroppable>
{renderDraggableContent}
</Droppable>
)}
{gridComponent?.children?.map((id, index) => (
<DashboardComponent
key={id}
id={id}
parentId={gridComponent.id}
depth={depth + 1}
index={index}
availableColumnCount={GRID_COLUMN_COUNT}
columnWidth={columnWidth}
isComponentVisible={isComponentVisible}
onResizeStart={this.handleResizeStart}
onResize={this.handleResize}
onResizeStop={this.handleResizeStop}
onChangeTab={this.handleChangeTab}
/>
<React.Fragment key={id}>
<DashboardComponent
id={id}
parentId={gridComponent.id}
depth={depth + 1}
index={index}
availableColumnCount={GRID_COLUMN_COUNT}
columnWidth={columnWidth}
isComponentVisible={isComponentVisible}
onResizeStart={this.handleResizeStart}
onResize={this.handleResize}
onResizeStop={this.handleResizeStop}
onChangeTab={this.handleChangeTab}
/>
{/* make the area below components droppable */}
{editMode && (
<Droppable
component={gridComponent}
depth={depth}
parentComponent={null}
index={index + 1}
orientation="column"
onDrop={handleComponentDrop}
className="empty-droptarget"
editMode
>
{renderDraggableContent}
</Droppable>
)}
</React.Fragment>
))}
{/* make the area below components droppable */}
{editMode && gridComponent?.children?.length > 0 && (
<DragDroppable
component={gridComponent}
depth={depth}
parentComponent={null}
index={gridComponent.children.length}
orientation="column"
onDrop={handleComponentDrop}
className="empty-droptarget"
editMode
>
{renderDraggableContentBottom}
</DragDroppable>
)}
{isResizing &&
Array(GRID_COLUMN_COUNT)
.fill(null)
Expand Down
Loading

0 comments on commit 37fd859

Please sign in to comment.