You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by cc...@apache.org on 2018/03/16 01:33:38 UTC
[incubator-superset] branch chris--top-level-tabs updated:
[top-level-tabs] simplify redux and disable ability to displace top-level
tabs with other tabs
This is an automated email from the ASF dual-hosted git repository.
ccwilliams pushed a commit to branch chris--top-level-tabs
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/chris--top-level-tabs by this push:
new 82bd8a1 [top-level-tabs] simplify redux and disable ability to displace top-level tabs with other tabs
82bd8a1 is described below
commit 82bd8a181d9ca952a37a73f8469a625936e74376
Author: Chris Williams <ch...@airbnb.com>
AuthorDate: Thu Mar 15 18:33:21 2018 -0700
[top-level-tabs] simplify redux and disable ability to displace top-level tabs with other tabs
---
.../javascripts/dashboard/v2/actions/index.js | 51 +++++++---
.../dashboard/v2/components/DashboardBuilder.jsx | 38 ++++----
.../dashboard/v2/components/dnd/handleDrop.js | 2 +-
.../v2/components/gridComponents/Tabs.jsx | 8 +-
.../javascripts/dashboard/v2/reducers/dashboard.js | 105 +++++++++++----------
.../dashboard/v2/stylesheets/components/tabs.less | 11 +--
.../javascripts/dashboard/v2/util/findParentId.js | 15 +++
.../dashboard/v2/util/newEntitiesFromDrop.js | 27 ++----
8 files changed, 143 insertions(+), 114 deletions(-)
diff --git a/superset/assets/javascripts/dashboard/v2/actions/index.js b/superset/assets/javascripts/dashboard/v2/actions/index.js
index 42b9e31..effdeeb 100644
--- a/superset/assets/javascripts/dashboard/v2/actions/index.js
+++ b/superset/assets/javascripts/dashboard/v2/actions/index.js
@@ -1,6 +1,8 @@
-import { DASHBOARD_ROOT_ID, DASHBOARD_GRID_ID } from '../util/constants';
+import { DASHBOARD_ROOT_ID } from '../util/constants';
+import findParentId from '../util/findParentId';
import { TABS_TYPE } from '../util/componentTypes';
+// Component CRUD -------------------------------------------------------------
export const UPDATE_COMPONENTS = 'UPDATE_COMPONENTS';
export function updateComponents(nextComponents) {
return {
@@ -32,6 +34,17 @@ export function createComponent(dropResult) {
};
}
+// Tabs -----------------------------------------------------------------------
+export const CREATE_TOP_LEVEL_TABS = 'CREATE_TOP_LEVEL_TABS';
+export function createTopLevelTabs(dropResult) {
+ return {
+ type: CREATE_TOP_LEVEL_TABS,
+ payload: {
+ dropResult,
+ },
+ };
+}
+
export const DELETE_TOP_LEVEL_TABS = 'DELETE_TOP_LEVEL_TABS';
export function deleteTopLevelTabs() {
return {
@@ -53,22 +66,36 @@ export function moveComponent(dropResult) {
export const HANDLE_COMPONENT_DROP = 'HANDLE_COMPONENT_DROP';
export function handleComponentDrop(dropResult) {
- return (dispatch) => {
- if (
- dropResult.destination
- && dropResult.source
+ return (dispatch, getState) => {
+ const { source, destination } = dropResult;
+ const droppedOnRoot = destination && destination.droppableId === DASHBOARD_ROOT_ID;
+ const isNewComponent = !source; // @TODO create NEW_COMPONENTS source for better readability
+
+ if (droppedOnRoot) {
+ dispatch(createTopLevelTabs(dropResult));
+ } else if (destination && isNewComponent) {
+ dispatch(createComponent(dropResult));
+ } else if (
+ destination
+ && source
&& !( // ensure it has moved
- dropResult.destination.droppableId === dropResult.source.droppableId
- && dropResult.destination.index === dropResult.source.index
+ destination.droppableId === source.droppableId
+ && destination.index === source.index
)
) {
- return dispatch(moveComponent(dropResult));
+ dispatch(moveComponent(dropResult));
+ }
- // new components don't have a source
- // @TODO should create a NEW_COMPONENTS source for these because it's more readable
- } else if (dropResult.destination && !dropResult.source) {
- return dispatch(createComponent(dropResult));
+ if (source) {
+ const { dashboard } = getState();
+ const sourceComponent = dashboard[source.droppableId];
+
+ if (sourceComponent.type === TABS_TYPE && sourceComponent.children.length === 0) {
+ const parentId = findParentId({ childId: source.droppableId, components: dashboard });
+ dispatch(deleteComponent(source.droppableId, parentId));
+ }
}
+
return null;
};
}
diff --git a/superset/assets/javascripts/dashboard/v2/components/DashboardBuilder.jsx b/superset/assets/javascripts/dashboard/v2/components/DashboardBuilder.jsx
index f59affd..4f4fe33 100644
--- a/superset/assets/javascripts/dashboard/v2/components/DashboardBuilder.jsx
+++ b/superset/assets/javascripts/dashboard/v2/components/DashboardBuilder.jsx
@@ -31,9 +31,11 @@ const defaultProps = {
};
class DashboardBuilder extends React.Component {
- static shouldFocusTabs(event) {
+ static shouldFocusTabs(event, container) {
// don't focus the tabs when we click on a tab
- return event.target.tagName === 'UL' || /icon-button/.test(event.target.className);
+ return event.target.tagName === 'UL' || (
+ /icon-button/.test(event.target.className) && container.contains(event.target)
+ );
}
constructor(props) {
@@ -63,21 +65,23 @@ class DashboardBuilder extends React.Component {
return (
<div className="dashboard-v2">
- <DragDroppable
- component={dashboardRoot}
- parentComponent={null}
- index={0}
- orientation="column"
- onDrop={handleComponentDrop}
- >
- {({ dropIndicatorProps }) => (
- <div>
- <DashboardHeader />
- {dropIndicatorProps &&
- <div {...dropIndicatorProps} />}
- </div>
- )}
- </DragDroppable>
+ {topLevelTabs ? ( // you cannot displace tabs if they already exist
+ <DashboardHeader />
+ ) : (
+ <DragDroppable
+ component={dashboardRoot}
+ parentComponent={null}
+ index={0}
+ orientation="column"
+ onDrop={topLevelTabs ? null : handleComponentDrop}
+ >
+ {({ dropIndicatorProps }) => (
+ <div>
+ <DashboardHeader />
+ {dropIndicatorProps && <div {...dropIndicatorProps} />}
+ </div>
+ )}
+ </DragDroppable>)}
{topLevelTabs &&
<WithPopoverMenu
diff --git a/superset/assets/javascripts/dashboard/v2/components/dnd/handleDrop.js b/superset/assets/javascripts/dashboard/v2/components/dnd/handleDrop.js
index cf790da..c81a04e 100644
--- a/superset/assets/javascripts/dashboard/v2/components/dnd/handleDrop.js
+++ b/superset/assets/javascripts/dashboard/v2/components/dnd/handleDrop.js
@@ -2,7 +2,7 @@ import getDropPosition, { DROP_TOP, DROP_RIGHT, DROP_BOTTOM, DROP_LEFT } from '.
export default function handleDrop(props, monitor, Component) {
// this may happen due to throttling
- if (!Component.mounted) return undefined;
+ if (!Component.mounted || !Component.props.onDrop) return undefined;
Component.setState(() => ({ dropIndicator: null }));
const dropPosition = getDropPosition(monitor, Component);
diff --git a/superset/assets/javascripts/dashboard/v2/components/gridComponents/Tabs.jsx b/superset/assets/javascripts/dashboard/v2/components/gridComponents/Tabs.jsx
index dba8072..3e2d9cc 100644
--- a/superset/assets/javascripts/dashboard/v2/components/gridComponents/Tabs.jsx
+++ b/superset/assets/javascripts/dashboard/v2/components/gridComponents/Tabs.jsx
@@ -8,7 +8,7 @@ import DashboardComponent from '../../containers/DashboardComponent';
import DeleteComponentButton from '../DeleteComponentButton';
import HoverMenu from '../menu/HoverMenu';
import { componentShape } from '../../util/propShapes';
-import { NEW_TAB_ID } from '../../util/constants';
+import { NEW_TAB_ID, DASHBOARD_ROOT_ID } from '../../util/constants';
import { RENDER_TAB, RENDER_TAB_CONTENT } from './Tab';
const NEW_TAB_INDEX = -1;
@@ -199,14 +199,14 @@ class Tabs extends React.PureComponent {
{tabIds.length < MAX_TAB_COUNT &&
<BootstrapTab
eventKey={NEW_TAB_INDEX}
- title={<div className="fa fa-plus-square" />}
+ title={<div className="fa fa-plus" />}
/>}
</BootstrapTabs>
+ {/* don't indicate that a drop on root is allowed when tabs already exist */}
{tabsDropIndicatorProps
- && tabsDropIndicatorProps.style
- && tabsDropIndicatorProps.style.width === '100%'
+ && parentComponent.id !== DASHBOARD_ROOT_ID
&& <div {...tabsDropIndicatorProps} />}
</div>
diff --git a/superset/assets/javascripts/dashboard/v2/reducers/dashboard.js b/superset/assets/javascripts/dashboard/v2/reducers/dashboard.js
index 4518273..418b2aa 100644
--- a/superset/assets/javascripts/dashboard/v2/reducers/dashboard.js
+++ b/superset/assets/javascripts/dashboard/v2/reducers/dashboard.js
@@ -3,13 +3,14 @@ import newComponentFactory from '../util/newComponentFactory';
import newEntitiesFromDrop from '../util/newEntitiesFromDrop';
import reorderItem from '../util/dnd-reorder';
import shouldWrapChildInRow from '../util/shouldWrapChildInRow';
-import { TABS_TYPE, ROW_TYPE } from '../util/componentTypes';
+import { TAB_TYPE, TABS_TYPE, ROW_TYPE } from '../util/componentTypes';
import {
UPDATE_COMPONENTS,
DELETE_COMPONENT,
CREATE_COMPONENT,
MOVE_COMPONENT,
+ CREATE_TOP_LEVEL_TABS,
DELETE_TOP_LEVEL_TABS,
} from '../actions';
@@ -74,57 +75,6 @@ const actionHandlers = {
if (!source || !destination || !draggableId) return state;
- // If we've dropped on the root, move previous root children to drag item
- if (draggableType === TABS_TYPE && destination.droppableId === DASHBOARD_ROOT_ID) {
- const rootComponent = state[DASHBOARD_ROOT_ID];
-
- const topLevelId = rootComponent.children[0];
- const topLevelComponent = state[topLevelId];
- const topLevelComponentIsTabs = topLevelComponent.type === TABS_TYPE;
- const childrenToMove = topLevelComponentIsTabs
- ? [topLevelId] // just move the tabs
- : [...topLevelComponent.children]; // move all children
-
- const draggingTabs = state[draggableId];
- const firstTabId = draggingTabs.children[0];
- const firstTab = state[firstTabId];
-
- const updatedEntities = {
- [DASHBOARD_ROOT_ID]: {
- ...rootComponent,
- children: [draggableId],
- },
- [firstTabId]: {
- ...firstTab,
- children: [
- ...firstTab.children,
- ...childrenToMove.filter(id => id !== draggableId),
- ],
- },
- };
-
- if (!topLevelComponentIsTabs) {
- updatedEntities[topLevelId] = { ...topLevelComponent, children: [] };
- } else {
- // find the moved item and remove it as a child
- topLevelComponent.children.forEach((tabId) => {
- const tabComponent = state[tabId];
- const containsItem = tabComponent.children.includes(draggableId);
- if (containsItem > -1) {
- updatedEntities[tabId] = {
- ...tabComponent,
- children: [...tabComponent.children].filter(id => id !== draggableId),
- };
- }
- });
- }
-
- return {
- ...state,
- ...updatedEntities,
- };
- }
-
const nextEntities = reorderItem({
entitiesMap: state,
source,
@@ -153,6 +103,57 @@ const actionHandlers = {
};
},
+ [CREATE_TOP_LEVEL_TABS](state, action) {
+ const { payload: { dropResult } } = action;
+ const { source, draggableId } = dropResult;
+
+ // move children of current root to be children of the dragging tab
+ const rootComponent = state[DASHBOARD_ROOT_ID];
+ const topLevelId = rootComponent.children[0];
+ const topLevelComponent = state[topLevelId];
+
+ if (source) {
+ const draggingTabs = state[draggableId];
+ const draggingTabId = draggingTabs.children[0];
+ const draggingTab = state[draggingTabId];
+
+ // move all children except the one that is dragging
+ const childrenToMove = [...topLevelComponent.children].filter(id => id !== draggableId);
+
+ return {
+ ...state,
+ [DASHBOARD_ROOT_ID]: {
+ ...rootComponent,
+ children: [draggableId],
+ },
+ [topLevelId]: {
+ ...topLevelComponent,
+ children: [],
+ },
+ [draggingTabId]: {
+ ...draggingTab,
+ children: [
+ ...draggingTab.children,
+ ...childrenToMove,
+ ],
+ },
+ };
+ }
+ const newEntities = newEntitiesFromDrop({ dropResult, components: state });
+ const newEntitiesArray = Object.values(newEntities);
+ const tabComponent = newEntitiesArray.find(component => component.type === TAB_TYPE);
+ const tabsComponent = newEntitiesArray.find(component => component.type === TABS_TYPE);
+
+ tabComponent.children = [...topLevelComponent.children];
+ newEntities[topLevelId] = { ...topLevelComponent, children: [] };
+ newEntities[DASHBOARD_ROOT_ID] = { ...rootComponent, children: [tabsComponent.id] };
+
+ return {
+ ...state,
+ ...newEntities,
+ };
+ },
+
[DELETE_TOP_LEVEL_TABS](state) {
const rootComponent = state[DASHBOARD_ROOT_ID];
const topLevelId = rootComponent.children[0];
diff --git a/superset/assets/javascripts/dashboard/v2/stylesheets/components/tabs.less b/superset/assets/javascripts/dashboard/v2/stylesheets/components/tabs.less
index 2205ce3..f86365f 100644
--- a/superset/assets/javascripts/dashboard/v2/stylesheets/components/tabs.less
+++ b/superset/assets/javascripts/dashboard/v2/stylesheets/components/tabs.less
@@ -18,7 +18,7 @@
}
.dashboard-component-tabs .nav-tabs > li > a {
- color: #263238;
+ color: @almost-black;
border: none;
padding: 12px 0 14px 0;
font-size: 15px;
@@ -40,7 +40,7 @@
.dashboard-component-tabs .nav-tabs > li > a:hover {
border: none;
background: inherit;
- color: #000000;
+ color: @almost-black;
}
.dashboard-component-tabs .nav-tabs > li > a:focus {
@@ -59,9 +59,6 @@
}
.dashboard-component-tabs .fa-plus-square {
- background: linear-gradient(135deg, #E32464, #2C2261);
- -webkit-background-clip: text;
- -webkit-text-fill-color: transparent;
- display: initial;
- font-size: 16px;
+ color: @almost-black;
+ font-size: 14px;
}
diff --git a/superset/assets/javascripts/dashboard/v2/util/findParentId.js b/superset/assets/javascripts/dashboard/v2/util/findParentId.js
new file mode 100644
index 0000000..0ca15a6
--- /dev/null
+++ b/superset/assets/javascripts/dashboard/v2/util/findParentId.js
@@ -0,0 +1,15 @@
+export default function findParentId({ childId, components = {} }) {
+ let parentId = null;
+
+ const ids = Object.keys(components);
+ for (let i = 0; i < ids.length - 1; i += 1) {
+ const id = ids[i];
+ const component = components[id] || {};
+ if (id !== childId && component.children && component.children.includes(childId)) {
+ parentId = id;
+ break;
+ }
+ }
+
+ return parentId;
+}
diff --git a/superset/assets/javascripts/dashboard/v2/util/newEntitiesFromDrop.js b/superset/assets/javascripts/dashboard/v2/util/newEntitiesFromDrop.js
index a554b08..a0d92fa 100644
--- a/superset/assets/javascripts/dashboard/v2/util/newEntitiesFromDrop.js
+++ b/superset/assets/javascripts/dashboard/v2/util/newEntitiesFromDrop.js
@@ -8,8 +8,6 @@ import {
TAB_TYPE,
} from './componentTypes';
-import { DASHBOARD_ROOT_ID } from './constants';
-
export default function newEntitiesFromDrop({ dropResult, components }) {
const { draggableId, destination } = dropResult;
@@ -29,7 +27,6 @@ export default function newEntitiesFromDrop({ dropResult, components }) {
const dropType = dropEntity.type;
let newDropChild = newComponentFactory(dragType);
const wrapChildInRow = shouldWrapChildInRow({ parentType: dropType, childType: dragType });
- const droppedOnRoot = dropEntity.id === DASHBOARD_ROOT_ID;
const newEntities = {
[newDropChild.id]: newDropChild,
@@ -44,27 +41,15 @@ export default function newEntitiesFromDrop({ dropResult, components }) {
const tabChild = newComponentFactory(TAB_TYPE);
newDropChild.children = [tabChild.id];
newEntities[tabChild.id] = tabChild;
-
- if (droppedOnRoot) {
- // If we've dropped tabs on the root, move previous root children to new tab
- const rootComponent = components[DASHBOARD_ROOT_ID];
- const topLevelId = rootComponent.children[0];
- const topLevelComponent = components[topLevelId];
- tabChild.children = [...topLevelComponent.children];
- newEntities[topLevelId] = { ...topLevelComponent, children: [] };
- newEntities[DASHBOARD_ROOT_ID] = { ...rootComponent, children: [newDropChild.id] };
- }
}
- if (!droppedOnRoot) {
- const nextDropChildren = [...dropEntity.children];
- nextDropChildren.splice(destination.index, 0, newDropChild.id);
+ const nextDropChildren = [...dropEntity.children];
+ nextDropChildren.splice(destination.index, 0, newDropChild.id);
- newEntities[destination.droppableId] = {
- ...dropEntity,
- children: nextDropChildren,
- };
- }
+ newEntities[destination.droppableId] = {
+ ...dropEntity,
+ children: nextDropChildren,
+ };
return newEntities;
}
--
To stop receiving notification emails like this one, please contact
ccwilliams@apache.org.