Skip to content

Commit

Permalink
feat(dashboard): Let users re-arrange native filters (apache#16154)
Browse files Browse the repository at this point in the history
* feat. flag: ON

* refactor filter tabs

* WIP

* Drag and Rearrange, styling

* refactoring dnd code

* Add apache license header

* Fix bug reported during PR review

* Minor fix on remove

* turn off filters

* Fix status

* Fix a test

* Address PR comments

* iSort fixes

* Add type key to the new filters

* Fix wrong attribute

* indent

* PR comments

* PR comments

* Fix failing tests

* Styling

* Fix remove filter

* Fix the drag issue

* Save works

* fix

* Write tests

* Style changes

* New Icon

* Grab & Grabbing Cursor

* Commented out code

* Fix tests, fix CI

* Fix failing tests

* Fix test

* Style fixes

* portal nodes dependency

* More style fixes

* PR comments

* add unique ids to collapse panels

* Filter removal bug fixed

* PR comments

* Fix test warnings

* delete filter tabs

* Fix the breaking test

* Fix warnings

* Fix the weird bug on cancel

* refactor

* Fix broken scope
  • Loading branch information
m-ajay authored and Emmanuel Bavoux committed Nov 14, 2021
1 parent 3788aa1 commit 1ac5a4a
Show file tree
Hide file tree
Showing 20 changed files with 1,084 additions and 394 deletions.
36 changes: 36 additions & 0 deletions superset-frontend/spec/fixtures/mockNativeFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -448,3 +448,39 @@ export const mockQueryDataForCountries = [
{ country_name: 'Zambia', 'SUM(SP_POP_TOTL)': 438847085 },
{ country_name: 'Zimbabwe', 'SUM(SP_POP_TOTL)': 509866860 },
];

export const buildNativeFilter = (
id: string,
name: string,
parents: string[],
) => ({
id,
controlValues: {
multiSelect: true,
enableEmptyFilter: false,
defaultToFirstItem: false,
inverseSelection: false,
searchAllOptions: false,
},
name,
filterType: 'filter_select',
targets: [
{
datasetId: 1,
column: {
name,
},
},
],
defaultDataMask: {
extraFormData: {},
filterState: {},
ownState: {},
},
cascadeParentIds: parents,
scope: {
rootPath: ['ROOT_ID'],
excluded: [],
},
type: 'NATIVE_FILTER',
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,16 @@
* specific language governing permissions and limitations
* under the License.
*/
import { ReactWrapper } from 'enzyme';
import React from 'react';
import { styledMount as mount } from 'spec/helpers/theming';
import { DndProvider } from 'react-dnd';
import { HTML5Backend } from 'react-dnd-html5-backend';
import { act } from 'react-dom/test-utils';
import { ReactWrapper } from 'enzyme';
import { Provider } from 'react-redux';
import Alert from 'src/components/Alert';
import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
import { mockStore } from 'spec/fixtures/mockStore';
import { styledMount as mount } from 'spec/helpers/theming';
import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
import Alert from 'src/components/Alert';
import { FiltersConfigModal } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal';

Object.defineProperty(window, 'matchMedia', {
Expand Down Expand Up @@ -66,7 +68,9 @@ describe('FiltersConfigModal', () => {
function setup(overridesProps?: any) {
return mount(
<Provider store={mockStore}>
<FiltersConfigModal {...mockedProps} {...overridesProps} />
<DndProvider backend={HTML5Backend}>
<FiltersConfigModal {...mockedProps} {...overridesProps} />
</DndProvider>
</Provider>,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,9 @@ describe('FilterBar', () => {

const renderWrapper = (props = closedBarProps, state?: object) =>
render(<FilterBar {...props} width={280} height={400} offset={0} />, {
useRedux: true,
initialState: state,
useDnd: true,
useRedux: true,
useRouter: true,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { getFilterBarTestId } from '..';

export interface FCBProps {
createNewOnOpen?: boolean;
dashboardId?: number;
}

const HeaderButton = styled(Button)`
Expand All @@ -35,6 +36,7 @@ const HeaderButton = styled(Button)`

export const FilterConfigurationLink: React.FC<FCBProps> = ({
createNewOnOpen,
dashboardId,
children,
}) => {
const dispatch = useDispatch();
Expand Down Expand Up @@ -65,6 +67,7 @@ export const FilterConfigurationLink: React.FC<FCBProps> = ({
onSave={submit}
onCancel={close}
createNewOnOpen={createNewOnOpen}
key={`filters-for-${dashboardId}`}
/>
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,23 @@
* under the License.
*/
import React, { FC, useMemo, useState } from 'react';
import { DataMask, styled, t } from '@superset-ui/core';
import { css } from '@emotion/react';
import * as portals from 'react-reverse-portal';
import { DataMaskStateWithId } from 'src/dataMask/types';
import { DataMask, styled, t } from '@superset-ui/core';
import {
createHtmlPortalNode,
InPortal,
OutPortal,
} from 'react-reverse-portal';
import { Collapse } from 'src/common/components';
import { DataMaskStateWithId } from 'src/dataMask/types';
import {
useDashboardHasTabs,
useSelectFiltersInScope,
} from 'src/dashboard/components/nativeFilters/state';
import { Filter } from 'src/dashboard/components/nativeFilters/types';
import CascadePopover from '../CascadeFilters/CascadePopover';
import { buildCascadeFiltersTree } from './utils';
import { useFilters } from '../state';
import { Filter } from '../../types';
import { useDashboardHasTabs, useSelectFiltersInScope } from '../../state';
import { buildCascadeFiltersTree } from './utils';

const Wrapper = styled.div`
padding: ${({ theme }) => theme.gridUnit * 4}px;
Expand All @@ -52,7 +59,7 @@ const FilterControls: FC<FilterControlsProps> = ({
const portalNodes = React.useMemo(() => {
const nodes = new Array(filterValues.length);
for (let i = 0; i < filterValues.length; i += 1) {
nodes[i] = portals.createHtmlPortalNode();
nodes[i] = createHtmlPortalNode();
}
return nodes;
}, [filterValues.length]);
Expand All @@ -78,7 +85,7 @@ const FilterControls: FC<FilterControlsProps> = ({
{portalNodes
.filter((node, index) => cascadeFilterIds.has(filterValues[index].id))
.map((node, index) => (
<portals.InPortal node={node}>
<InPortal node={node}>
<CascadePopover
data-test="cascade-filters-control"
key={cascadeFilters[index].id}
Expand All @@ -92,11 +99,11 @@ const FilterControls: FC<FilterControlsProps> = ({
directPathToChild={directPathToChild}
inView={false}
/>
</portals.InPortal>
</InPortal>
))}
{filtersInScope.map(filter => {
const index = filterValues.findIndex(f => f.id === filter.id);
return <portals.OutPortal node={portalNodes[index]} inView />;
return <OutPortal node={portalNodes[index]} inView />;
})}
{showCollapsePanel && (
<Collapse
Expand Down Expand Up @@ -134,7 +141,7 @@ const FilterControls: FC<FilterControlsProps> = ({
>
{filtersOutOfScope.map(filter => {
const index = cascadeFilters.findIndex(f => f.id === filter.id);
return <portals.OutPortal node={portalNodes[index]} inView />;
return <OutPortal node={portalNodes[index]} inView />;
})}
</Collapse.Panel>
</Collapse>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ const Header: FC<HeaderProps> = ({
const canEdit = useSelector<RootState, boolean>(
({ dashboardInfo }) => dashboardInfo.dash_edit_perm,
);
const dashboardId = useSelector<RootState, number>(
({ dashboardInfo }) => dashboardInfo.id,
);

const isClearAllDisabled = Object.values(dataMaskApplied).every(
filter =>
Expand All @@ -99,7 +102,10 @@ const Header: FC<HeaderProps> = ({
<TitleArea>
<span>{t('Filters')}</span>
{canEdit && (
<FilterConfigurationLink createNewOnOpen={filterValues.length === 0}>
<FilterConfigurationLink
dashboardId={dashboardId}
createNewOnOpen={filterValues.length === 0}
>
<Icons.Edit
data-test="create-filter"
iconColor={theme.colors.grayscale.base}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/**
* 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 { styled } from '@superset-ui/core';
import React, { useRef } from 'react';
import {
DragSourceMonitor,
DropTargetMonitor,
useDrag,
useDrop,
XYCoord,
} from 'react-dnd';
import Icons, { IconType } from 'src/components/Icons';

interface TitleContainerProps {
readonly isDragging: boolean;
}

const FILTER_TYPE = 'FILTER';

const Container = styled.div<TitleContainerProps>`
${({ isDragging, theme }) => `
opacity: ${isDragging ? 0.3 : 1};
cursor: ${isDragging ? 'grabbing' : 'pointer'};
width: 100%;
display: flex;
padding: ${theme.gridUnit}px
`}
`;

const DragIcon = styled(Icons.Drag, {
shouldForwardProp: propName => propName !== 'isDragging',
})<IconType & { isDragging: boolean }>`
${({ isDragging, theme }) => `
font-size: ${theme.typography.sizes.m}px;
margin-top: 15px;
cursor: ${isDragging ? 'grabbing' : 'grab'};
padding-left: ${theme.gridUnit}px;
`}
`;

interface FilterTabTitleProps {
index: number;
filterIds: string[];
onRearrage: (dragItemIndex: number, targetIndex: number) => void;
}

interface DragItem {
index: number;
filterIds: string[];
type: string;
}

export const DraggableFilter: React.FC<FilterTabTitleProps> = ({
index,
onRearrage,
filterIds,
children,
}) => {
const ref = useRef<HTMLDivElement>(null);
const [{ isDragging }, drag] = useDrag({
item: { filterIds, type: FILTER_TYPE, index },
collect: (monitor: DragSourceMonitor) => ({
isDragging: monitor.isDragging(),
}),
});
const [, drop] = useDrop({
accept: FILTER_TYPE,
hover: (item: DragItem, monitor: DropTargetMonitor) => {
if (!ref.current) {
return;
}

const dragIndex = item.index;
const hoverIndex = index;

// Don't replace items with themselves
if (dragIndex === hoverIndex) {
return;
}
// Determine rectangle on screen
const hoverBoundingRect = ref.current?.getBoundingClientRect();

// Get vertical middle
const hoverMiddleY =
(hoverBoundingRect.bottom - hoverBoundingRect.top) / 2;

// Determine mouse position
const clientOffset = monitor.getClientOffset();

// Get pixels to the top
const hoverClientY = (clientOffset as XYCoord).y - hoverBoundingRect.top;

// Only perform the move when the mouse has crossed half of the items height
// When dragging downwards, only move when the cursor is below 50%
// When dragging upwards, only move when the cursor is above 50%

// Dragging downwards
if (dragIndex < hoverIndex && hoverClientY < hoverMiddleY) {
return;
}

// Dragging upwards
if (dragIndex > hoverIndex && hoverClientY > hoverMiddleY) {
return;
}

onRearrage(dragIndex, hoverIndex);
// Note: we're mutating the monitor item here.
// Generally it's better to avoid mutations,
// but it's good here for the sake of performance
// to avoid expensive index searches.
// eslint-disable-next-line no-param-reassign
item.index = hoverIndex;
},
});
drag(drop(ref));
return (
<Container ref={ref} isDragging={isDragging}>
<DragIcon isDragging={isDragging} alt="Move icon" className="dragIcon" />
<div css={{ flexGrow: 4 }}>{children}</div>
</Container>
);
};

export default DraggableFilter;
Loading

0 comments on commit 1ac5a4a

Please sign in to comment.