Merge pull request #5 from williaster/chris--factor-out-components

[dashboard builder] don't pass dashboard layout to all dashboard components

superset/assets/javascripts/dashboard/v2/components/DashboardGrid.jsx

@@ -94,7 +94,10 @@ class DashboardGrid extends React.PureComponent {
     return (
       <div
         ref={(ref) => { this.grid = ref; }}
-        className={cx('grid-container', isResizing && 'grid-container--resizing')}
+        className={cx(
+          'grid-container',
+          isResizing && 'grid-container--resizing',
+        )}
       >
         <ParentSize>
           {({ width }) => {
@@ -108,9 +111,9 @@ class DashboardGrid extends React.PureComponent {
                   <DashboardComponent
                     key={id}
                     id={id}
+                    parentId={rootComponent.id}
                     depth={0}
                     index={index}
-                    parentId={rootComponent.id}
                     availableColumnCount={GRID_COLUMN_COUNT}
                     columnWidth={columnWidth}
                     onResizeStart={this.handleResizeStart}
@@ -122,10 +125,9 @@ class DashboardGrid extends React.PureComponent {
                 {rootComponent.children.length === 0 &&
                   <DragDroppable
                     component={rootComponent}
-                    components={components}
-                    orientation="column"
+                    parentComponent={null}
                     index={0}
-                    parentId={null}
+                    orientation="column"
                     onDrop={handleComponentDrop}
                   >
                     {({ dropIndicatorProps }) => (

superset/assets/javascripts/dashboard/v2/components/dnd/DragDroppable.jsx

@@ -2,24 +2,18 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { DragSource, DropTarget } from 'react-dnd';
 import cx from 'classnames';
-import throttle from 'lodash.throttle';
 
-import isValidChild from '../../util/isValidChild';
-import handleHover from './handleHover';
-import handleDrop from './handleDrop';
 import { dragConfig, dropConfig } from './dragDroppableConfig';
 import { componentShape } from '../../util/propShapes';
 
-const HOVER_THROTTLE_MS = 200;
 
 const propTypes = {
   children: PropTypes.func,
   component: componentShape.isRequired,
-  components: PropTypes.object.isRequired,
+  parentComponent: componentShape,
   disableDragDrop: PropTypes.bool,
   orientation: PropTypes.oneOf(['row', 'column']),
   index: PropTypes.number.isRequired,
-  parentId: PropTypes.string,
 
   // from react-dnd
   isDragging: PropTypes.bool.isRequired,
@@ -29,35 +23,24 @@ const propTypes = {
   dragSourceRef: PropTypes.func.isRequired,
   dragPreviewRef: PropTypes.func.isRequired,
 
-  // from redux (@TODO)
-  handleHover: PropTypes.func,
-  handleDrop: PropTypes.func,
-  onDrop: PropTypes.func, // @TODO rename broadcastDrop?
-  isValidChild: PropTypes.func,
-  isValidSibling: PropTypes.func,
+  // from redux
+  onDrop: PropTypes.func,
 };
 
 const defaultProps = {
+  parentComponent: null,
   disableDragDrop: false,
   children() {},
   onDrop() {},
-  parentId: null,
   orientation: 'row',
-  handleDrop,
-  handleHover,
-  isValidChild,
-  isValidSibling({ parentType, siblingType: childType }) {
-    return isValidChild({ parentType, childType });
-  },
 };
 
 class DragDroppable extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
-      dropIndicator: null,
+      dropIndicator: null, // this gets set/modified by the react-dnd HOCs
     };
-    this.handleHover = throttle(this.hover.bind(this), HOVER_THROTTLE_MS).bind(this);
   }
 
   componentDidMount() {
@@ -68,10 +51,6 @@ class DragDroppable extends React.Component {
     this.mounted = false;
   }
 
-  hover(props, monitor, Component) {
-    if (this.mounted) this.props.handleHover(props, monitor, Component);
-  }
-
   render() {
     const {
       children,

superset/assets/javascripts/dashboard/v2/components/dnd/dragDroppableConfig.js

@@ -1,3 +1,6 @@
+import handleHover from './handleHover';
+import handleDrop from './handleDrop';
+
 // note: the 'type' hook is not useful for us as dropping is contigent on other properties
 const TYPE = 'DRAG_DROPPABLE';
 
@@ -8,8 +11,13 @@ export const dragConfig = [
       return !props.disableDragDrop;
     },
     beginDrag(props /* , monitor, component */) {
-      const { component, index, parentId } = props;
-      return { draggableId: component.id, index, parentId, type: component.type };
+      const { component, index, parentComponent } = props;
+      return {
+        draggableId: component.id,
+        index,
+        parentId: parentComponent && parentComponent.id,
+        type: component.type,
+      };
     },
   },
   function dragStateToProps(connect, monitor) {
@@ -28,9 +36,9 @@ export const dropConfig = [
       if (
         component
         && component.decoratedComponentInstance
-        && component.decoratedComponentInstance.handleHover
-      ) { // use the component instance so we can throttle calls
-        component.decoratedComponentInstance.handleHover(
+        && component.decoratedComponentInstance.mounted
+      ) {
+        handleHover(
           props,
           monitor,
           component.decoratedComponentInstance,
@@ -43,8 +51,8 @@ export const dropConfig = [
     drop(props, monitor, component) {
       const Component = component.decoratedComponentInstance;
       const dropResult = monitor.getDropResult();
-      if ((!dropResult || !dropResult.destination) && Component.props.handleDrop) {
-        return Component.props.handleDrop(props, monitor, Component);
+      if ((!dropResult || !dropResult.destination) && Component.mounted) {
+        return handleDrop(props, monitor, Component);
       }
       return undefined;
     },

superset/assets/javascripts/dashboard/v2/components/dnd/handleDrop.js

@@ -1,21 +1,23 @@
+import isValidChild from '../../util/isValidChild';
+import shouldWrapChildInRow from '../../util/shouldWrapChildInRow';
+
 export default function handleDrop(props, monitor, Component) {
+  if (!Component.mounted) return undefined;
+
   Component.setState(() => ({ dropIndicator: null }));
+
   const {
-    components,
+    parentComponent,
     component,
-    parentId,
     index: componentIndex,
     onDrop,
     orientation,
-    isValidChild,
-    isValidSibling,
   } = Component.props;
 
   const draggingItem = monitor.getItem();
 
   // if dropped self on self, do nothing
   if (!draggingItem || draggingItem.draggableId === component.id) {
-    console.log(draggingItem ? 'drop self' : 'drop no item');
     return undefined;
   }
 
@@ -25,13 +27,17 @@ export default function handleDrop(props, monitor, Component) {
     childType: draggingItem.type,
   });
 
-  const validSibling = isValidSibling({
-    parentType: components[parentId] && components[parentId].type,
-    siblingType: draggingItem.type,
+  const validSibling = isValidChild({
+    parentType: parentComponent && parentComponent.type,
+    childType: draggingItem.type,
+  });
+
+  const shouldWrapSibling = shouldWrapChildInRow({
+    parentType: parentComponent && parentComponent.type,
+    childType: draggingItem.type,
   });
 
   if (!validChild && !validSibling) {
-    console.log('not valid drop child or sibling')
     return undefined;
   }
 
@@ -43,15 +49,16 @@ export default function handleDrop(props, monitor, Component) {
     draggableId: draggingItem.draggableId,
   };
 
-  if (validChild) { // append it to component.children
+  if (validChild && (!validSibling || shouldWrapSibling)) { // append it to component.children
     dropResult.destination = {
       droppableId: component.id,
       index: component.children.length,
     };
   } else { // insert as sibling
     // if the item is in the same list with a smaller index, you must account for the
     // "missing" index upon movement within the list
-    const sameList = draggingItem.parentId && draggingItem.parentId === parentId;
+    const sameList =
+      draggingItem.parentId && parentComponent && draggingItem.parentId === parentComponent.id;
     const sameListLowerIndex = sameList && draggingItem.index < componentIndex;
 
     let nextIndex = sameListLowerIndex ? componentIndex - 1 : componentIndex;
@@ -72,7 +79,7 @@ export default function handleDrop(props, monitor, Component) {
     }
 
     dropResult.destination = {
-      droppableId: parentId,
+      droppableId: parentComponent.id,
       index: nextIndex,
     };
   }

superset/assets/javascripts/dashboard/v2/components/dnd/handleHover.js

@@ -1,12 +1,17 @@
-export default function handleHover(props, monitor, Component) {
+import throttle from 'lodash.throttle';
+import isValidChild from '../../util/isValidChild';
+import shouldWrapChildInRow from '../../util/shouldWrapChildInRow';
+
+const HOVER_THROTTLE_MS = 200;
+
+function handleHover(props, monitor, Component) {
+  if (!Component.mounted) return;
+
   const {
     component,
-    components,
-    parentId,
+    parentComponent,
     orientation,
     isDraggingOverShallow,
-    isValidChild,
-    isValidSibling,
   } = Component.props;
 
   const draggingItem = monitor.getItem();
@@ -21,18 +26,22 @@ export default function handleHover(props, monitor, Component) {
     childType: draggingItem.type,
   });
 
-  const validSibling = isValidSibling({
-    parentType: components[parentId] && components[parentId].type,
-    siblingType: draggingItem.type,
+  const validSibling = isValidChild({
+    parentType: parentComponent && parentComponent.type,
+    childType: draggingItem.type,
+  });
+
+  const shouldWrapSibling = shouldWrapChildInRow({
+    parentType: parentComponent && parentComponent.type,
+    childType: draggingItem.type,
   });
 
-  if (validChild && !isDraggingOverShallow) {
+  if ((validChild && !isDraggingOverShallow) || (!validChild && !validSibling)) {
     Component.setState(() => ({ dropIndicator: null }));
     return;
   }
 
-  if (validChild) { // indicate drop in container
-    console.log('valid child', component.type, draggingItem.type);
+  if (validChild && (!validSibling || shouldWrapSibling)) { // indicate drop in container
     const indicatorOrientation = orientation === 'row' ? 'column' : 'row';
 
     Component.setState(() => ({
@@ -49,8 +58,7 @@ export default function handleHover(props, monitor, Component) {
         zIndex: 10,
       },
     }));
-  } else if (validSibling) { // indicate drop near parent
-    console.log('valid sibling', components[parentId].type, draggingItem.type);
+  } else { // indicate drop near parent
     const refBoundingRect = Component.ref.getBoundingClientRect();
     const clientOffset = monitor.getClientOffset();
 
@@ -80,3 +88,6 @@ export default function handleHover(props, monitor, Component) {
     }
   }
 }
+
+// this is called very frequently by react-dnd
+export default throttle(handleHover, HOVER_THROTTLE_MS);

superset/assets/javascripts/dashboard/v2/components/gridComponents/Chart.jsx

@@ -15,16 +15,16 @@ import {
 } from '../../util/constants';
 
 const propTypes = {
+  id: PropTypes.string.isRequired,
+  parentId: PropTypes.string.isRequired,
   component: componentShape.isRequired,
-  components: PropTypes.object.isRequired,
+  parentComponent: componentShape.isRequired,
   index: PropTypes.number.isRequired,
   depth: PropTypes.number.isRequired,
-  parentId: PropTypes.string.isRequired,
 
   // grid related
   availableColumnCount: PropTypes.number.isRequired,
   columnWidth: PropTypes.number.isRequired,
-  rowHeight: PropTypes.number,
   onResizeStart: PropTypes.func.isRequired,
   onResize: PropTypes.func.isRequired,
   onResizeStop: PropTypes.func.isRequired,
@@ -35,7 +35,6 @@ const propTypes = {
 };
 
 const defaultProps = {
-  rowHeight: null,
 };
 
 class Chart extends React.Component {
@@ -54,35 +53,32 @@ class Chart extends React.Component {
   }
 
   handleDeleteComponent() {
-    const { deleteComponent, component, parentId } = this.props;
-    deleteComponent(component.id, parentId);
+    const { deleteComponent, id, parentId } = this.props;
+    deleteComponent(id, parentId);
   }
 
   render() {
     const { isFocused } = this.state;
 
     const {
       component,
-      components,
+      parentComponent,
       index,
       depth,
-      parentId,
       availableColumnCount,
       columnWidth,
-      rowHeight,
       onResizeStart,
       onResize,
       onResizeStop,
       handleComponentDrop,
     } = this.props;
-    console.log('chart depth', depth)
+
     return (
       <DragDroppable
         component={component}
-        components={components}
+        parentComponent={parentComponent}
         orientation={depth % 2 === 1 ? 'column' : 'row'}
         index={index}
-        parentId={parentId}
         onDrop={handleComponentDrop}
         disableDragDrop={isFocused}
       >