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.

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

fix(dashboard): drag and drop indicator UX #26699

Merged
merged 3 commits into from
Feb 21, 2024
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
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 @@ -613,8 +621,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 @@ -627,7 +636,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
style={draggableStyle}
>
{renderDraggableContent}
</DragDroppable>
</Droppable>
</StyledHeader>
<StyledContent fullSizeChartId={fullSizeChartId}>
<Global
Expand Down
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);
});
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 @@ -31,10 +32,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 @@ -105,12 +116,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
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
Loading