From a6409d448631f13ded251ebf869f0cb6ba6f1df9 Mon Sep 17 00:00:00 2001 From: justin-park Date: Tue, 19 Jul 2022 13:24:42 -0700 Subject: [PATCH 1/9] feat(dashboard): Resizable side FilterBar * Add Resizable panel in DashboardBuilder to adjust the width for FiltersPanel * store the adjusted width for individual dashboard in localStorage to memorize the state * migrate DashboardBuilder test code by testing-library and jest --- superset-frontend/spec/helpers/setup.ts | 2 + .../spec/helpers/testing-library.tsx | 24 +- .../DashboardBuilder.test.jsx | 196 ------------- .../DashboardBuilder.test.tsx | 276 ++++++++++++++++++ .../DashboardBuilder/DashboardBuilder.tsx | 80 ++++- .../useStoredFilterBarWidth.test.ts | 85 ++++++ .../useStoredFilterBarWidth.ts | 52 ++++ .../ActionButtons/ActionButtons.test.tsx | 26 ++ .../FilterBar/ActionButtons/index.tsx | 10 +- .../nativeFilters/FilterBar/index.tsx | 1 + superset-frontend/src/dashboard/constants.ts | 1 + .../src/utils/localStorageHelpers.ts | 2 + 12 files changed, 532 insertions(+), 223 deletions(-) delete mode 100644 superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.jsx create mode 100644 superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx create mode 100644 superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.test.ts create mode 100644 superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.ts diff --git a/superset-frontend/spec/helpers/setup.ts b/superset-frontend/spec/helpers/setup.ts index c2c991f95621d..bd2961e23cec0 100644 --- a/superset-frontend/spec/helpers/setup.ts +++ b/superset-frontend/spec/helpers/setup.ts @@ -19,9 +19,11 @@ import 'jest-enzyme'; import './shim'; import { configure as configureTestingLibrary } from '@testing-library/react'; +import { matchers } from '@emotion/jest'; configureTestingLibrary({ testIdAttribute: 'data-test', }); document.body.innerHTML = '
'; +expect.extend(matchers); diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 56489cce84714..b0e04902c2e16 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/superset-frontend/spec/helpers/testing-library.tsx @@ -22,7 +22,13 @@ import { render, RenderOptions } from '@testing-library/react'; import { ThemeProvider, supersetTheme } from '@superset-ui/core'; import { BrowserRouter } from 'react-router-dom'; import { Provider } from 'react-redux'; -import { combineReducers, createStore, applyMiddleware, compose } from 'redux'; +import { + combineReducers, + createStore, + applyMiddleware, + compose, + Store, +} from 'redux'; import thunk from 'redux-thunk'; import { DndProvider } from 'react-dnd'; import { HTML5Backend } from 'react-dnd-html5-backend'; @@ -36,6 +42,7 @@ type Options = Omit & { useRouter?: boolean; initialState?: {}; reducers?: {}; + store?: Store; }; function createWrapper(options?: Options) { @@ -46,6 +53,7 @@ function createWrapper(options?: Options) { useRouter, initialState, reducers, + store, } = options || {}; return ({ children }: { children?: ReactNode }) => { @@ -58,13 +66,15 @@ function createWrapper(options?: Options) { } if (useRedux) { - const store = createStore( - combineReducers(reducers || reducerIndex), - initialState || {}, - compose(applyMiddleware(thunk)), - ); + const mockStore = + store ?? + createStore( + combineReducers(reducers || reducerIndex), + initialState || {}, + compose(applyMiddleware(thunk)), + ); - result = {result}; + result = {result}; } if (useQueryParams) { diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.jsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.jsx deleted file mode 100644 index 937012a9f8bea..0000000000000 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.jsx +++ /dev/null @@ -1,196 +0,0 @@ -/** - * 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 { Provider } from 'react-redux'; -import React from 'react'; -import { mount } from 'enzyme'; -import sinon from 'sinon'; -import fetchMock from 'fetch-mock'; -import { ParentSize } from '@vx/responsive'; -import { supersetTheme, ThemeProvider } from '@superset-ui/core'; -import Tabs from 'src/components/Tabs'; -import { DndProvider } from 'react-dnd'; -import { HTML5Backend } from 'react-dnd-html5-backend'; -import BuilderComponentPane from 'src/dashboard/components/BuilderComponentPane'; -import DashboardBuilder from 'src/dashboard/components/DashboardBuilder/DashboardBuilder'; -import DashboardComponent from 'src/dashboard/containers/DashboardComponent'; -import DashboardHeader from 'src/dashboard/containers/DashboardHeader'; -import DashboardGrid from 'src/dashboard/containers/DashboardGrid'; -import * as dashboardStateActions from 'src/dashboard/actions/dashboardState'; -import { - dashboardLayout as undoableDashboardLayout, - dashboardLayoutWithTabs as undoableDashboardLayoutWithTabs, -} from 'spec/fixtures/mockDashboardLayout'; -import { mockStoreWithTabs, storeWithState } from 'spec/fixtures/mockStore'; -import mockState from 'spec/fixtures/mockState'; -import { - DASHBOARD_ROOT_ID, - DASHBOARD_GRID_ID, -} from 'src/dashboard/util/constants'; - -fetchMock.get('glob:*/csstemplateasyncmodelview/api/read', {}); - -jest.mock('src/dashboard/actions/dashboardState'); - -describe('DashboardBuilder', () => { - let favStarStub; - let activeTabsStub; - - beforeAll(() => { - // this is invoked on mount, so we stub it instead of making a request - favStarStub = sinon - .stub(dashboardStateActions, 'fetchFaveStar') - .returns({ type: 'mock-action' }); - activeTabsStub = sinon - .stub(dashboardStateActions, 'setActiveTabs') - .returns({ type: 'mock-action' }); - }); - - afterAll(() => { - favStarStub.restore(); - activeTabsStub.restore(); - }); - - function setup(overrideState = {}, overrideStore) { - const store = - overrideStore ?? - storeWithState({ - ...mockState, - dashboardLayout: undoableDashboardLayout, - ...overrideState, - }); - return mount( - - - - - , - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { theme: supersetTheme }, - }, - ); - } - - it('should render a StickyContainer with class "dashboard"', () => { - const wrapper = setup(); - const stickyContainer = wrapper.find('[data-test="dashboard-content"]'); - expect(stickyContainer).toHaveLength(1); - expect(stickyContainer.prop('className')).toBe('dashboard'); - }); - - it('should add the "dashboard--editing" class if editMode=true', () => { - const wrapper = setup({ dashboardState: { editMode: true } }); - const stickyContainer = wrapper.find('[data-test="dashboard-content"]'); - expect(stickyContainer.prop('className')).toBe( - 'dashboard dashboard--editing', - ); - }); - - it('should render a DragDroppable DashboardHeader', () => { - const wrapper = setup(); - expect(wrapper.find(DashboardHeader)).toExist(); - }); - - it('should render a Sticky top-level Tabs if the dashboard has tabs', () => { - const wrapper = setup( - { dashboardLayout: undoableDashboardLayoutWithTabs }, - mockStoreWithTabs, - ); - - const sticky = wrapper.find('[data-test="top-level-tabs"]'); - const dashboardComponent = sticky.find(DashboardComponent); - - const tabChildren = - undoableDashboardLayoutWithTabs.present.TABS_ID.children; - expect(dashboardComponent).toHaveLength(1 + tabChildren.length); // tab + tabs - expect(dashboardComponent.at(0).prop('id')).toBe('TABS_ID'); - tabChildren.forEach((tabId, i) => { - expect(dashboardComponent.at(i + 1).prop('id')).toBe(tabId); - }); - }); - - it('should render one Tabs and two TabPane', () => { - const wrapper = setup({ dashboardLayout: undoableDashboardLayoutWithTabs }); - const parentSize = wrapper.find(ParentSize); - expect(parentSize.find(Tabs)).toHaveLength(1); - expect(parentSize.find(Tabs.TabPane)).toHaveLength(2); - }); - - it('should render a TabPane and DashboardGrid for first Tab', () => { - const wrapper = setup({ dashboardLayout: undoableDashboardLayoutWithTabs }); - const parentSize = wrapper.find(ParentSize); - const expectedCount = - undoableDashboardLayoutWithTabs.present.TABS_ID.children.length; - expect(parentSize.find(Tabs.TabPane)).toHaveLength(expectedCount); - expect( - parentSize.find(Tabs.TabPane).first().find(DashboardGrid), - ).toHaveLength(1); - }); - - it('should render a TabPane and DashboardGrid for second Tab', () => { - const wrapper = setup({ - dashboardLayout: undoableDashboardLayoutWithTabs, - dashboardState: { - ...mockState, - directPathToChild: [DASHBOARD_ROOT_ID, 'TABS_ID', 'TAB_ID2'], - }, - }); - const parentSize = wrapper.find(ParentSize); - const expectedCount = - undoableDashboardLayoutWithTabs.present.TABS_ID.children.length; - expect(parentSize.find(Tabs.TabPane)).toHaveLength(expectedCount); - expect( - parentSize.find(Tabs.TabPane).at(1).find(DashboardGrid), - ).toHaveLength(1); - }); - - it('should render a BuilderComponentPane if editMode=false and user selects "Insert Components" pane', () => { - const wrapper = setup(); - expect(wrapper.find(BuilderComponentPane)).not.toExist(); - }); - - it('should render a BuilderComponentPane if editMode=true and user selects "Insert Components" pane', () => { - const wrapper = setup({ dashboardState: { editMode: true } }); - expect(wrapper.find(BuilderComponentPane)).toExist(); - }); - - it('should change redux state if a top-level Tab is clicked', () => { - dashboardStateActions.setDirectPathToChild = jest.fn(arg0 => ({ - type: 'type', - arg0, - })); - const wrapper = setup({ - ...mockStoreWithTabs, - dashboardLayout: undoableDashboardLayoutWithTabs, - }); - - expect(wrapper.find(Tabs).at(1).prop('activeKey')).toBe(DASHBOARD_GRID_ID); - - wrapper - .find('.dashboard-component-tabs .ant-tabs .ant-tabs-tab') - .at(1) - .simulate('click'); - - expect(dashboardStateActions.setDirectPathToChild).toHaveBeenCalledWith([ - 'ROOT_ID', - 'TABS_ID', - 'TAB_ID2', - ]); - }); -}); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx new file mode 100644 index 0000000000000..2ffe9018c10de --- /dev/null +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx @@ -0,0 +1,276 @@ +/** + * 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 { Store } from 'redux'; +import React from 'react'; +import fetchMock from 'fetch-mock'; +import { render } from 'spec/helpers/testing-library'; +import { fireEvent, within } from '@testing-library/dom'; +import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; +import DashboardBuilder from 'src/dashboard/components/DashboardBuilder/DashboardBuilder'; +import useStoredFilterBarWidth from 'src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth'; +import { + fetchFaveStar, + setActiveTabs, + setDirectPathToChild, +} from 'src/dashboard/actions/dashboardState'; +import { + dashboardLayout as undoableDashboardLayout, + dashboardLayoutWithTabs as undoableDashboardLayoutWithTabs, +} from 'spec/fixtures/mockDashboardLayout'; +import { mockStoreWithTabs, storeWithState } from 'spec/fixtures/mockStore'; +import mockState from 'spec/fixtures/mockState'; +import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants'; + +fetchMock.get('glob:*/csstemplateasyncmodelview/api/read', {}); + +jest.mock('src/dashboard/actions/dashboardState', () => ({ + ...jest.requireActual('src/dashboard/actions/dashboardState'), + fetchFaveStar: jest.fn(), + setActiveTabs: jest.fn(), + setDirectPathToChild: jest.fn(), +})); +jest.mock('src/featureFlags'); +jest.mock('src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth'); + +// mock following dependant components to fix the prop warnings +jest.mock('src/components/Icons/Icon', () => () => ( +
+)); +jest.mock('src/components/Select/WindowedSelect', () => () => ( +
+)); +jest.mock('src/components/Select', () => () => ( +
+)); +jest.mock('src/components/Select/Select', () => () => ( +
+)); +jest.mock('src/components/Select/AsyncSelect', () => () => ( +
+)); +jest.mock('src/dashboard/components/Header/HeaderActionsDropdown', () => () => ( +
+)); +jest.mock('src/components/PageHeaderWithActions', () => ({ + PageHeaderWithActions: () => ( +
+ ), +})); +jest.mock( + 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal', + () => () =>
, +); +jest.mock('src/dashboard/components/BuilderComponentPane', () => () => ( +
+)); +jest.mock('src/dashboard/components/nativeFilters/FilterBar', () => () => ( +
+)); +jest.mock('src/dashboard/containers/DashboardGrid', () => () => ( +
+)); + +describe('DashboardBuilder', () => { + let favStarStub: jest.Mock; + let activeTabsStub: jest.Mock; + + beforeAll(() => { + // this is invoked on mount, so we stub it instead of making a request + favStarStub = (fetchFaveStar as jest.Mock).mockReturnValue({ + type: 'mock-action', + }); + activeTabsStub = (setActiveTabs as jest.Mock).mockReturnValue({ + type: 'mock-action', + }); + (useStoredFilterBarWidth as jest.Mock).mockImplementation(() => [ + 100, + jest.fn(), + ]); + (isFeatureEnabled as jest.Mock).mockImplementation(() => false); + }); + + afterAll(() => { + favStarStub.mockReset(); + activeTabsStub.mockReset(); + (useStoredFilterBarWidth as jest.Mock).mockReset(); + }); + + function setup(overrideState = {}, overrideStore?: Store) { + return render(, { + useRedux: true, + store: storeWithState({ + ...mockState, + dashboardLayout: undoableDashboardLayout, + ...overrideState, + }), + useDnd: true, + }); + } + + it('should render a StickyContainer with class "dashboard"', () => { + const { getByTestId } = setup(); + const stickyContainer = getByTestId('dashboard-content'); + expect(stickyContainer.className).toBe('dashboard'); + }); + + it('should add the "dashboard--editing" class if editMode=true', () => { + const { getByTestId } = setup({ + dashboardState: { ...mockState.dashboardState, editMode: true }, + }); + const stickyContainer = getByTestId('dashboard-content'); + expect(stickyContainer.className).toBe('dashboard dashboard--editing'); + }); + + it('should render a DragDroppable DashboardHeader', () => { + const { queryByTestId } = setup(); + const header = queryByTestId('dashboard-header-container'); + expect(header).toBeTruthy(); + }); + + it('should render a Sticky top-level Tabs if the dashboard has tabs', async () => { + const { findAllByTestId } = setup( + { dashboardLayout: undoableDashboardLayoutWithTabs }, + mockStoreWithTabs, + ); + const sticky = await findAllByTestId('nav-list'); + + expect(sticky).toHaveLength(1); + expect(sticky[0].getAttribute('id')).toContain('TABS_ID'); + + const dashboardTabComponents = within(sticky[0]).getAllByRole('tab'); + const tabChildren = + undoableDashboardLayoutWithTabs.present.TABS_ID.children; + expect(dashboardTabComponents).toHaveLength(tabChildren.length); + tabChildren.forEach((tabId, i) => { + expect(dashboardTabComponents[i].getAttribute('id')).toContain(tabId); + }); + }); + + it('should render one Tabs and two TabPane', async () => { + const { findAllByRole } = setup({ + dashboardLayout: undoableDashboardLayoutWithTabs, + }); + const tabs = await findAllByRole('tablist'); + expect(tabs).toHaveLength(1); + const tabPanels = await findAllByRole('tabpanel'); + expect(tabPanels).toHaveLength(2); + }); + + it('should render a TabPane and DashboardGrid for first Tab', async () => { + const { findByTestId } = setup({ + dashboardLayout: undoableDashboardLayoutWithTabs, + }); + const parentSize = await findByTestId('grid-container'); + const expectedCount = + undoableDashboardLayoutWithTabs.present.TABS_ID.children.length; + const tabPanels = within(parentSize).getAllByRole('tabpanel', { + // to include invisiable tab panels + hidden: true, + }); + expect(tabPanels).toHaveLength(expectedCount); + expect( + within(tabPanels[0]).getAllByTestId('mock-dashboard-grid'), + ).toHaveLength(1); + }); + + it('should render a TabPane and DashboardGrid for second Tab', async () => { + const { findByTestId } = setup({ + dashboardLayout: undoableDashboardLayoutWithTabs, + dashboardState: { + ...mockState.dashboardState, + directPathToChild: [DASHBOARD_ROOT_ID, 'TABS_ID', 'TAB_ID2'], + }, + }); + const parentSize = await findByTestId('grid-container'); + const expectedCount = + undoableDashboardLayoutWithTabs.present.TABS_ID.children.length; + const tabPanels = within(parentSize).getAllByRole('tabpanel', { + // to include invisiable tab panels + hidden: true, + }); + expect(tabPanels).toHaveLength(expectedCount); + expect( + within(tabPanels[1]).getAllByTestId('mock-dashboard-grid'), + ).toHaveLength(1); + }); + + it('should render a BuilderComponentPane if editMode=false and user selects "Insert Components" pane', () => { + const { queryAllByTestId } = setup(); + const builderComponents = queryAllByTestId('mock-builder-component-pane'); + expect(builderComponents).toHaveLength(0); + }); + + it('should render a BuilderComponentPane if editMode=true and user selects "Insert Components" pane', () => { + const { queryAllByTestId } = setup({ dashboardState: { editMode: true } }); + const builderComponents = queryAllByTestId('mock-builder-component-pane'); + expect(builderComponents.length).toBeGreaterThanOrEqual(1); + }); + + it('should change redux state if a top-level Tab is clicked', async () => { + (setDirectPathToChild as jest.Mock).mockImplementation(arg0 => ({ + type: 'type', + arg0, + })); + const { findByRole } = setup( + { + dashboardLayout: undoableDashboardLayoutWithTabs, + }, + mockStoreWithTabs, + ); + const tabList = await findByRole('tablist'); + const tabs = within(tabList).getAllByRole('tab'); + expect(setDirectPathToChild).toHaveBeenCalledTimes(0); + fireEvent.click(tabs[1]); + expect(setDirectPathToChild).toHaveBeenCalledWith([ + 'ROOT_ID', + 'TABS_ID', + 'TAB_ID2', + ]); + (setDirectPathToChild as jest.Mock).mockReset(); + }); + + describe('when nativeFiltersEnabled', () => { + beforeEach(() => { + (isFeatureEnabled as jest.Mock).mockImplementation( + flag => flag === FeatureFlag.DASHBOARD_NATIVE_FILTERS, + ); + }); + afterEach(() => { + (isFeatureEnabled as jest.Mock).mockReset(); + }); + + it('should set FilterBar width by useStoredFilterBarWidth', () => { + const expectedValue = 200; + const setter = jest.fn(); + (useStoredFilterBarWidth as jest.Mock).mockImplementation(() => [ + expectedValue, + setter, + ]); + const { getByTestId } = setup({ + dashboardInfo: { + ...mockState.dashboardInfo, + dash_edit_perm: true, + metadata: { show_native_filters: true }, + }, + }); + const filterbar = getByTestId('dashboard-filters-panel'); + expect(filterbar).toHaveStyleRule('width', `${expectedValue}px`); + }); + }); +}); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 8352482ed8ab8..7ff6432d79451 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -26,6 +26,7 @@ import React, { useMemo, useRef, } from 'react'; +import { Resizable } from 're-resizable'; import { JsonObject, styled, css, t } from '@superset-ui/core'; import { Global } from '@emotion/react'; import { useDispatch, useSelector } from 'react-redux'; @@ -68,10 +69,12 @@ import { FILTER_BAR_TABS_HEIGHT, MAIN_HEADER_HEIGHT, OPEN_FILTER_BAR_WIDTH, + OPEN_FILTER_BAR_MAX_WIDTH, } from 'src/dashboard/constants'; import { shouldFocusTabs, getRootLevelTabsComponent } from './utils'; import DashboardContainer from './DashboardContainer'; import { useNativeFilters } from './state'; +import useStoredFilterBarWidth from './useStoredFilterBarWidth'; type DashboardBuilderProps = {}; @@ -125,10 +128,11 @@ const StyledDiv = styled.div` `; // @z-index-above-dashboard-charts + 1 = 11 -const FiltersPanel = styled.div` +const FiltersPanel = styled.div<{ width: number }>` grid-column: 1; grid-row: 1 / span 2; z-index: 11; + width: ${({ width }) => width}px; `; const StickyPanel = styled.div<{ width: number }>` @@ -215,10 +219,34 @@ const StyledDashboardContent = styled.div<{ } `; +const ResizableFilterBarWrapper = styled.div` + position: absolute; + + :hover .filterbar-resizer::after { + background-color: ${({ theme }) => theme.colors.primary.base}; + } + + .filterbar-resizer { + // @z-index-above-sticky-header (100) + 1 = 101 + z-index: 101; + } + + .filterbar-resizer::after { + display: block; + content: ''; + width: 1px; + height: 100%; + margin: 0 auto; + } +`; + const DashboardBuilder: FC = () => { const dispatch = useDispatch(); const uiConfig = useUiConfig(); + const dashboardId = useSelector( + ({ dashboardInfo }) => `${dashboardInfo.id}`, + ); const dashboardLayout = useSelector( state => state.dashboardLayout.present, ); @@ -298,8 +326,11 @@ const DashboardBuilder: FC = () => { nativeFiltersEnabled, } = useNativeFilters(); + const [adjustedFilterBarWidth, setAdjustedFilterBarWidth] = + useStoredFilterBarWidth(dashboardId); + const filterBarWidth = dashboardFiltersOpen - ? OPEN_FILTER_BAR_WIDTH + ? adjustedFilterBarWidth : CLOSED_FILTER_BAR_WIDTH; const [containerRef, isSticky] = useElementOnScreen({ @@ -392,20 +423,37 @@ const DashboardBuilder: FC = () => { return ( {nativeFiltersEnabled && !editMode && ( - - - - - - - + <> + + + setAdjustedFilterBarWidth(filterBarWidth + d.width) + } + /> + + + + + + + + + )} {/* @ts-ignore */} diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.test.ts b/superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.test.ts new file mode 100644 index 0000000000000..1a9da6658a720 --- /dev/null +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.test.ts @@ -0,0 +1,85 @@ +/** + * 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 { renderHook, act } from '@testing-library/react-hooks'; +import { + LocalStorageKeys, + setItem, + getItem, +} from 'src/utils/localStorageHelpers'; +import { OPEN_FILTER_BAR_WIDTH } from 'src/dashboard/constants'; +import useStoredFilterBarWidth from './useStoredFilterBarWidth'; + +describe('useStoredFilterBarWidth', () => { + beforeEach(() => { + localStorage.clear(); + }); + + afterAll(() => { + localStorage.clear(); + }); + + it('returns a default filterBar width by OPEN_FILTER_BAR_WIDTH', () => { + const dashboardId = '123'; + const { result } = renderHook(() => useStoredFilterBarWidth(dashboardId)); + const [actualWidth] = result.current; + + expect(actualWidth).toEqual(OPEN_FILTER_BAR_WIDTH); + }); + + it('returns a stored filterBar width from localStorage', () => { + const dashboardId = '123'; + const expectedWidth = 378; + setItem(LocalStorageKeys.dashboard__custom_filter_bar_widths, { + [dashboardId]: expectedWidth, + '456': 250, + }); + const { result } = renderHook(() => useStoredFilterBarWidth(dashboardId)); + const [actualWidth] = result.current; + + expect(actualWidth).toEqual(expectedWidth); + expect(actualWidth).not.toEqual(250); + }); + + it('returns a setter for filterBar width that stores the state in localStorage together', () => { + const dashboardId = '123'; + const expectedWidth = 378; + const otherDashboardId = '456'; + const otherDashboardWidth = 253; + setItem(LocalStorageKeys.dashboard__custom_filter_bar_widths, { + [dashboardId]: 300, + [otherDashboardId]: otherDashboardWidth, + }); + const { result } = renderHook(() => useStoredFilterBarWidth(dashboardId)); + const [prevWidth, setter] = result.current; + + expect(prevWidth).toEqual(300); + + act(() => setter(expectedWidth)); + + const updatedWidth = result.current[0]; + const widthsMap = getItem( + LocalStorageKeys.dashboard__custom_filter_bar_widths, + {}, + ); + expect(widthsMap[dashboardId]).toEqual(expectedWidth); + expect(widthsMap[otherDashboardId]).toEqual(otherDashboardWidth); + expect(updatedWidth).toEqual(expectedWidth); + expect(updatedWidth).not.toEqual(250); + }); +}); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.ts b/superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.ts new file mode 100644 index 0000000000000..93121b1d8afa8 --- /dev/null +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.ts @@ -0,0 +1,52 @@ +/** + * 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 { useEffect, useRef, useState } from 'react'; +import { + LocalStorageKeys, + setItem, + getItem, +} from 'src/utils/localStorageHelpers'; +import { + OPEN_FILTER_BAR_WIDTH, +} from 'src/dashboard/constants'; + +export default function useStoredFilterBarWidth(dashboardId: string) { + const widthsMapRef = useRef>(); + const [filterBarWidth, setFilterBarWidth] = useState(OPEN_FILTER_BAR_WIDTH); + + useEffect(() => { + widthsMapRef.current = widthsMapRef.current ?? getItem( + LocalStorageKeys.dashboard__custom_filter_bar_widths, + {}, + ); + if (widthsMapRef.current[dashboardId]) { + setFilterBarWidth(widthsMapRef.current[dashboardId]); + } + }, [dashboardId]); + + function setStoredFilterBarWidth(updatedWidth: number) { + setFilterBarWidth(updatedWidth); + setItem(LocalStorageKeys.dashboard__custom_filter_bar_widths, { + ...widthsMapRef.current, + [dashboardId]: updatedWidth, + }); + } + + return [filterBarWidth, setStoredFilterBarWidth] as const; +} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/ActionButtons.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/ActionButtons.test.tsx index 77aede5be3534..3c3f838c4ab08 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/ActionButtons.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/ActionButtons.test.tsx @@ -17,6 +17,7 @@ * under the License. */ import React from 'react'; +import { OPEN_FILTER_BAR_WIDTH } from 'src/dashboard/constants'; import userEvent from '@testing-library/user-event'; import { render, screen } from 'spec/helpers/testing-library'; import { ActionButtons } from './index'; @@ -77,3 +78,28 @@ test('should apply', () => { userEvent.click(applyBtn); expect(mockedProps.onApply).toHaveBeenCalled(); }); + +describe('custom width', () => { + it('sets its default width with OPEN_FILTER_BAR_WIDTH', () => { + const mockedProps = createProps(); + render(, { useRedux: true }); + const container = screen.getByTestId('filterbar-action-buttons'); + expect(container).toHaveStyleRule( + 'width', + `${OPEN_FILTER_BAR_WIDTH - 1}px`, + ); + }); + + it('sets custom width', () => { + const mockedProps = createProps(); + const expectedWidth = 423; + const { getByTestId } = render( + , + { + useRedux: true, + }, + ); + const container = getByTestId('filterbar-action-buttons'); + expect(container).toHaveStyleRule('width', `${expectedWidth - 1}px`); + }); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx index d94885b1c970e..da8ec83702547 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx @@ -31,6 +31,7 @@ import { rgba } from 'emotion-rgba'; import { getFilterBarTestId } from '../index'; interface ActionButtonsProps { + width?: number; onApply: () => void; onClearAll: () => void; dataMaskSelected: DataMaskState; @@ -38,8 +39,8 @@ interface ActionButtonsProps { isApplyDisabled: boolean; } -const ActionButtonsContainer = styled.div` - ${({ theme }) => css` +const ActionButtonsContainer = styled.div<{ width: number }>` + ${({ theme, width }) => css` display: flex; flex-direction: column; align-items: center; @@ -48,7 +49,7 @@ const ActionButtonsContainer = styled.div` z-index: 100; // filter bar width minus 1px for border - width: ${OPEN_FILTER_BAR_WIDTH - 1}px; + width: ${width - 1}px; bottom: 0; padding: ${theme.gridUnit * 4}px; @@ -85,6 +86,7 @@ const ActionButtonsContainer = styled.div` `; export const ActionButtons = ({ + width, onApply, onClearAll, dataMaskApplied, @@ -103,7 +105,7 @@ export const ActionButtons = ({ ); return ( - +
)} ; + dashboard__custom_filter_bar_widths: Record; }; /* From bf1d0681809c0c596ab0950f7a6b3f1c17b55775 Mon Sep 17 00:00:00 2001 From: justin-park Date: Tue, 19 Jul 2022 14:30:44 -0700 Subject: [PATCH 2/9] replace to testing-library/react --- .../components/DashboardBuilder/DashboardBuilder.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx index 2ffe9018c10de..d21ef5ba843d6 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx @@ -20,7 +20,7 @@ import { Store } from 'redux'; import React from 'react'; import fetchMock from 'fetch-mock'; import { render } from 'spec/helpers/testing-library'; -import { fireEvent, within } from '@testing-library/dom'; +import { fireEvent, within } from '@testing-library/react'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import DashboardBuilder from 'src/dashboard/components/DashboardBuilder/DashboardBuilder'; import useStoredFilterBarWidth from 'src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth'; From 43d77a870611bc901623e428f5555f4dfb9dac22 Mon Sep 17 00:00:00 2001 From: justin-park Date: Tue, 19 Jul 2022 14:40:49 -0700 Subject: [PATCH 3/9] test lint --- .../DashboardBuilder/DashboardBuilder.test.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx index d21ef5ba843d6..99b157ec46318 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx @@ -126,7 +126,7 @@ describe('DashboardBuilder', () => { it('should render a StickyContainer with class "dashboard"', () => { const { getByTestId } = setup(); const stickyContainer = getByTestId('dashboard-content'); - expect(stickyContainer.className).toBe('dashboard'); + expect(stickyContainer).toHaveClass('dashboard'); }); it('should add the "dashboard--editing" class if editMode=true', () => { @@ -134,7 +134,7 @@ describe('DashboardBuilder', () => { dashboardState: { ...mockState.dashboardState, editMode: true }, }); const stickyContainer = getByTestId('dashboard-content'); - expect(stickyContainer.className).toBe('dashboard dashboard--editing'); + expect(stickyContainer).toHaveClass('dashboard dashboard--editing'); }); it('should render a DragDroppable DashboardHeader', () => { @@ -151,14 +151,18 @@ describe('DashboardBuilder', () => { const sticky = await findAllByTestId('nav-list'); expect(sticky).toHaveLength(1); - expect(sticky[0].getAttribute('id')).toContain('TABS_ID'); + expect(sticky[0]).toHaveAttribute('id', 'TABS_ID'); const dashboardTabComponents = within(sticky[0]).getAllByRole('tab'); const tabChildren = undoableDashboardLayoutWithTabs.present.TABS_ID.children; expect(dashboardTabComponents).toHaveLength(tabChildren.length); tabChildren.forEach((tabId, i) => { - expect(dashboardTabComponents[i].getAttribute('id')).toContain(tabId); + const idMatcher = new RegExp(tabId + '$'); + expect(dashboardTabComponents[i]).toHaveAttribute( + 'id', + expect.stringMatching(idMatcher), + ); }); }); From 843c46310dd687ef2253ee5a323ac890a72d736a Mon Sep 17 00:00:00 2001 From: justin-park Date: Tue, 19 Jul 2022 14:44:31 -0700 Subject: [PATCH 4/9] replace toHaveLength() by length and toBe --- .../DashboardBuilder.test.tsx | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx index 99b157ec46318..78f6f56876bdd 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx @@ -150,13 +150,13 @@ describe('DashboardBuilder', () => { ); const sticky = await findAllByTestId('nav-list'); - expect(sticky).toHaveLength(1); + expect(sticky.length).toBe(1); expect(sticky[0]).toHaveAttribute('id', 'TABS_ID'); const dashboardTabComponents = within(sticky[0]).getAllByRole('tab'); const tabChildren = undoableDashboardLayoutWithTabs.present.TABS_ID.children; - expect(dashboardTabComponents).toHaveLength(tabChildren.length); + expect(dashboardTabComponents.length).toBe(tabChildren.length); tabChildren.forEach((tabId, i) => { const idMatcher = new RegExp(tabId + '$'); expect(dashboardTabComponents[i]).toHaveAttribute( @@ -171,9 +171,9 @@ describe('DashboardBuilder', () => { dashboardLayout: undoableDashboardLayoutWithTabs, }); const tabs = await findAllByRole('tablist'); - expect(tabs).toHaveLength(1); + expect(tabs.length).toBe(1); const tabPanels = await findAllByRole('tabpanel'); - expect(tabPanels).toHaveLength(2); + expect(tabPanels.length).toBe(2); }); it('should render a TabPane and DashboardGrid for first Tab', async () => { @@ -187,10 +187,10 @@ describe('DashboardBuilder', () => { // to include invisiable tab panels hidden: true, }); - expect(tabPanels).toHaveLength(expectedCount); + expect(tabPanels.length).toBe(expectedCount); expect( - within(tabPanels[0]).getAllByTestId('mock-dashboard-grid'), - ).toHaveLength(1); + within(tabPanels[0]).getAllByTestId('mock-dashboard-grid').length, + ).toBe(1); }); it('should render a TabPane and DashboardGrid for second Tab', async () => { @@ -208,16 +208,16 @@ describe('DashboardBuilder', () => { // to include invisiable tab panels hidden: true, }); - expect(tabPanels).toHaveLength(expectedCount); + expect(tabPanels.length).toBe(expectedCount); expect( - within(tabPanels[1]).getAllByTestId('mock-dashboard-grid'), - ).toHaveLength(1); + within(tabPanels[1]).getAllByTestId('mock-dashboard-grid').length, + ).toBe(1); }); it('should render a BuilderComponentPane if editMode=false and user selects "Insert Components" pane', () => { const { queryAllByTestId } = setup(); const builderComponents = queryAllByTestId('mock-builder-component-pane'); - expect(builderComponents).toHaveLength(0); + expect(builderComponents.length).toBe(0); }); it('should render a BuilderComponentPane if editMode=true and user selects "Insert Components" pane', () => { From 3ca5ca267ef37effe10e277df0a267dcf544ac62 Mon Sep 17 00:00:00 2001 From: justin-park Date: Tue, 19 Jul 2022 14:45:49 -0700 Subject: [PATCH 5/9] prettier --- .../DashboardBuilder/useStoredFilterBarWidth.ts | 15 +++++++-------- .../FilterBar/ActionButtons/index.tsx | 5 ++++- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.ts b/superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.ts index 93121b1d8afa8..0cbdc74182222 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.ts +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/useStoredFilterBarWidth.ts @@ -22,19 +22,18 @@ import { setItem, getItem, } from 'src/utils/localStorageHelpers'; -import { - OPEN_FILTER_BAR_WIDTH, -} from 'src/dashboard/constants'; +import { OPEN_FILTER_BAR_WIDTH } from 'src/dashboard/constants'; export default function useStoredFilterBarWidth(dashboardId: string) { const widthsMapRef = useRef>(); - const [filterBarWidth, setFilterBarWidth] = useState(OPEN_FILTER_BAR_WIDTH); + const [filterBarWidth, setFilterBarWidth] = useState( + OPEN_FILTER_BAR_WIDTH, + ); useEffect(() => { - widthsMapRef.current = widthsMapRef.current ?? getItem( - LocalStorageKeys.dashboard__custom_filter_bar_widths, - {}, - ); + widthsMapRef.current = + widthsMapRef.current ?? + getItem(LocalStorageKeys.dashboard__custom_filter_bar_widths, {}); if (widthsMapRef.current[dashboardId]) { setFilterBarWidth(widthsMapRef.current[dashboardId]); } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx index da8ec83702547..9a1507d511c9d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx @@ -105,7 +105,10 @@ export const ActionButtons = ({ ); return ( - +