From 95b43238a04e2b5c27bd6ab45dccfc23fab624a2 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Thu, 27 Oct 2022 08:58:00 -0600 Subject: [PATCH] fix(dashboard): Remove bar at bottom of dashboard edit sidebar (#21807) --- superset-frontend/package-lock.json | 42 ----- superset-frontend/package.json | 2 - .../BuilderComponentPane.test.tsx | 2 +- .../components/BuilderComponentPane/index.tsx | 158 +++++++----------- .../DashboardBuilder/DashboardBuilder.tsx | 7 +- .../src/dashboard/components/SliceAdder.jsx | 59 ++++--- .../dashboard/components/SliceAdder.test.jsx | 12 +- .../stylesheets/builder-sidepane.less | 132 --------------- .../src/dashboard/stylesheets/index.less | 1 - 9 files changed, 107 insertions(+), 308 deletions(-) delete mode 100644 superset-frontend/src/dashboard/stylesheets/builder-sidepane.less diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 1782a2515..4174d104a 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -121,7 +121,6 @@ "react-select": "^3.1.0", "react-sortable-hoc": "^1.11.0", "react-split": "^2.0.9", - "react-sticky": "^6.0.3", "react-syntax-highlighter": "^15.4.5", "react-table": "^7.6.3", "react-transition-group": "^2.5.3", @@ -194,7 +193,6 @@ "@types/react-redux": "^7.1.10", "@types/react-router-dom": "^5.1.5", "@types/react-select": "^3.0.19", - "@types/react-sticky": "^6.0.3", "@types/react-table": "^7.0.19", "@types/react-ultimate-pagination": "^1.2.0", "@types/react-virtualized": "^9.21.10", @@ -18449,15 +18447,6 @@ "@types/react-transition-group": "*" } }, - "node_modules/@types/react-sticky": { - "version": "6.0.3", - "resolved": "https://registry.npmjs.org/@types/react-sticky/-/react-sticky-6.0.3.tgz", - "integrity": "sha512-tW0Y1hTr2Tao4yX58iKl0i7BaqrdObGXAzsyzd8VGVrWVEgbQuV6P6QKVd/kFC7FroXyelftiVNJ09pnfkcjww==", - "dev": true, - "dependencies": { - "@types/react": "*" - } - }, "node_modules/@types/react-syntax-highlighter": { "version": "11.0.5", "resolved": "https://registry.npmjs.org/@types/react-syntax-highlighter/-/react-syntax-highlighter-11.0.5.tgz", @@ -45551,19 +45540,6 @@ "resolved": "https://registry.npmjs.org/split.js/-/split.js-1.6.2.tgz", "integrity": "sha512-72C7zcQePzlmWqPOKkB2Ro0sUmnWSx+qEWXjLJKk6Qp4jAkFRz1hJgJb+ay6ZQyz/Aw9r8N/PZiCEKbPVpFoDQ==" }, - "node_modules/react-sticky": { - "version": "6.0.3", - "resolved": "https://registry.npmjs.org/react-sticky/-/react-sticky-6.0.3.tgz", - "integrity": "sha512-LNH4UJlRatOqo29/VHxDZOf6fwbgfgcHO4mkEFvrie5FuaZCSTGtug5R8NGqJ0kSnX8gHw8qZN37FcvnFBJpTQ==", - "dependencies": { - "prop-types": "^15.5.8", - "raf": "^3.3.0" - }, - "peerDependencies": { - "react": ">=15", - "react-dom": ">=15" - } - }, "node_modules/react-style-proptype": { "version": "3.2.2", "resolved": "https://registry.npmjs.org/react-style-proptype/-/react-style-proptype-3.2.2.tgz", @@ -71323,15 +71299,6 @@ "@types/react-transition-group": "*" } }, - "@types/react-sticky": { - "version": "6.0.3", - "resolved": "https://registry.npmjs.org/@types/react-sticky/-/react-sticky-6.0.3.tgz", - "integrity": "sha512-tW0Y1hTr2Tao4yX58iKl0i7BaqrdObGXAzsyzd8VGVrWVEgbQuV6P6QKVd/kFC7FroXyelftiVNJ09pnfkcjww==", - "dev": true, - "requires": { - "@types/react": "*" - } - }, "@types/react-syntax-highlighter": { "version": "11.0.5", "resolved": "https://registry.npmjs.org/@types/react-syntax-highlighter/-/react-syntax-highlighter-11.0.5.tgz", @@ -92432,15 +92399,6 @@ "react-style-proptype": "^3.2.2" } }, - "react-sticky": { - "version": "6.0.3", - "resolved": "https://registry.npmjs.org/react-sticky/-/react-sticky-6.0.3.tgz", - "integrity": "sha512-LNH4UJlRatOqo29/VHxDZOf6fwbgfgcHO4mkEFvrie5FuaZCSTGtug5R8NGqJ0kSnX8gHw8qZN37FcvnFBJpTQ==", - "requires": { - "prop-types": "^15.5.8", - "raf": "^3.3.0" - } - }, "react-style-proptype": { "version": "3.2.2", "resolved": "https://registry.npmjs.org/react-style-proptype/-/react-style-proptype-3.2.2.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 5d09c502d..b78caa278 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -185,7 +185,6 @@ "react-select": "^3.1.0", "react-sortable-hoc": "^1.11.0", "react-split": "^2.0.9", - "react-sticky": "^6.0.3", "react-syntax-highlighter": "^15.4.5", "react-table": "^7.6.3", "react-transition-group": "^2.5.3", @@ -258,7 +257,6 @@ "@types/react-redux": "^7.1.10", "@types/react-router-dom": "^5.1.5", "@types/react-select": "^3.0.19", - "@types/react-sticky": "^6.0.3", "@types/react-table": "^7.0.19", "@types/react-ultimate-pagination": "^1.2.0", "@types/react-virtualized": "^9.21.10", diff --git a/superset-frontend/src/dashboard/components/BuilderComponentPane/BuilderComponentPane.test.tsx b/superset-frontend/src/dashboard/components/BuilderComponentPane/BuilderComponentPane.test.tsx index a5ff5b131..be2b429f5 100644 --- a/superset-frontend/src/dashboard/components/BuilderComponentPane/BuilderComponentPane.test.tsx +++ b/superset-frontend/src/dashboard/components/BuilderComponentPane/BuilderComponentPane.test.tsx @@ -24,7 +24,7 @@ import BuilderComponentPane from '.'; jest.mock('src/dashboard/containers/SliceAdder'); test('BuilderComponentPane has correct tabs in correct order', () => { - render(); + render(); const tabs = screen.getAllByRole('tab'); expect(tabs).toHaveLength(2); expect(tabs[0]).toHaveTextContent('Charts'); diff --git a/superset-frontend/src/dashboard/components/BuilderComponentPane/index.tsx b/superset-frontend/src/dashboard/components/BuilderComponentPane/index.tsx index 39e7fd4bc..d9b34cfed 100644 --- a/superset-frontend/src/dashboard/components/BuilderComponentPane/index.tsx +++ b/superset-frontend/src/dashboard/components/BuilderComponentPane/index.tsx @@ -18,12 +18,9 @@ */ /* eslint-env browser */ import React from 'react'; +import { rgba } from 'emotion-rgba'; import Tabs from 'src/components/Tabs'; -import { StickyContainer, Sticky } from 'react-sticky'; -import { ParentSize } from '@vx/responsive'; - -import { t, styled } from '@superset-ui/core'; - +import { t, css, SupersetTheme } from '@superset-ui/core'; import SliceAdder from 'src/dashboard/containers/SliceAdder'; import dashboardComponents from 'src/visualizations/presets/dashboardComponents'; import NewColumn from '../gridComponents/new/NewColumn'; @@ -34,99 +31,72 @@ import NewTabs from '../gridComponents/new/NewTabs'; import NewMarkdown from '../gridComponents/new/NewMarkdown'; import NewDynamicComponent from '../gridComponents/new/NewDynamicComponent'; -export interface BCPProps { - isStandalone: boolean; - topOffset: number; -} +const BUILDER_PANE_WIDTH = 374; -const SUPERSET_HEADER_HEIGHT = 59; -const SIDEPANE_ADJUST_OFFSET = 4; -const TOP_PANEL_OFFSET = 210; - -const BuilderComponentPaneTabs = styled(Tabs)` - line-height: inherit; - margin-top: ${({ theme }) => theme.gridUnit * 2}px; -`; - -const DashboardBuilderSidepane = styled.div<{ - topOffset: number; -}>` - height: calc(100% - ${TOP_PANEL_OFFSET}px); - position: fixed; - right: 0; - top: 0; -`; - -const BuilderComponentPane: React.FC = ({ - isStandalone, - topOffset = 0, -}) => ( - ( +
- - {({ height }) => ( - - - {({ style, isSticky }: { style: any; isSticky: boolean }) => { - const { pageYOffset } = window; - const hasHeader = - pageYOffset < SUPERSET_HEADER_HEIGHT && !isStandalone; - const withHeaderTopOffset = - topOffset + - (SUPERSET_HEADER_HEIGHT - pageYOffset - SIDEPANE_ADJUST_OFFSET); +
css` + position: absolute; + height: 100%; + width: ${BUILDER_PANE_WIDTH}px; + box-shadow: -4px 0 4px 0 ${rgba(theme.colors.grayscale.dark2, 0.1)}; + background-color: ${theme.colors.grayscale.light5}; + `} + > + css` + line-height: inherit; + margin-top: ${theme.gridUnit * 2}px; + height: 100%; - return ( -
- - - - - - - - - - - - {dashboardComponents - .getAll() - .map(({ key: componentKey, metadata }) => ( - - ))} - - -
- ); - }} - - - )} - - + & .ant-tabs-content-holder { + height: 100%; + & .ant-tabs-content { + height: 100%; + } + } + `} + > + + + + + + + + + + + {dashboardComponents + .getAll() + .map(({ key: componentKey, metadata }) => ( + + ))} + + +
+
); export default BuilderComponentPane; diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index c99d1d866..cd0cfdb3f 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -487,12 +487,7 @@ const DashboardBuilder: FC = () => { ) : ( )} - {editMode && ( - - )} + {editMode && } diff --git a/superset-frontend/src/dashboard/components/SliceAdder.jsx b/superset-frontend/src/dashboard/components/SliceAdder.jsx index 8a6368f38..7f8792dca 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.jsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.jsx @@ -19,7 +19,7 @@ /* eslint-env browser */ import React from 'react'; import PropTypes from 'prop-types'; -import { List } from 'react-virtualized'; +import { List, AutoSizer } from 'react-virtualized'; import { createFilter } from 'react-search-input'; import { t, @@ -57,7 +57,6 @@ const propTypes = { userId: PropTypes.string.isRequired, selectedSliceIds: PropTypes.arrayOf(PropTypes.number), editMode: PropTypes.bool, - height: PropTypes.number, filterboxMigrationState: FILTER_BOX_MIGRATION_STATES, dashboardId: PropTypes.number, }; @@ -66,7 +65,6 @@ const defaultProps = { selectedSliceIds: [], editMode: false, errorMessage: '', - height: window.innerHeight, filterboxMigrationState: FILTER_BOX_MIGRATION_STATES.NOOP, }; @@ -80,9 +78,6 @@ const KEYS_TO_SORT = { const DEFAULT_SORT_KEY = 'changed_on'; -const MARGIN_BOTTOM = 16; -const SIDEPANE_HEADER_HEIGHT = 30; -const SLICE_ADDER_CONTROL_HEIGHT = 64; const DEFAULT_CELL_HEIGHT = 128; const Controls = styled.div` @@ -119,6 +114,11 @@ const NewChartButton = styled(Button)` `} `; +export const ChartList = styled.div` + flex-grow: 1; + min-height: 0; +`; + class SliceAdder extends React.Component { static sortByComparator(attr) { const desc = attr === 'changed_on' ? -1 : 1; @@ -282,13 +282,14 @@ class SliceAdder extends React.Component { } render() { - const slicesListHeight = - this.props.height - - SIDEPANE_HEADER_HEIGHT - - SLICE_ADDER_CONTROL_HEIGHT - - MARGIN_BOTTOM; return ( -
+
{this.props.isLoading && } {!this.props.isLoading && this.state.filteredSlices.length > 0 && ( - + + + {({ height, width }) => ( + + )} + + )} {this.props.errorMessage && ( -
{this.props.errorMessage}
+
+ {this.props.errorMessage} +
)} {/* Drag preview is just a single fixed-position element */} diff --git a/superset-frontend/src/dashboard/components/SliceAdder.test.jsx b/superset-frontend/src/dashboard/components/SliceAdder.test.jsx index 7bfd971cb..72327d63c 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.test.jsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.test.jsx @@ -20,10 +20,9 @@ import React from 'react'; import { shallow } from 'enzyme'; import sinon from 'sinon'; -import { List } from 'react-virtualized'; - -import SliceAdder from 'src/dashboard/components/SliceAdder'; +import SliceAdder, { ChartList } from 'src/dashboard/components/SliceAdder'; import { sliceEntitiesForDashboard as mockSliceEntities } from 'spec/fixtures/mockSliceEntities'; +import { styledShallow } from 'spec/helpers/theming'; describe('SliceAdder', () => { const mockEvent = { @@ -39,7 +38,6 @@ describe('SliceAdder', () => { fetchSortedSlices: () => {}, selectedSliceIds: [127, 128], userId: '1', - height: 100, }; const errorProps = { ...props, @@ -72,10 +70,10 @@ describe('SliceAdder', () => { }); }); - it('render List', () => { - const wrapper = shallow(); + it('render chart list', () => { + const wrapper = styledShallow(); wrapper.setState({ filteredSlices: Object.values(props.slices) }); - expect(wrapper.find(List)).toExist(); + expect(wrapper.find(ChartList)).toExist(); }); it('render error', () => { diff --git a/superset-frontend/src/dashboard/stylesheets/builder-sidepane.less b/superset-frontend/src/dashboard/stylesheets/builder-sidepane.less deleted file mode 100644 index ad2687777..000000000 --- a/superset-frontend/src/dashboard/stylesheets/builder-sidepane.less +++ /dev/null @@ -1,132 +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 '../../assets/stylesheets/less/variables.less'; - -.dashboard-builder-sidepane { - .dashboard-builder-sidepane-header { - font-size: @font-size-l; - font-weight: @font-weight-bold; - border-top: 1px solid @gray-light; - border-bottom: 1px solid @gray-light; - padding: 16px; - display: flex; - align-items: center; - } - - .trigger { - font-size: @font-size-l; - color: @almost-black; - opacity: 1; - margin-left: auto; - cursor: pointer; - } - - .slices-layer .trigger { - margin-left: 0; - margin-right: 20px; - } - - .viewport { - position: absolute; - transform: none !important; - overflow: hidden; - width: @builder-pane-width; - height: 100%; - box-shadow: -4px 0 4px 0 fade(@darkest, @opacity-light); - background-color: @lightest; - } - - .slider-container { - position: absolute; - background: @lightest; - width: @builder-pane-width * 2; - height: 100vh; - display: flex; - transition: all @timing-normal ease; - - &.slide-in { - left: -@builder-pane-width; - } - - &.slide-out { - left: 0; - } - - .slide-content { - width: @builder-pane-width; - } - } - - .component-layer .new-component.static, - .slices-layer .dashboard-builder-sidepane-header { - cursor: pointer; - } - - .component-layer { - .new-component.static { - cursor: pointer; - } - } - - .new-component-label { - flex-grow: 1; - } - - .slice-adder-container { - position: relative; - background-color: white; - min-height: 200px; /* for loader positioning */ - - .error-message { - padding: 16px; - } - - .controls { - display: flex; - padding: 16px; - - /* the input is wrapped in a div */ - .search-input { - flex-grow: 1; - margin-right: 16px; - } - - .dropdown.btn-group button, - input { - font-size: @font-size-m; - padding: 7px 12px; - height: 32px; - border: 1px solid @gray-light; - } - - input { - width: 100%; - - &:focus { - outline: none; - border-color: @gray; - } - } - } - - .ReactVirtualized__Grid.ReactVirtualized__List:focus { - outline: none; - } - } -} diff --git a/superset-frontend/src/dashboard/stylesheets/index.less b/superset-frontend/src/dashboard/stylesheets/index.less index ba46688df..0b11c2bec 100644 --- a/superset-frontend/src/dashboard/stylesheets/index.less +++ b/superset-frontend/src/dashboard/stylesheets/index.less @@ -19,7 +19,6 @@ @import '../../assets/stylesheets/less/variables.less'; @import './builder.less'; -@import './builder-sidepane.less'; @import './dashboard.less'; @import './dnd.less'; @import './filter-scope-selector.less';