From 09ed1f2dd1722dbc29f7de21922e40cc690af900 Mon Sep 17 00:00:00 2001 From: justinpark Date: Mon, 18 Mar 2024 15:57:25 -0700 Subject: [PATCH 1/9] fix(explore): drag and drop indicator UX --- .../ExploreContainer.test.tsx | 79 +++++++++++++++++++ .../components/ExploreContainer/index.tsx | 55 +++++++++++++ .../components/ExploreViewContainer/index.jsx | 8 +- .../DndColumnSelectControl/DndSelectLabel.tsx | 5 +- .../controls/OptionControls/index.tsx | 40 ++++++++-- 5 files changed, 173 insertions(+), 14 deletions(-) create mode 100644 superset-frontend/src/explore/components/ExploreContainer/ExploreContainer.test.tsx create mode 100644 superset-frontend/src/explore/components/ExploreContainer/index.tsx diff --git a/superset-frontend/src/explore/components/ExploreContainer/ExploreContainer.test.tsx b/superset-frontend/src/explore/components/ExploreContainer/ExploreContainer.test.tsx new file mode 100644 index 000000000000..61ee2027fa04 --- /dev/null +++ b/superset-frontend/src/explore/components/ExploreContainer/ExploreContainer.test.tsx @@ -0,0 +1,79 @@ +/** + * 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 from 'react'; +import { fireEvent, render } from 'spec/helpers/testing-library'; +import { OptionControlLabel } from 'src/explore/components/controls/OptionControls'; + +import ExploreContainer, { DraggingContext } from '.'; + +const MockChildren = () => { + const dragging = React.useContext(DraggingContext); + return ( +
+ {dragging ? 'dragging' : 'not dragging'} +
+ ); +}; + +test('should render children', () => { + const { getByTestId, getByText } = render( + + + , + { useRedux: true, useDnd: true }, + ); + expect(getByTestId('mock-children')).toBeInTheDocument(); + expect(getByText('not dragging')).toBeInTheDocument(); +}); + +test('should update the style on dragging state', () => { + const defaultProps = { + label: Test label, + tooltipTitle: 'This is a tooltip title', + onRemove: jest.fn(), + onMoveLabel: jest.fn(), + onDropLabel: jest.fn(), + type: 'test', + index: 0, + }; + const { container, getByText } = render( + + Label 1} + /> + Label 2} + /> + + , + { + useRedux: true, + useDnd: true, + }, + ); + expect(container.getElementsByClassName('dragging')).toHaveLength(0); + fireEvent.dragStart(getByText('Label 1')); + expect(container.getElementsByClassName('dragging')).toHaveLength(1); + fireEvent.dragEnd(getByText('Label 1')); + expect(container.getElementsByClassName('dragging')).toHaveLength(0); +}); diff --git a/superset-frontend/src/explore/components/ExploreContainer/index.tsx b/superset-frontend/src/explore/components/ExploreContainer/index.tsx new file mode 100644 index 000000000000..b7f0df3a084d --- /dev/null +++ b/superset-frontend/src/explore/components/ExploreContainer/index.tsx @@ -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, { useEffect } from 'react'; +import { styled } from '@superset-ui/core'; +import { useDragDropManager } from 'react-dnd'; + +export const DraggingContext = React.createContext(false); +const StyledDiv = styled.div` + display: flex; + flex-direction: column; + height: 100%; + min-height: 0; +`; +const ExploreContainer: React.FC<{}> = ({ children }) => { + const dragDropManager = useDragDropManager(); + const [dragging, setDragging] = React.useState( + dragDropManager.getMonitor().isDragging(), + ); + + useEffect(() => { + const monitor = dragDropManager.getMonitor(); + const unsub = monitor.subscribeToStateChange(() => { + const isDragging = monitor.isDragging(); + setDragging(isDragging); + }); + + return () => { + unsub(); + }; + }, [dragDropManager]); + + return ( + + {children} + + ); +}; + +export default ExploreContainer; diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx index d3a32a2bd64a..0da43ebdc06c 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx @@ -68,6 +68,7 @@ import ConnectedControlPanelsContainer from '../ControlPanelsContainer'; import SaveModal from '../SaveModal'; import DataSourcePanel from '../DatasourcePanel'; import ConnectedExploreChartHeader from '../ExploreChartHeader'; +import ExploreContainer from '../ExploreContainer'; const propTypes = { ...ExploreChartPanel.propTypes, @@ -90,13 +91,6 @@ const propTypes = { isSaveModalVisible: PropTypes.bool, }; -const ExploreContainer = styled.div` - display: flex; - flex-direction: column; - height: 100%; - min-height: 0; -`; - const ExplorePanelContainer = styled.div` ${({ theme }) => css` background: ${theme.colors.grayscale.light5}; diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx index 397d6f54e05e..8e647318f6cc 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { ReactNode, useMemo } from 'react'; +import React, { ReactNode, useContext, useMemo } from 'react'; import { useDrop } from 'react-dnd'; import { t, useTheme } from '@superset-ui/core'; import ControlHeader from 'src/explore/components/ControlHeader'; @@ -31,6 +31,7 @@ import { } from 'src/explore/components/DatasourcePanel/types'; import Icons from 'src/components/Icons'; import { DndItemType } from '../../DndItemType'; +import { DraggingContext } from '../../ExploreContainer'; export type DndSelectLabelProps = { name: string; @@ -70,6 +71,7 @@ export default function DndSelectLabel({ type: monitor.getItemType(), }), }); + const isDragging = useContext(DraggingContext); const values = useMemo(() => valuesRenderer(), [valuesRenderer]); @@ -94,6 +96,7 @@ export default function DndSelectLabel({ data-test="dnd-labels-container" canDrop={canDrop} isOver={isOver} + isDragging={isDragging} > {values} {displayGhostButton && renderGhostButton()} diff --git a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx index 90ae73c6370a..60183b7f1dc3 100644 --- a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx +++ b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx @@ -106,18 +106,46 @@ export const LabelsContainer = styled.div` export const DndLabelsContainer = styled.div<{ canDrop?: boolean; isOver?: boolean; + isDragging: boolean; }>` + position: relative; padding: ${({ theme }) => theme.gridUnit}px; - border: ${({ canDrop, isOver, theme }) => { - if (canDrop) { - return `dashed 1px ${theme.colors.info.dark1}`; - } - if (isOver && !canDrop) { - return `dashed 1px ${theme.colors.error.dark1}`; + border: ${({ canDrop, isDragging, theme }) => { + if (isDragging) { + return `dashed 1px ${ + canDrop ? theme.colors.info.dark1 : theme.colors.error.dark1 + }`; } return `solid 1px ${theme.colors.grayscale.light2}`; }}; border-radius: ${({ theme }) => theme.gridUnit}px; + &:before, + &:after { + content: ' '; + position: absolute; + border-radius: ${({ theme }) => theme.gridUnit}px; + } + &:before { + display: ${({ isDragging }) => (isDragging ? 'block' : 'none')}; + background-color: ${({ theme, canDrop }) => + canDrop ? theme.colors.primary.base : theme.colors.error.light1}; + z-index: 10; + opacity: 0.2; + top: 1px; + right: 1px; + bottom: 1px; + left: 1px; + } + &:after { + display: ${({ isOver, canDrop }) => (canDrop && isOver ? 'block' : 'none')}; + background-color: ${({ theme }) => theme.colors.primary.base}; + z-index: 10; + opacity: 0.3; + top: -4px; + right: -4px; + bottom: -4px; + left: -4px; + } `; export const AddControlLabel = styled.div<{ From d944476a76be19783a5034b4f81aed4d684dc64f Mon Sep 17 00:00:00 2001 From: justinpark Date: Mon, 18 Mar 2024 18:11:06 -0700 Subject: [PATCH 2/9] tsc --- .../src/explore/components/controls/OptionControls/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx index 60183b7f1dc3..7fbd9ebb325c 100644 --- a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx +++ b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx @@ -106,7 +106,7 @@ export const LabelsContainer = styled.div` export const DndLabelsContainer = styled.div<{ canDrop?: boolean; isOver?: boolean; - isDragging: boolean; + isDragging?: boolean; }>` position: relative; padding: ${({ theme }) => theme.gridUnit}px; From f8358a2acc9f2f5ef05e1504ed0de4faaadd2657 Mon Sep 17 00:00:00 2001 From: justinpark Date: Mon, 18 Mar 2024 20:42:47 -0700 Subject: [PATCH 3/9] missing useDnd by ExploreContainer --- superset-frontend/src/pages/Chart/Chart.test.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/superset-frontend/src/pages/Chart/Chart.test.tsx b/superset-frontend/src/pages/Chart/Chart.test.tsx index 674a33d44db8..f8fb9fdcbba4 100644 --- a/superset-frontend/src/pages/Chart/Chart.test.tsx +++ b/superset-frontend/src/pages/Chart/Chart.test.tsx @@ -65,6 +65,7 @@ describe('ChartPage', () => { const { getByTestId } = render(, { useRouter: true, useRedux: true, + useDnd: true, }); await waitFor(() => expect(fetchMock.calls(exploreApiRoute).length).toBe(1), @@ -110,6 +111,7 @@ describe('ChartPage', () => { const { getByTestId } = render(, { useRouter: true, useRedux: true, + useDnd: true, }); await waitFor(() => expect(fetchMock.calls(exploreApiRoute).length).toBe(1), @@ -156,6 +158,7 @@ describe('ChartPage', () => { { useRouter: true, useRedux: true, + useDnd: true, }, ); await waitFor(() => From 9ef305fb06da2e09e551b2a8ea9d7cdf7fa42965 Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 19 Mar 2024 08:50:14 -0700 Subject: [PATCH 4/9] use theme variable --- .../components/controls/OptionControls/index.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx index 7fbd9ebb325c..dd484f329608 100644 --- a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx +++ b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx @@ -129,8 +129,8 @@ export const DndLabelsContainer = styled.div<{ display: ${({ isDragging }) => (isDragging ? 'block' : 'none')}; background-color: ${({ theme, canDrop }) => canDrop ? theme.colors.primary.base : theme.colors.error.light1}; - z-index: 10; - opacity: 0.2; + z-index: ${({ theme }) => theme.zIndex.aboveDashboardCharts}; + opacity: ${({ theme }) => theme.opacity.light}; top: 1px; right: 1px; bottom: 1px; @@ -139,12 +139,12 @@ export const DndLabelsContainer = styled.div<{ &:after { display: ${({ isOver, canDrop }) => (canDrop && isOver ? 'block' : 'none')}; background-color: ${({ theme }) => theme.colors.primary.base}; - z-index: 10; - opacity: 0.3; - top: -4px; - right: -4px; - bottom: -4px; - left: -4px; + z-index: ${({ theme }) => theme.zIndex.dropdown}; + opacity: ${({ theme }) => theme.opacity.mediumLight}; + top: ${({ theme }) => -theme.gridUnit}px; + right: ${({ theme }) => -theme.gridUnit}px; + bottom: ${({ theme }) => -theme.gridUnit}px; + left: ${({ theme }) => -theme.gridUnit}px; } `; From 9c26fa3dae162fc80fc757e9650729409b3ffb83 Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 19 Mar 2024 09:46:29 -0700 Subject: [PATCH 5/9] fix unavailable sorting issue --- .../ExploreContainer/ExploreContainer.test.tsx | 10 ++++++++-- .../src/explore/components/ExploreContainer/index.tsx | 5 +++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/explore/components/ExploreContainer/ExploreContainer.test.tsx b/superset-frontend/src/explore/components/ExploreContainer/ExploreContainer.test.tsx index 61ee2027fa04..50922256eaee 100644 --- a/superset-frontend/src/explore/components/ExploreContainer/ExploreContainer.test.tsx +++ b/superset-frontend/src/explore/components/ExploreContainer/ExploreContainer.test.tsx @@ -21,6 +21,7 @@ import { fireEvent, render } from 'spec/helpers/testing-library'; import { OptionControlLabel } from 'src/explore/components/controls/OptionControls'; import ExploreContainer, { DraggingContext } from '.'; +import OptionWrapper from '../controls/DndColumnSelectControl/OptionWrapper'; const MockChildren = () => { const dragging = React.useContext(DraggingContext); @@ -59,10 +60,12 @@ test('should update the style on dragging state', () => { index={1} label={Label 1} /> - Label 2} + label="Label 2" + clickClose={() => {}} + onShiftOptions={() => {}} /> , @@ -76,4 +79,7 @@ test('should update the style on dragging state', () => { expect(container.getElementsByClassName('dragging')).toHaveLength(1); fireEvent.dragEnd(getByText('Label 1')); expect(container.getElementsByClassName('dragging')).toHaveLength(0); + // don't show dragging state for the sorting item + fireEvent.dragStart(getByText('Label 2')); + expect(container.getElementsByClassName('dragging')).toHaveLength(0); }); diff --git a/superset-frontend/src/explore/components/ExploreContainer/index.tsx b/superset-frontend/src/explore/components/ExploreContainer/index.tsx index b7f0df3a084d..3c643739487e 100644 --- a/superset-frontend/src/explore/components/ExploreContainer/index.tsx +++ b/superset-frontend/src/explore/components/ExploreContainer/index.tsx @@ -36,6 +36,11 @@ const ExploreContainer: React.FC<{}> = ({ children }) => { useEffect(() => { const monitor = dragDropManager.getMonitor(); const unsub = monitor.subscribeToStateChange(() => { + const item = monitor.getItem() || {}; + // don't show dragging state for the sorting item + if ('dragIndex' in item) { + return; + } const isDragging = monitor.isDragging(); setDragging(isDragging); }); From 9eb62d18284d9cd211d09bd87fdd624d527484d2 Mon Sep 17 00:00:00 2001 From: justinpark Date: Wed, 20 Mar 2024 15:09:34 -0700 Subject: [PATCH 6/9] loading indicator for verifying values --- .../DndColumnSelectControl/DndSelectLabel.tsx | 3 +++ .../controls/OptionControls/index.tsx | 24 +++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx index 8e647318f6cc..d01d72116191 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx @@ -44,12 +44,14 @@ export type DndSelectLabelProps = { valuesRenderer: () => ReactNode; displayGhostButton?: boolean; onClickGhostButton: () => void; + isLoading?: boolean; }; export default function DndSelectLabel({ displayGhostButton = true, accept, valuesRenderer, + isLoading, ...props }: DndSelectLabelProps) { const theme = useTheme(); @@ -97,6 +99,7 @@ export default function DndSelectLabel({ canDrop={canDrop} isOver={isOver} isDragging={isDragging} + isLoading={isLoading} > {values} {displayGhostButton && renderGhostButton()} diff --git a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx index dd484f329608..c84d63b9bea7 100644 --- a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx +++ b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx @@ -18,7 +18,7 @@ */ import React, { useRef } from 'react'; import { useDrag, useDrop, DropTargetMonitor } from 'react-dnd'; -import { styled, t, useTheme } from '@superset-ui/core'; +import { styled, t, useTheme, keyframes, css } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import { Tooltip } from 'src/components/Tooltip'; import Icons from 'src/components/Icons'; @@ -103,10 +103,17 @@ export const LabelsContainer = styled.div` border-radius: ${({ theme }) => theme.gridUnit}px; `; +const borderPulse = keyframes` + to { + background-size: 100% 0px + } +`; + export const DndLabelsContainer = styled.div<{ canDrop?: boolean; isOver?: boolean; isDragging?: boolean; + isLoading?: boolean; }>` position: relative; padding: ${({ theme }) => theme.gridUnit}px; @@ -137,7 +144,8 @@ export const DndLabelsContainer = styled.div<{ left: 1px; } &:after { - display: ${({ isOver, canDrop }) => (canDrop && isOver ? 'block' : 'none')}; + display: ${({ isOver, canDrop, isLoading }) => + isLoading || (canDrop && isOver) ? 'block' : 'none'}; background-color: ${({ theme }) => theme.colors.primary.base}; z-index: ${({ theme }) => theme.zIndex.dropdown}; opacity: ${({ theme }) => theme.opacity.mediumLight}; @@ -145,6 +153,18 @@ export const DndLabelsContainer = styled.div<{ right: ${({ theme }) => -theme.gridUnit}px; bottom: ${({ theme }) => -theme.gridUnit}px; left: ${({ theme }) => -theme.gridUnit}px; + + ${({ theme, isLoading }) => + isLoading && + css` + animation: ${borderPulse} 3s ease-out infinite; + background: linear-gradient(currentColor 0 0) 0 100%/0% 3px no-repeat; + top: auto; + right: ${theme.gridUnit}px; + left: ${theme.gridUnit}px; + bottom: -${theme.gridUnit / 2}px; + height: ${theme.gridUnit / 2}px; + `} } `; From acc8223709ce2b8bb24a71deb0981cd3adb861d7 Mon Sep 17 00:00:00 2001 From: justinpark Date: Thu, 21 Mar 2024 08:54:40 -0700 Subject: [PATCH 7/9] change loading pulse animation --- .../components/controls/OptionControls/index.tsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx index c84d63b9bea7..aa54b95937c3 100644 --- a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx +++ b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx @@ -104,8 +104,17 @@ export const LabelsContainer = styled.div` `; const borderPulse = keyframes` - to { - background-size: 100% 0px + 0% { + right: 100%; + } + 50% { + left: 4px; + } + 90% { + right: 4px; + } + 100% { + left: 100%; } `; @@ -157,8 +166,9 @@ export const DndLabelsContainer = styled.div<{ ${({ theme, isLoading }) => isLoading && css` - animation: ${borderPulse} 3s ease-out infinite; + animation: ${borderPulse} 2s ease-in infinite; background: linear-gradient(currentColor 0 0) 0 100%/0% 3px no-repeat; + background-size: 100% ${theme.gridUnit / 2}px; top: auto; right: ${theme.gridUnit}px; left: ${theme.gridUnit}px; From 9d81dca03788d2246f5c340feefa1586ce5a6093 Mon Sep 17 00:00:00 2001 From: justinpark Date: Thu, 21 Mar 2024 12:19:40 -0700 Subject: [PATCH 8/9] hide dropzone while loading --- .../controls/OptionControls/index.tsx | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx index aa54b95937c3..a2b1c618d7d9 100644 --- a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx +++ b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx @@ -126,13 +126,17 @@ export const DndLabelsContainer = styled.div<{ }>` position: relative; padding: ${({ theme }) => theme.gridUnit}px; - border: ${({ canDrop, isDragging, theme }) => { - if (isDragging) { + border: ${({ canDrop, isDragging, isLoading, theme }) => { + if (!isLoading && isDragging) { return `dashed 1px ${ canDrop ? theme.colors.info.dark1 : theme.colors.error.dark1 }`; } - return `solid 1px ${theme.colors.grayscale.light2}`; + return `solid 1px ${ + isLoading && isDragging + ? theme.colors.warning.light1 + : theme.colors.grayscale.light2 + }`; }}; border-radius: ${({ theme }) => theme.gridUnit}px; &:before, @@ -142,7 +146,8 @@ export const DndLabelsContainer = styled.div<{ border-radius: ${({ theme }) => theme.gridUnit}px; } &:before { - display: ${({ isDragging }) => (isDragging ? 'block' : 'none')}; + display: ${({ isDragging, isLoading }) => + isDragging || isLoading ? 'block' : 'none'}; background-color: ${({ theme, canDrop }) => canDrop ? theme.colors.primary.base : theme.colors.error.light1}; z-index: ${({ theme }) => theme.zIndex.aboveDashboardCharts}; @@ -151,17 +156,6 @@ export const DndLabelsContainer = styled.div<{ right: 1px; bottom: 1px; left: 1px; - } - &:after { - display: ${({ isOver, canDrop, isLoading }) => - isLoading || (canDrop && isOver) ? 'block' : 'none'}; - background-color: ${({ theme }) => theme.colors.primary.base}; - z-index: ${({ theme }) => theme.zIndex.dropdown}; - opacity: ${({ theme }) => theme.opacity.mediumLight}; - top: ${({ theme }) => -theme.gridUnit}px; - right: ${({ theme }) => -theme.gridUnit}px; - bottom: ${({ theme }) => -theme.gridUnit}px; - left: ${({ theme }) => -theme.gridUnit}px; ${({ theme, isLoading }) => isLoading && @@ -176,6 +170,19 @@ export const DndLabelsContainer = styled.div<{ height: ${theme.gridUnit / 2}px; `} } + &:after { + display: ${({ isOver, canDrop, isLoading }) => + isLoading || (canDrop && isOver) ? 'block' : 'none'}; + background-color: ${({ theme, isLoading }) => + isLoading ? theme.colors.grayscale.light3 : theme.colors.primary.base}; + z-index: ${({ theme }) => theme.zIndex.dropdown}; + opacity: ${({ theme }) => theme.opacity.mediumLight}; + top: ${({ theme }) => -theme.gridUnit}px; + right: ${({ theme }) => -theme.gridUnit}px; + bottom: ${({ theme }) => -theme.gridUnit}px; + left: ${({ theme }) => -theme.gridUnit}px; + cursor: ${({ isLoading }) => (isLoading ? 'wait' : 'auto')}; + } `; export const AddControlLabel = styled.div<{ From bdfb926255941c29d4e7840f0483c771d516ca1a Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 26 Mar 2024 10:30:25 -0700 Subject: [PATCH 9/9] group all theme ref --- .../DndColumnSelectControl/DndSelectLabel.tsx | 2 +- .../controls/OptionControls/index.tsx | 73 ++++++++++--------- 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx index d01d72116191..f4da2d1729dc 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx @@ -57,7 +57,7 @@ export default function DndSelectLabel({ const theme = useTheme(); const [{ isOver, canDrop }, datasourcePanelDrop] = useDrop({ - accept, + accept: isLoading ? [] : accept, drop: (item: DatasourcePanelDndItem) => { props.onDrop(item); diff --git a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx index a2b1c618d7d9..2e2bf1f5d7b8 100644 --- a/superset-frontend/src/explore/components/controls/OptionControls/index.tsx +++ b/superset-frontend/src/explore/components/controls/OptionControls/index.tsx @@ -124,39 +124,55 @@ export const DndLabelsContainer = styled.div<{ isDragging?: boolean; isLoading?: boolean; }>` + ${({ theme, isLoading, canDrop, isDragging, isOver }) => ` position: relative; - padding: ${({ theme }) => theme.gridUnit}px; - border: ${({ canDrop, isDragging, isLoading, theme }) => { - if (!isLoading && isDragging) { - return `dashed 1px ${ - canDrop ? theme.colors.info.dark1 : theme.colors.error.dark1 - }`; - } - return `solid 1px ${ - isLoading && isDragging - ? theme.colors.warning.light1 - : theme.colors.grayscale.light2 - }`; - }}; - border-radius: ${({ theme }) => theme.gridUnit}px; + padding: ${theme.gridUnit}px; + border: ${ + !isLoading && isDragging + ? `dashed 1px ${ + canDrop ? theme.colors.info.dark1 : theme.colors.error.dark1 + }` + : `solid 1px ${ + isLoading && isDragging + ? theme.colors.warning.light1 + : theme.colors.grayscale.light2 + }` + }; + border-radius: ${theme.gridUnit}px; &:before, &:after { content: ' '; position: absolute; - border-radius: ${({ theme }) => theme.gridUnit}px; + border-radius: ${theme.gridUnit}px; } &:before { - display: ${({ isDragging, isLoading }) => - isDragging || isLoading ? 'block' : 'none'}; - background-color: ${({ theme, canDrop }) => - canDrop ? theme.colors.primary.base : theme.colors.error.light1}; - z-index: ${({ theme }) => theme.zIndex.aboveDashboardCharts}; - opacity: ${({ theme }) => theme.opacity.light}; + display: ${isDragging || isLoading ? 'block' : 'none'}; + background-color: ${ + canDrop ? theme.colors.primary.base : theme.colors.error.light1 + }; + z-index: ${theme.zIndex.aboveDashboardCharts}; + opacity: ${theme.opacity.light}; top: 1px; right: 1px; bottom: 1px; left: 1px; + } + &:after { + display: ${isLoading || (canDrop && isOver) ? 'block' : 'none'}; + background-color: ${ + isLoading ? theme.colors.grayscale.light3 : theme.colors.primary.base + }; + z-index: ${theme.zIndex.dropdown}; + opacity: ${theme.opacity.mediumLight}; + top: ${-theme.gridUnit}px; + right: ${-theme.gridUnit}px; + bottom: ${-theme.gridUnit}px; + left: ${-theme.gridUnit}px; + cursor: ${isLoading ? 'wait' : 'auto'}; + } + `} + &:before { ${({ theme, isLoading }) => isLoading && css` @@ -168,20 +184,7 @@ export const DndLabelsContainer = styled.div<{ left: ${theme.gridUnit}px; bottom: -${theme.gridUnit / 2}px; height: ${theme.gridUnit / 2}px; - `} - } - &:after { - display: ${({ isOver, canDrop, isLoading }) => - isLoading || (canDrop && isOver) ? 'block' : 'none'}; - background-color: ${({ theme, isLoading }) => - isLoading ? theme.colors.grayscale.light3 : theme.colors.primary.base}; - z-index: ${({ theme }) => theme.zIndex.dropdown}; - opacity: ${({ theme }) => theme.opacity.mediumLight}; - top: ${({ theme }) => -theme.gridUnit}px; - right: ${({ theme }) => -theme.gridUnit}px; - bottom: ${({ theme }) => -theme.gridUnit}px; - left: ${({ theme }) => -theme.gridUnit}px; - cursor: ${({ isLoading }) => (isLoading ? 'wait' : 'auto')}; + `}; } `;