diff --git a/frontend/src/metabase/components/CreateDashboardModal.jsx b/frontend/src/metabase/components/CreateDashboardModal.jsx index fd2af6ae0582ffe4b9545291b3ea960a6a102fb8..ae1c4333bceaef63873f18f278864152ba610890 100644 --- a/frontend/src/metabase/components/CreateDashboardModal.jsx +++ b/frontend/src/metabase/components/CreateDashboardModal.jsx @@ -1,20 +1,13 @@ import React, { Component } from "react"; import PropTypes from "prop-types"; -import { t } from "c-3po"; import { connect } from "react-redux"; import { withRouter } from "react-router"; import { push } from "react-router-redux"; import * as Urls from "metabase/lib/urls"; -import FormField from "metabase/components/form/FormField.jsx"; -import ModalContent from "metabase/components/ModalContent.jsx"; - -import Button from "metabase/components/Button.jsx"; -import CollectionSelect from "metabase/containers/CollectionSelect.jsx"; - -import Dashboards from "metabase/entities/dashboards"; import Collections from "metabase/entities/collections"; +import DashboardForm from "metabase/containers/DashboardForm"; const mapStateToProps = (state, props) => ({ initialCollectionId: Collections.selectors.getInitialCollectionId( @@ -24,133 +17,29 @@ const mapStateToProps = (state, props) => ({ }); const mapDispatchToProps = { - createDashboard: Dashboards.actions.create, - push, + onChangeLocation: push, }; @withRouter @connect(mapStateToProps, mapDispatchToProps) export default class CreateDashboardModal extends Component { - constructor(props, context) { - super(props, context); - - this.state = { - name: null, - description: null, - errors: null, - collection_id: props.initialCollectionId, - }; - } - static propTypes = { - createDashboard: PropTypes.func.isRequired, onClose: PropTypes.func, }; - setName = event => { - this.setState({ name: event.target.value }); - }; - - setDescription = event => { - this.setState({ description: event.target.value }); - }; - - createNewDash = async event => { - event.preventDefault(); - - let name = this.state.name && this.state.name.trim(); - let description = this.state.description && this.state.description.trim(); - - // populate a new Dash object - let newDash = { - name: name && name.length > 0 ? name : null, - description: description && description.length > 0 ? description : null, - collection_id: this.state.collection_id, - }; - - const { payload } = await this.props.createDashboard(newDash); - this.props.push(Urls.dashboard(payload.result)); - this.props.onClose(); - }; - render() { - let formError; - if (this.state.errors) { - let errorMessage = t`Server error encountered`; - if (this.state.errors.data && this.state.errors.data.message) { - errorMessage = this.state.errors.data.message; - } - - // TODO: timeout display? - formError = <span className="text-error px2">{errorMessage}</span>; - } - - let name = this.state.name && this.state.name.trim(); - - let formReady = name !== null && name !== ""; - + const { initialCollectionId, onClose, onChangeLocation } = this.props; return ( - <ModalContent - id="CreateDashboardModal" - title={t`Create dashboard`} - footer={[ - formError, - <Button - mr={1} - onClick={() => this.props.onClose()} - >{t`Cancel`}</Button>, - <Button - primary={formReady} - disabled={!formReady} - onClick={this.createNewDash} - >{t`Create`}</Button>, - ]} - onClose={this.props.onClose} - > - <form className="Modal-form" onSubmit={this.createNewDash}> - <div> - <FormField - name="name" - displayName={t`Name`} - formError={this.state.errors} - > - <input - className="Form-input full" - name="name" - placeholder={t`What is the name of your dashboard?`} - value={this.state.name} - onChange={this.setName} - autoFocus - /> - </FormField> - - <FormField - name="description" - displayName={t`Description`} - formError={this.state.errors} - > - <input - className="Form-input full" - name="description" - placeholder={t`It's optional but oh, so helpful`} - value={this.state.description} - onChange={this.setDescription} - /> - </FormField> - - <FormField - displayName={t`Which collection should this go in?`} - fieldName="collection_id" - errors={this.state.errors} - > - <CollectionSelect - value={this.state.collection_id} - onChange={collection_id => this.setState({ collection_id })} - /> - </FormField> - </div> - </form> - </ModalContent> + <DashboardForm + dashboard={{ collection_id: initialCollectionId }} + onClose={onClose} + onSaved={dashboard => { + onChangeLocation(Urls.dashboard(dashboard.id)); + if (onClose) { + onClose(); + } + }} + /> ); } } diff --git a/frontend/src/metabase/components/form/StandardForm.jsx b/frontend/src/metabase/components/form/StandardForm.jsx index d198748544b9a62f95dc19b300c7675b4b092c85..511b0eeae7e2c9fcf51b5c4e4f42033dbb26574b 100644 --- a/frontend/src/metabase/components/form/StandardForm.jsx +++ b/frontend/src/metabase/components/form/StandardForm.jsx @@ -6,6 +6,7 @@ import FormMessage from "metabase/components/form/FormMessage"; import Button from "metabase/components/Button"; +import { t } from "c-3po"; import cx from "classnames"; import { getIn } from "icepick"; @@ -23,11 +24,12 @@ const StandardForm = ({ className, resetButton = false, newForm = true, + onClose = null, ...props }) => ( <form onSubmit={handleSubmit} className={cx(className, { NewForm: newForm })}> - <div className="m1"> + <div> {form.fields(values).map(formField => { const nameComponents = formField.name.split("."); const field = getIn(fields, nameComponents); @@ -47,25 +49,30 @@ const StandardForm = ({ ); })} </div> - <div className={cx("m1", { "Form-offset": !newForm })}> - <Button - type="submit" - primary - disabled={submitting || invalid} - className="mr1" - > - {values.id != null ? "Update" : "Create"} - </Button> - {resetButton && ( + <div className={cx("flex", { "Form-offset": !newForm })}> + <div className="ml-auto flex align-center"> + {onClose && ( + <Button className="mr1" onClick={onClose}>{t`Cancel`}</Button> + )} <Button - type="button" - disabled={submitting || !dirty} - onClick={resetForm} + type="submit" + primary={!(submitting || invalid)} + disabled={submitting || invalid} + className="mr1" > - Reset + {values.id != null ? t`Update` : t`Create`} </Button> - )} - {error && <FormMessage message={error} formError />} + {resetButton && ( + <Button + type="button" + disabled={submitting || !dirty} + onClick={resetForm} + > + {t`Reset`} + </Button> + )} + {error && <FormMessage message={error} formError />} + </div> </div> </form> ); diff --git a/frontend/src/metabase/containers/DashboardForm.jsx b/frontend/src/metabase/containers/DashboardForm.jsx index 5272aaef4617a7429fbc1d1e3bd3b5220241189a..be722f4350473137cb77b7082027e1fef4f2db33 100644 --- a/frontend/src/metabase/containers/DashboardForm.jsx +++ b/frontend/src/metabase/containers/DashboardForm.jsx @@ -6,11 +6,16 @@ import ModalContent from "metabase/components/ModalContent"; const DashboardForm = ({ dashboard, onClose, ...props }) => ( <ModalContent title={ - dashboard && dashboard.id != null ? dashboard.name : t`New dashboard` + dashboard && dashboard.id != null ? dashboard.name : t`Create dashboard` } onClose={onClose} > - <EntityForm entityType="dashboards" entityObject={dashboard} {...props} /> + <EntityForm + entityType="dashboards" + entityObject={dashboard} + onClose={onClose} + {...props} + /> </ModalContent> ); diff --git a/frontend/src/metabase/containers/Overworld.jsx b/frontend/src/metabase/containers/Overworld.jsx index 8ea953333352f9cbad66f8592cf0ef04b598ef5b..f755d21cd760c04c7cb7788d46609a0e7010a1a1 100644 --- a/frontend/src/metabase/containers/Overworld.jsx +++ b/frontend/src/metabase/containers/Overworld.jsx @@ -30,31 +30,42 @@ import { entityListLoader } from "metabase/entities/containers/EntityListLoader" const PAGE_PADDING = [1, 2, 4]; +import { createSelector } from "reselect"; + +// use reselect select to avoid re-render if list doesn't change +const getParitionedCollections = createSelector( + [(state, props) => props.list], + list => { + const [collections, items] = _.partition( + list, + item => item.model === "collection", + ); + const [pinned, unpinned] = _.partition( + items, + item => item.collection_position != null, + ); + + // sort the pinned items by collection_position + pinned.sort((a, b) => a.collection_position - b.collection_position); + return { + collections, + pinned, + unpinned, + }; + }, +); + //class Overworld extends Zelda @entityListLoader({ entityType: "search", - entityQuery: (state, props) => ({ collection: "root" }), + entityQuery: { collection: "root" }, wrapped: true, }) -@connect((state, props) => { +@connect((state, props) => ({ // split out collections, pinned, and unpinned since bulk actions only apply to unpinned - const [collections, items] = _.partition( - props.list, - item => item.model === "collection", - ); - const [pinned, unpinned] = _.partition( - items, - item => item.collection_position != null, - ); - // sort the pinned items by collection_position - pinned.sort((a, b) => a.collection_position - b.collection_position); - return { - collections, - pinned, - unpinned, - user: getUser(state), - }; -}) + ...getParitionedCollections(state, props), + user: getUser(state, props), +})) class Overworld extends React.Component { render() { const { user } = this.props; diff --git a/frontend/src/metabase/entities/containers/EntityListLoader.jsx b/frontend/src/metabase/entities/containers/EntityListLoader.jsx index ab13658f82b9572238a567ce2a53b30525426ae9..31987dd1e3f31365ad4d1cd3fd74f1d069062f4b 100644 --- a/frontend/src/metabase/entities/containers/EntityListLoader.jsx +++ b/frontend/src/metabase/entities/containers/EntityListLoader.jsx @@ -4,6 +4,7 @@ import React from "react"; import { connect } from "react-redux"; import _ from "underscore"; import { createSelector } from "reselect"; +import { createMemoizedSelector } from "metabase/lib/redux"; import entityType from "./EntityType"; import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; @@ -25,11 +26,24 @@ export type RenderProps = { reload: () => void, }; +const getEntityQuery = (state, props) => + typeof props.entityQuery === "function" + ? props.entityQuery(state, props) + : props.entityQuery; + +// NOTE: Memoize entityQuery so we don't re-render even if a new but identical +// object is created. This works because entityQuery must be JSON serializable +// NOTE: Technically leaks a small amount of memory because it uses an unbounded +// memoization cache, but that's probably ok. +const getMemoizedEntityQuery = createMemoizedSelector( + [getEntityQuery], + entityQuery => entityQuery, +); + @entityType() -@connect((state, { entityDef, entityQuery, ...props }) => { - if (typeof entityQuery === "function") { - entityQuery = entityQuery(state, props); - } +@connect((state, props) => { + const { entityDef } = props; + const entityQuery = getMemoizedEntityQuery(state, props); return { entityQuery, list: entityDef.selectors.getList(state, { entityQuery }), diff --git a/frontend/src/metabase/entities/containers/EntityType.jsx b/frontend/src/metabase/entities/containers/EntityType.jsx index 0b234f179b12146c368111556196b4796b1e7fff..c666f9009f4bf8b043796b5111c0269609e0f6f0 100644 --- a/frontend/src/metabase/entities/containers/EntityType.jsx +++ b/frontend/src/metabase/entities/containers/EntityType.jsx @@ -34,6 +34,16 @@ export default (entityType?: string) => ( } } + shouldComponentUpdate(nextProps, nextState) { + if ( + nextProps.entityDef !== this.props.entityDef || + nextProps.dispatch !== this.props.dispatch + ) { + return true; + } + return false; + } + _bindActionCreators({ entityDef, dispatch }) { this._boundActionCreators = bindActionCreators( entityDef.actions, diff --git a/frontend/src/metabase/entities/dashboards.js b/frontend/src/metabase/entities/dashboards.js index 857e42b9cf62a083108154069e54058cbe61f3cf..58722ca0d834788c295fba4072a0da7ee7916f01 100644 --- a/frontend/src/metabase/entities/dashboards.js +++ b/frontend/src/metabase/entities/dashboards.js @@ -4,6 +4,7 @@ import { createEntity, undo } from "metabase/lib/entities"; import * as Urls from "metabase/lib/urls"; import { normal } from "metabase/lib/colors"; import { assocIn } from "icepick"; +import { t } from "c-3po"; import { POST, DELETE } from "metabase/lib/api"; import { canonicalCollectionId } from "metabase/entities/collections"; @@ -86,7 +87,25 @@ const Dashboards = createEntity({ }, form: { - fields: [{ name: "name" }, { name: "description", type: "text" }], + fields: [ + { + name: "name", + placeholder: t`What is the name of your dashboard?`, + validate: name => (!name ? "Name is required" : null), + }, + { + name: "description", + type: "text", + placeholder: t`It's optional but oh, so helpful`, + }, + { + name: "collection_id", + title: t`Which collection should this go in?`, + type: "collection", + validate: colelctionId => + colelctionId === undefined ? "Collection is required" : null, + }, + ], }, }); diff --git a/frontend/src/metabase/lib/redux.js b/frontend/src/metabase/lib/redux.js index 1840f4b6ab1eb7d7fd983e12b5c91acdb6b0dd1a..0ef70bb5691bf9c4a581b775b1ed1c6f346f0c33 100644 --- a/frontend/src/metabase/lib/redux.js +++ b/frontend/src/metabase/lib/redux.js @@ -8,6 +8,9 @@ import { setRequestState, clearRequestState } from "metabase/redux/requests"; export { combineReducers } from "redux"; export { handleActions, createAction } from "redux-actions"; +import { createSelectorCreator } from "reselect"; +import memoize from "lodash.memoize"; + // similar to createAction but accepts a (redux-thunk style) thunk and dispatches based on whether // the promise returned from the thunk resolves or rejects, similar to redux-promise export function createThunkAction(actionType, actionThunkCreator) { @@ -192,3 +195,8 @@ export const formDomOnlyProps = ({ defaultValue, ...domProps }) => domProps; + +export const createMemoizedSelector = createSelectorCreator( + memoize, + (...args) => JSON.stringify(args), +); diff --git a/package.json b/package.json index 56bcaf1ffa9555a1006fde6e1e25af77224ad63f..d67fe71108549b507aac6636b5840ca008ec1d23 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "leaflet": "^1.2.0", "leaflet-draw": "^0.4.9", "leaflet.heat": "^0.2.0", + "lodash.memoize": "^4.1.2", "moment": "2.19.3", "node-libs-browser": "^2.0.0", "normalizr": "^3.0.2",