etr2460 commented on a change in pull request #12008: URL: https://github.com/apache/incubator-superset/pull/12008#discussion_r544456538
########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; Review comment: `actions` sounds like either an array or an object, can we do better than `any` here? ########## File path: superset-frontend/src/explore/components/ControlPanelsContainer.jsx ########## @@ -159,7 +159,11 @@ class ControlPanelsContainer extends React.Component { // When the item is a React element return controlItem; } - if (controlItem.name && controlItem.config) { + if ( + controlItem.name && + controlItem.config && + controlItem.name !== 'datasource' Review comment: Wouldn't it be better to not pass in the datasource control item here? Instead of filtering it out in the render logic? ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + } + .ant-collapse-item.ant-collapse-item-active { + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + } + .header { + font-size: ${({ theme }) => theme.typography.sizes.l}px; + margin-left: ${({ theme }) => theme.gridUnit * -2}px; + } + .ant-collapse-content-box > div { + margin-left: -14px; + } + .type-label { + text-align: left; + } + .metric-option .option-label { + margin-left: -20px; + } +`; + +const DataSourcePanel = ({ + datasource, + controls: { datasource: datasourceControl }, + actions, +}: Props) => { + const [lists, setColList] = useState({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + const search = ({ target: { value } }: { target: { value: string } }) => { + const columns = datasource.columns.filter( + obj => obj.column_name.indexOf(value) !== -1, + ); + const metrics = lists.metrics.filter( + objs => objs.metric_name.indexOf(value) !== -1, + ); + if (value === '') { + setColList({ columns: datasource.columns, metrics: datasource.metrics }); + } else setColList({ columns, metrics }); + }; + useEffect(() => { + setColList({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + }, [datasource]); + + const Metrics = ({ index }: number) => { Review comment: naming nit: maybe `Metric` or `MetricRenderer`? since this only renders a single metric ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + } + .ant-collapse-item.ant-collapse-item-active { + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + } + .header { + font-size: ${({ theme }) => theme.typography.sizes.l}px; + margin-left: ${({ theme }) => theme.gridUnit * -2}px; + } + .ant-collapse-content-box > div { + margin-left: -14px; + } + .type-label { + text-align: left; + } + .metric-option .option-label { + margin-left: -20px; + } +`; + +const DataSourcePanel = ({ + datasource, + controls: { datasource: datasourceControl }, + actions, +}: Props) => { + const [lists, setColList] = useState({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + const search = ({ target: { value } }: { target: { value: string } }) => { + const columns = datasource.columns.filter( + obj => obj.column_name.indexOf(value) !== -1, + ); + const metrics = lists.metrics.filter( + objs => objs.metric_name.indexOf(value) !== -1, + ); + if (value === '') { + setColList({ columns: datasource.columns, metrics: datasource.metrics }); + } else setColList({ columns, metrics }); + }; + useEffect(() => { + setColList({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + }, [datasource]); + + const Metrics = ({ index }: number) => { + return ( + <div key={lists.metrics[index].metric_name} className="metric"> + <MetricOption metric={lists.metrics[index]} showType /> + </div> + ); + }; + + const Columns = ({ index }: number) => { + return ( + <div key={lists.columns[index].column_name} className="column"> + <ColumnOption column={lists.columns[index]} showType /> + </div> + ); + }; Review comment: you don't need a curly brace here if it's a one line return ########## File path: superset-frontend/src/explore/components/controls/DatasourceControl.jsx ########## @@ -70,19 +73,23 @@ const Styles = styled.div` svg.datasource-modal-trigger { color: ${({ theme }) => theme.colors.primary.base}; - vertical-align: middle; + vertical-align: ${({ theme }) => theme.gridUnit + 2}px; cursor: pointer; } -`; - -/** - * <Col> used in column details. - */ -const ColumnsCol = styled(Col)` - overflow: auto; /* for very very long columns names */ - white-space: nowrap; /* make sure tooltip trigger is on the same line as the metric */ - .and-more { - padding-left: 38px; + .title-select { + width: ${({ theme }) => theme.gridUnit * 52}px; + display: inline-block; + background-color: #f0f0f0; Review comment: can we pull this color from the theme? ########## File path: superset-frontend/src/explore/components/QueryAndSaveBtns.jsx ########## @@ -40,26 +39,13 @@ const defaultProps = { onSave: () => {}, }; -// Prolly need to move this to a global context Review comment: are we completely removing these shortcuts? If so, any reason why? They're probably quite useful for both power users and provide accessibility to folks who prefer keyboards ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + } + .ant-collapse-item.ant-collapse-item-active { + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + } + .header { + font-size: ${({ theme }) => theme.typography.sizes.l}px; + margin-left: ${({ theme }) => theme.gridUnit * -2}px; + } + .ant-collapse-content-box > div { + margin-left: -14px; + } + .type-label { + text-align: left; + } + .metric-option .option-label { + margin-left: -20px; + } +`; + +const DataSourcePanel = ({ + datasource, + controls: { datasource: datasourceControl }, + actions, +}: Props) => { + const [lists, setColList] = useState({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + const search = ({ target: { value } }: { target: { value: string } }) => { + const columns = datasource.columns.filter( + obj => obj.column_name.indexOf(value) !== -1, Review comment: can we use something more descriptive than `obj`? perhaps `column` or `columnObject`? ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; Review comment: I assume we can do better than `any` here? If it's an object then we could at least do `Record<string, any>` or `Record<string, unknown>` (the latter is preferred, but might be harder) ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + } + .ant-collapse-item.ant-collapse-item-active { + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + } + .header { + font-size: ${({ theme }) => theme.typography.sizes.l}px; + margin-left: ${({ theme }) => theme.gridUnit * -2}px; + } + .ant-collapse-content-box > div { + margin-left: -14px; + } + .type-label { + text-align: left; + } + .metric-option .option-label { + margin-left: -20px; + } +`; + +const DataSourcePanel = ({ + datasource, + controls: { datasource: datasourceControl }, + actions, +}: Props) => { + const [lists, setColList] = useState({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + const search = ({ target: { value } }: { target: { value: string } }) => { + const columns = datasource.columns.filter( + obj => obj.column_name.indexOf(value) !== -1, + ); + const metrics = lists.metrics.filter( + objs => objs.metric_name.indexOf(value) !== -1, + ); + if (value === '') { + setColList({ columns: datasource.columns, metrics: datasource.metrics }); + } else setColList({ columns, metrics }); + }; + useEffect(() => { + setColList({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + }, [datasource]); + + const Metrics = ({ index }: number) => { + return ( + <div key={lists.metrics[index].metric_name} className="metric"> + <MetricOption metric={lists.metrics[index]} showType /> + </div> + ); + }; + + const Columns = ({ index }: number) => { + return ( + <div key={lists.columns[index].column_name} className="column"> + <ColumnOption column={lists.columns[index]} showType /> + </div> + ); + }; + return ( + <DatasourceContainer> + <Control + {...datasourceControl} + name="datasource" + validationErrors={datasourceControl.validationErrors} + actions={actions} + formData={datasourceControl.mapStateToProps} + /> + <div className="field-selections"> + <input + type="text" + onChange={search} + className="form-control input-sm" + placeholder={t('Search Metrics & Columns')} + /> + <Collapse + accordion + bordered={false} + defaultActiveKey={['column', 'metrics']} + > + <Collapse.Panel + header={<span className="header">Columns</span>} + key="column" + > + <List + height={100} + itemCount={lists.columns.length} + itemSize={35} + width={250} + > + {Columns} + </List> + </Collapse.Panel> + </Collapse> + <Collapse accordion bordered={false}> + <Collapse.Panel + header={<span className="header">Metrics</span>} Review comment: wrap `Metrics` in a t() string ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + } + .ant-collapse-item.ant-collapse-item-active { + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + } + .header { + font-size: ${({ theme }) => theme.typography.sizes.l}px; + margin-left: ${({ theme }) => theme.gridUnit * -2}px; + } + .ant-collapse-content-box > div { + margin-left: -14px; + } + .type-label { + text-align: left; + } + .metric-option .option-label { + margin-left: -20px; + } +`; + +const DataSourcePanel = ({ + datasource, + controls: { datasource: datasourceControl }, + actions, +}: Props) => { + const [lists, setColList] = useState({ Review comment: naming nit: prefer `setColumnList`, there's no need to abbreviate here ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + } + .ant-collapse-item.ant-collapse-item-active { + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + } + .header { + font-size: ${({ theme }) => theme.typography.sizes.l}px; + margin-left: ${({ theme }) => theme.gridUnit * -2}px; + } + .ant-collapse-content-box > div { + margin-left: -14px; + } + .type-label { + text-align: left; + } + .metric-option .option-label { + margin-left: -20px; + } +`; + +const DataSourcePanel = ({ + datasource, + controls: { datasource: datasourceControl }, + actions, +}: Props) => { + const [lists, setColList] = useState({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + const search = ({ target: { value } }: { target: { value: string } }) => { + const columns = datasource.columns.filter( + obj => obj.column_name.indexOf(value) !== -1, + ); + const metrics = lists.metrics.filter( + objs => objs.metric_name.indexOf(value) !== -1, + ); + if (value === '') { + setColList({ columns: datasource.columns, metrics: datasource.metrics }); + } else setColList({ columns, metrics }); + }; + useEffect(() => { + setColList({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + }, [datasource]); + + const Metrics = ({ index }: number) => { + return ( + <div key={lists.metrics[index].metric_name} className="metric"> + <MetricOption metric={lists.metrics[index]} showType /> + </div> + ); + }; + + const Columns = ({ index }: number) => { + return ( + <div key={lists.columns[index].column_name} className="column"> + <ColumnOption column={lists.columns[index]} showType /> + </div> + ); + }; + return ( + <DatasourceContainer> + <Control + {...datasourceControl} + name="datasource" + validationErrors={datasourceControl.validationErrors} + actions={actions} + formData={datasourceControl.mapStateToProps} + /> + <div className="field-selections"> + <input + type="text" + onChange={search} + className="form-control input-sm" + placeholder={t('Search Metrics & Columns')} + /> + <Collapse + accordion + bordered={false} + defaultActiveKey={['column', 'metrics']} + > + <Collapse.Panel + header={<span className="header">Columns</span>} Review comment: wrap `Columns` in a t() string ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + } + .ant-collapse-item.ant-collapse-item-active { + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + } + .header { + font-size: ${({ theme }) => theme.typography.sizes.l}px; + margin-left: ${({ theme }) => theme.gridUnit * -2}px; + } + .ant-collapse-content-box > div { + margin-left: -14px; + } + .type-label { + text-align: left; + } + .metric-option .option-label { + margin-left: -20px; + } +`; + +const DataSourcePanel = ({ + datasource, + controls: { datasource: datasourceControl }, + actions, +}: Props) => { + const [lists, setColList] = useState({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + const search = ({ target: { value } }: { target: { value: string } }) => { + const columns = datasource.columns.filter( + obj => obj.column_name.indexOf(value) !== -1, + ); + const metrics = lists.metrics.filter( + objs => objs.metric_name.indexOf(value) !== -1, + ); + if (value === '') { + setColList({ columns: datasource.columns, metrics: datasource.metrics }); + } else setColList({ columns, metrics }); + }; + useEffect(() => { + setColList({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + }, [datasource]); + + const Metrics = ({ index }: number) => { + return ( + <div key={lists.metrics[index].metric_name} className="metric"> + <MetricOption metric={lists.metrics[index]} showType /> + </div> + ); + }; Review comment: you don't need a curly brace here if it's a one line return ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + } + .ant-collapse-item.ant-collapse-item-active { + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + } + .header { + font-size: ${({ theme }) => theme.typography.sizes.l}px; + margin-left: ${({ theme }) => theme.gridUnit * -2}px; + } + .ant-collapse-content-box > div { + margin-left: -14px; + } + .type-label { + text-align: left; + } + .metric-option .option-label { + margin-left: -20px; + } +`; + +const DataSourcePanel = ({ + datasource, + controls: { datasource: datasourceControl }, + actions, +}: Props) => { + const [lists, setColList] = useState({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + const search = ({ target: { value } }: { target: { value: string } }) => { + const columns = datasource.columns.filter( + obj => obj.column_name.indexOf(value) !== -1, + ); + const metrics = lists.metrics.filter( + objs => objs.metric_name.indexOf(value) !== -1, + ); + if (value === '') { + setColList({ columns: datasource.columns, metrics: datasource.metrics }); + } else setColList({ columns, metrics }); + }; + useEffect(() => { + setColList({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + }, [datasource]); + + const Metrics = ({ index }: number) => { + return ( + <div key={lists.metrics[index].metric_name} className="metric"> + <MetricOption metric={lists.metrics[index]} showType /> + </div> + ); + }; + + const Columns = ({ index }: number) => { Review comment: naming nit: maybe Column or ColumnRenderer? since this only renders a single column ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + } + .ant-collapse-item.ant-collapse-item-active { + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + } + .header { + font-size: ${({ theme }) => theme.typography.sizes.l}px; + margin-left: ${({ theme }) => theme.gridUnit * -2}px; + } + .ant-collapse-content-box > div { + margin-left: -14px; + } + .type-label { + text-align: left; + } + .metric-option .option-label { + margin-left: -20px; + } +`; + +const DataSourcePanel = ({ + datasource, + controls: { datasource: datasourceControl }, + actions, +}: Props) => { + const [lists, setColList] = useState({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + const search = ({ target: { value } }: { target: { value: string } }) => { + const columns = datasource.columns.filter( + obj => obj.column_name.indexOf(value) !== -1, + ); + const metrics = lists.metrics.filter( + objs => objs.metric_name.indexOf(value) !== -1, Review comment: same comment here, maybe `metric`? ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; Review comment: why is there an `!important` here? And in general, rotating icons feels like a bit of a code smell. Perhaps we should replace the icon with an arrow facing the other direction when the panel is collapsed/open. I assume anticon includes arrows pointing in at least 4 different directions that should be sufficient here ########## File path: superset-frontend/src/explore/components/DatasourcePanel.tsx ########## @@ -0,0 +1,189 @@ +/** + * 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, useState } from 'react'; +import { styled, t, QueryFormData } from '@superset-ui/core'; +import { Collapse } from 'src/common/components'; +import { FixedSizeList as List } from 'react-window'; +import { + ColumnOption, + MetricOption, + ControlType, +} from '@superset-ui/chart-controls'; +import Control from './Control'; + +interface DatasourceControl { + validationErrors: any; + mapStateToProps: QueryFormData; + type: ControlType; + label: string; + datasource?: any; +} + +interface Props { + datasource: { + columns: Array<any>; + metrics: Array<any>; + }; + controls: { + datasource: DatasourceControl; + }; + actions: any; +} + +const DatasourceContainer = styled.div` + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .field-selections { + padding: 0 ${({ theme }) => 2 * theme.gridUnit}px; + } + .ant-collapse + > .ant-collapse-item + > .ant-collapse-header + .ant-collapse-arrow { + right: ${({ theme }) => theme.gridUnit * -50}px; + } + .ant-collapse > .ant-collapse-item > .ant-collapse-header { + padding-left: 10px; + } + .form-control.input-sm { + margin-bottom: ${({ theme }) => theme.gridUnit * 3}px; + } + .ant-collapse-item { + background-color: ${({ theme }) => theme.colors.grayscale.light4}; + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(90deg) !important; + } + } + .ant-collapse-item.ant-collapse-item-active { + .anticon.anticon-right.ant-collapse-arrow > svg { + transform: rotate(-90deg) !important; + } + } + .header { + font-size: ${({ theme }) => theme.typography.sizes.l}px; + margin-left: ${({ theme }) => theme.gridUnit * -2}px; + } + .ant-collapse-content-box > div { + margin-left: -14px; + } + .type-label { + text-align: left; + } + .metric-option .option-label { + margin-left: -20px; + } +`; + +const DataSourcePanel = ({ + datasource, + controls: { datasource: datasourceControl }, + actions, +}: Props) => { + const [lists, setColList] = useState({ + columns: datasource.columns, + metrics: datasource.metrics, + }); + const search = ({ target: { value } }: { target: { value: string } }) => { + const columns = datasource.columns.filter( + obj => obj.column_name.indexOf(value) !== -1, + ); + const metrics = lists.metrics.filter( + objs => objs.metric_name.indexOf(value) !== -1, + ); + if (value === '') { Review comment: maybe this `value === ''` should be done first to early exit from the filters (this matters for us since we have a datasource with 10k metrics) ########## File path: superset-frontend/src/explore/components/ExploreViewContainer.jsx ########## @@ -340,7 +414,57 @@ class ExploreViewContainer extends React.Component { sliceName={this.props.sliceName} /> )} - <div className="col-sm-4 control-pane"> + <div + className={ + collapse + ? 'no-show' + : 'data-tab explore-column data-source-selection' + } + > + <div className="title-container"> + <span className="horizontal-text">Datasource</span> + <span + role="button" + tabIndex={0} + className="action-button" + onClick={this.handleCollapse} + > + <Icon + name="expand" + color={supersetTheme.colors.primary.base} + className="collapse-icon" + width={16} + /> + </span> + </div> + <DataSourcePanel + datasource={this.props.datasource} + controls={this.props.controls} + actions={this.props.actions} + /> + </div> + {collapse ? ( + <div + className="sidebar" + onClick={this.handleCollapse} + data-test="open-datasource-tab" + role="button" + tabIndex={0} + > + <span role="button" tabIndex={0} className="action-button"> + <Tooltip title="Open Datasource Tab"> Review comment: wrap in a t() string ########## File path: superset-frontend/src/explore/components/ExploreViewContainer.jsx ########## @@ -259,6 +308,10 @@ class ExploreViewContainer extends React.Component { } } + handleCollapse() { Review comment: naming nit: perhaps `toggleCollapse` is more obvious? ########## File path: superset-frontend/src/explore/components/ExploreViewContainer.jsx ########## @@ -326,12 +379,33 @@ class ExploreViewContainer extends React.Component { } render() { + const { collapse } = this.state; if (this.props.standalone) { return this.renderChartContainer(); } - return ( - <Styles id="explore-container" height={this.state.height}> + <Styles id="explore-container"> + <Global + styles={css` + .navbar { + margin-bottom: 0; + } + body { + max-height: 100vh; + overflow: hidden; + } + #app-menu, #app { + flex: 1 1 auto; + overflow: hidden; + } + #app { + flex-basis: 100%; + } + #app-menu { + flex-shrink: 0; + } + `} Review comment: Could you add how you tested these changes to the test plan? it seems like it could have unexpected effects with different chart types/window sizes ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
