diff --git a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts index ef4fef5aa..f90695805 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/control.test.ts @@ -55,7 +55,7 @@ describe('Datasource control', () => { cy.get('a').contains('Edit Datasource').click(); cy.get(`input[value="${newMetricName}"]`) .closest('tr') - .find('.fa-close') + .find('.fa-trash') .click(); cy.get('.modal-footer button').contains('Save').click(); cy.get('.modal-footer button').contains('OK').click(); diff --git a/superset-frontend/spec/javascripts/profile/EditableTitle_spec.tsx b/superset-frontend/spec/javascripts/profile/EditableTitle_spec.tsx index c4a5ecee0..1b23b02ea 100644 --- a/superset-frontend/spec/javascripts/profile/EditableTitle_spec.tsx +++ b/superset-frontend/spec/javascripts/profile/EditableTitle_spec.tsx @@ -34,7 +34,7 @@ describe('EditableTitle', () => { value: 'new title', }, }; - const editableWrapper = shallow(); + let editableWrapper = shallow(); const notEditableWrapper = shallow( , ); @@ -60,8 +60,7 @@ describe('EditableTitle', () => { describe('should handle change', () => { afterEach(() => { - editableWrapper.setState({ title: 'my title' }); - editableWrapper.setState({ lastTitle: 'my title' }); + editableWrapper = shallow(); }); it('should change title', () => { editableWrapper.find('input').simulate('change', mockEvent); @@ -79,8 +78,7 @@ describe('EditableTitle', () => { }); afterEach(() => { callback.resetHistory(); - editableWrapper.setState({ title: 'my title' }); - editableWrapper.setState({ lastTitle: 'my title' }); + editableWrapper = shallow(); }); it('default input type should be text', () => { @@ -88,7 +86,7 @@ describe('EditableTitle', () => { }); it('should trigger callback', () => { - editableWrapper.setState({ title: 'new title' }); + editableWrapper.find('input').simulate('change', mockEvent); editableWrapper.find('input').simulate('blur'); expect(editableWrapper.find('input').props().type).toBe('button'); expect(callback.callCount).toBe(1); @@ -101,7 +99,6 @@ describe('EditableTitle', () => { expect(callback.callCount).toBe(0); }); it('should not save empty title', () => { - editableWrapper.setState({ title: '' }); editableWrapper.find('input').simulate('blur'); expect(editableWrapper.find('input').props().type).toBe('button'); expect(editableWrapper.find('input').props().value).toBe('my title'); diff --git a/superset-frontend/src/CRUD/CollectionTable.jsx b/superset-frontend/src/CRUD/CollectionTable.tsx similarity index 69% rename from superset-frontend/src/CRUD/CollectionTable.jsx rename to superset-frontend/src/CRUD/CollectionTable.tsx index 848d26366..f8632bbdb 100644 --- a/superset-frontend/src/CRUD/CollectionTable.jsx +++ b/superset-frontend/src/CRUD/CollectionTable.tsx @@ -16,8 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; -import PropTypes from 'prop-types'; +import React, { ReactNode } from 'react'; import shortid from 'shortid'; import { t } from '@superset-ui/translation'; import Button from '../components/Button'; @@ -25,47 +24,42 @@ import Fieldset from './Fieldset'; import { recurseReactClone } from './utils'; import './crud.less'; -const propTypes = { - collection: PropTypes.arrayOf(PropTypes.object).isRequired, - itemGenerator: PropTypes.func, - columnLabels: PropTypes.object, - tableColumns: PropTypes.array.isRequired, - onChange: PropTypes.func, - itemRenderers: PropTypes.object, - allowDeletes: PropTypes.bool, - expandFieldset: PropTypes.node, - emptyMessage: PropTypes.node, - extraButtons: PropTypes.node, - allowAddItem: PropTypes.bool, -}; -const defaultProps = { - onChange: () => {}, - itemRenderers: {}, - columnLabels: {}, - allowDeletes: false, - emptyMessage: 'No entries', - allowAddItem: false, - itemGenerator: () => ({}), - expandFieldset: null, - extraButtons: null, -}; -const Frame = props =>
{props.children}
; -Frame.propTypes = { children: PropTypes.node }; +interface CRUDCollectionProps { + allowAddItem?: boolean; + allowDeletes?: boolean; + collection: Array; + columnLabels?: object; + emptyMessage: ReactNode; + expandFieldset: ReactNode; + extraButtons: ReactNode; + itemGenerator?: () => any; + itemRenderers?: any; + onChange?: (arg0: any) => void; + tableColumns: Array; +} -function createKeyedCollection(arr) { - const newArr = arr.map(o => ({ +interface CRUDCollectionState { + collection: object; + expandedColumns: object; +} + +function createKeyedCollection(arr: Array) { + const newArr = arr.map((o: any) => ({ ...o, id: o.id || shortid.generate(), })); const map = {}; - newArr.forEach(o => { + newArr.forEach((o: any) => { map[o.id] = o; }); return map; } -export default class CRUDCollection extends React.PureComponent { - constructor(props) { +export default class CRUDCollection extends React.PureComponent< + CRUDCollectionProps, + CRUDCollectionState +> { + constructor(props: CRUDCollectionProps) { super(props); this.state = { expandedColumns: {}, @@ -79,14 +73,14 @@ export default class CRUDCollection extends React.PureComponent { this.renderTableBody = this.renderTableBody.bind(this); this.changeCollection = this.changeCollection.bind(this); } - UNSAFE_componentWillReceiveProps(nextProps) { + UNSAFE_componentWillReceiveProps(nextProps: CRUDCollectionProps) { if (nextProps.collection !== this.props.collection) { this.setState({ collection: createKeyedCollection(nextProps.collection), }); } } - onCellChange(id, col, val) { + onCellChange(id: number, col: string, val: boolean) { this.changeCollection({ ...this.state.collection, [id]: { @@ -96,35 +90,39 @@ export default class CRUDCollection extends React.PureComponent { }); } onAddItem() { - let newItem = this.props.itemGenerator(); - if (!newItem.id) { - newItem = { ...newItem, id: shortid.generate() }; + if (this.props.itemGenerator) { + let newItem = this.props.itemGenerator(); + if (!newItem.id) { + newItem = { ...newItem, id: shortid.generate() }; + } + this.changeCollection({ + ...this.state.collection, + [newItem.id]: newItem, + }); } - this.changeCollection({ - ...this.state.collection, - [newItem.id]: newItem, - }); } - onFieldsetChange(item) { + onFieldsetChange(item: any) { this.changeCollection({ ...this.state.collection, [item.id]: item, }); } - getLabel(col) { + getLabel(col: any) { const { columnLabels } = this.props; - let label = columnLabels[col] ? columnLabels[col] : col; + let label = columnLabels && columnLabels[col] ? columnLabels[col] : col; if (label.startsWith('__')) { // special label-free columns (ie: caret for expand, delete cross) label = ''; } return label; } - changeCollection(collection) { + changeCollection(collection: any) { this.setState({ collection }); - this.props.onChange(Object.keys(collection).map(k => collection[k])); + if (this.props.onChange) { + this.props.onChange(Object.keys(collection).map(k => collection[k])); + } } - deleteItem(id) { + deleteItem(id: number) { const newColl = { ...this.state.collection }; delete newColl[id]; this.changeCollection(newColl); @@ -136,7 +134,7 @@ export default class CRUDCollection extends React.PureComponent { : tableColumns; return expandFieldset ? ['__expand'].concat(cols) : cols; } - toggleExpand(id) { + toggleExpand(id: any) { this.onCellChange(id, '__expanded', false); this.setState({ expandedColumns: { @@ -147,19 +145,33 @@ export default class CRUDCollection extends React.PureComponent { } renderHeaderRow() { const cols = this.effectiveTableColumns(); + const { + allowAddItem, + allowDeletes, + expandFieldset, + extraButtons, + } = this.props; return ( - {this.props.expandFieldset && } + {expandFieldset && } {cols.map(col => ( {this.getLabel(col)} ))} - {this.props.allowDeletes && } + {extraButtons} + {allowDeletes && !allowAddItem && } + {allowAddItem && ( + + + + )} ); } - renderExpandableSection(item) { + renderExpandableSection(item: any) { const propsGenerator = () => ({ item, onChange: this.onFieldsetChange }); return recurseReactClone( this.props.expandFieldset, @@ -167,14 +179,19 @@ export default class CRUDCollection extends React.PureComponent { propsGenerator, ); } - renderCell(record, col) { - const renderer = this.props.itemRenderers[col]; + renderCell(record: any, col: any) { + const renderer = this.props.itemRenderers && this.props.itemRenderers[col]; const val = record[col]; const onChange = this.onCellChange.bind(this, record.id, col); return renderer ? renderer(val, onChange, this.getLabel(col)) : val; } - renderItem(record) { - const { tableColumns, allowDeletes, expandFieldset } = this.props; + renderItem(record: any) { + const { + allowAddItem, + allowDeletes, + expandFieldset, + tableColumns, + } = this.props; /* eslint-disable no-underscore-dangle */ const isExpanded = !!this.state.expandedColumns[record.id] || record.__expanded; @@ -198,13 +215,16 @@ export default class CRUDCollection extends React.PureComponent { {this.renderCell(record, col)} )), ); + if (allowAddItem) { + tds.push(); + } if (allowDeletes) { tds.push( , @@ -252,17 +272,7 @@ export default class CRUDCollection extends React.PureComponent { {this.renderHeaderRow()} {this.renderTableBody()} -
- {this.props.allowAddItem && ( - - )} - {this.props.extraButtons} -
); } } -CRUDCollection.defaultProps = defaultProps; -CRUDCollection.propTypes = propTypes; diff --git a/superset-frontend/src/components/EditableTitle.jsx b/superset-frontend/src/components/EditableTitle.jsx deleted file mode 100644 index 63c05cdd3..000000000 --- a/superset-frontend/src/components/EditableTitle.jsx +++ /dev/null @@ -1,236 +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 React from 'react'; -import PropTypes from 'prop-types'; -import cx from 'classnames'; -import { t } from '@superset-ui/translation'; -import TooltipWrapper from './TooltipWrapper'; - -const propTypes = { - title: PropTypes.string, - canEdit: PropTypes.bool, - multiLine: PropTypes.bool, - onSaveTitle: PropTypes.func, - noPermitTooltip: PropTypes.string, - showTooltip: PropTypes.bool, - emptyText: PropTypes.node, - style: PropTypes.object, - extraClasses: PropTypes.oneOfType([ - PropTypes.arrayOf(PropTypes.string), - PropTypes.string, - ]), -}; -const defaultProps = { - title: t('Title'), - canEdit: false, - multiLine: false, - showTooltip: true, - onSaveTitle: () => {}, - emptyText: '', - style: null, - extraClasses: null, -}; - -export default class EditableTitle extends React.PureComponent { - constructor(props) { - super(props); - this.state = { - isEditing: false, - title: this.props.title, - lastTitle: this.props.title, - }; - this.handleClick = this.handleClick.bind(this); - this.handleBlur = this.handleBlur.bind(this); - this.handleChange = this.handleChange.bind(this); - this.handleKeyPress = this.handleKeyPress.bind(this); - - // Used so we can access the DOM element if a user clicks on this component. - this.contentRef = React.createRef(); - } - - UNSAFE_componentWillReceiveProps(nextProps) { - if (nextProps.title !== this.state.title) { - this.setState({ - lastTitle: this.state.title, - title: nextProps.title, - }); - } - } - - handleClick() { - if (!this.props.canEdit || this.state.isEditing) { - return; - } - - // For multi-line values, save the actual rendered size of the displayed text. - // Later, if a textarea is constructed for editing the value, we'll need this. - const contentBoundingRect = this.contentRef.current - ? this.contentRef.current.getBoundingClientRect() - : null; - - this.setState({ isEditing: true, contentBoundingRect }); - } - - handleBlur() { - const title = this.state.title.trim(); - - if (!this.props.canEdit) { - return; - } - - this.setState({ - isEditing: false, - }); - - if (!title.length) { - this.setState({ - title: this.state.lastTitle, - }); - - return; - } - - if (this.state.lastTitle !== title) { - this.setState({ - lastTitle: title, - }); - } - - if (this.props.title !== title) { - this.props.onSaveTitle(title); - } - } - - // this entire method exists to support using EditableTitle as the title of a - // react-bootstrap Tab, as a workaround for this line in react-bootstrap https://goo.gl/ZVLmv4 - // - // tl;dr when a Tab EditableTitle is being edited, typically the Tab it's within has been - // clicked and is focused/active. for accessibility, when focused the Tab intercepts - // the ' ' key (among others, including all arrows) and onChange() doesn't fire. somehow - // keydown is still called so we can detect this and manually add a ' ' to the current title - handleKeyDown(event) { - if (event.key === ' ') { - event.stopPropagation(); - } - } - - handleChange(ev) { - if (!this.props.canEdit) { - return; - } - this.setState({ - title: ev.target.value, - }); - } - - handleKeyPress(ev) { - if (ev.key === 'Enter') { - ev.preventDefault(); - this.handleBlur(); - } - } - - render() { - const { isEditing, title, contentBoundingRect } = this.state; - const { - emptyText, - multiLine, - showTooltip, - canEdit, - noPermitTooltip, - style, - extraClasses, - } = this.props; - - let value; - if (title) { - value = title; - } else if (!isEditing) { - value = emptyText; - } - - // Construct an inline style based on previously-saved height of the rendered label. Only - // used in multi-line contexts. - const editStyle = - isEditing && contentBoundingRect - ? { height: `${contentBoundingRect.height}px` } - : null; - - // Create a textarea when we're editing a multi-line value, otherwise create an input (which may - // be text or a button). - let input = - multiLine && isEditing ? ( -