Closed fisjac closed 4 months ago
Hey @fisjac! 👋 I'm here to help you with any bugs, questions, or contributions you have for the repository. Let's tackle this together!
To resolve the issue where generating a new permalink in your dashboard appends the new activeTab
state to the previous state, causing incorrect nesting configurations, you need to ensure that the activeTabs
field in the DashboardPermalinkStateSchema
is correctly reset or updated without retaining the previous state.
Here is an example of how you might reset or update the activeTabs
field when generating a new permalink:
from marshmallow import Schema, fields
class DashboardPermalinkStateSchema(Schema):
dataMask = fields.Dict(
required=False,
allow_none=True,
metadata={"description": "Data mask used for native filter state"},
)
activeTabs = fields.List(
fields.String(),
required=False,
allow_none=True,
metadata={"description": "Current active dashboard tabs"},
)
urlParams = fields.List(
fields.Tuple(
(
fields.String(
required=True, allow_none=True, metadata={"description": "Key"}
),
fields.String(
required=True, allow_none=True, metadata={"description": "Value"}
),
),
required=False,
allow_none=True,
metadata={"description": "URL Parameter key-value pair"},
),
required=False,
allow_none=True,
metadata={"description": "URL Parameters"},
)
anchor = fields.String(
required=False,
allow_none=True,
metadata={"description": "Optional anchor link added to url hash"},
)
class DashboardPermalinkSchema(Schema):
dashboardId = fields.String(
required=True,
allow_none=False,
metadata={"description": "The id or slug of the dashboard"},
)
state = fields.Nested(DashboardPermalinkStateSchema())
# Example function to generate a new permalink
def generate_new_permalink(dashboard_id, new_active_tabs):
# Create a new state with the updated activeTabs
new_state = {
"dataMask": None,
"activeTabs": new_active_tabs,
"urlParams": None,
"anchor": None,
}
# Create the permalink data
permalink_data = {
"dashboardId": dashboard_id,
"state": new_state,
}
# Serialize the data using the schema
schema = DashboardPermalinkSchema()
result = schema.dump(permalink_data)
return result
# Example usage
new_permalink = generate_new_permalink("dashboard_123", ["tab1", "tab2"])
print(new_permalink)
Additionally, ensure that your Redux reducer handling the SET_ACTIVE_TAB
and SET_ACTIVE_TABS
actions is correctly updating the state without retaining the previous activeTabs
state. Here is an example from the dashboardStateReducer
:
import {
SET_ACTIVE_TAB,
SET_ACTIVE_TABS,
} from '../actions/dashboardState';
export default function dashboardStateReducer(state = {}, action) {
const actionHandlers = {
[SET_ACTIVE_TAB]() {
const newActiveTabs = new Set(state.activeTabs);
newActiveTabs.delete(action.prevTabId);
newActiveTabs.add(action.tabId);
return {
...state,
activeTabs: Array.from(newActiveTabs),
};
},
[SET_ACTIVE_TABS]() {
return {
...state,
activeTabs: action.activeTabs,
};
},
};
if (action.type in actionHandlers) {
return actionHandlers[action.type]();
}
return state;
}
In this example, the SET_ACTIVE_TABS
action directly sets the activeTabs
state to the new value provided in the action, ensuring that the previous state is not carried over. This should resolve the issue of incorrect nesting configurations when generating a new permalink [1][2].
Additionally, in the DashboardPage.tsx
file, ensure that the activeTabs
state is properly initialized and does not carry over any previous state when hydrating the dashboard:
useEffect(() => {
async function getDataMaskApplied() {
const permalinkKey = getUrlParam(URL_PARAMS.permalinkKey);
const nativeFilterKeyValue = getUrlParam(URL_PARAMS.nativeFiltersKey);
const isOldRison = getUrlParam(URL_PARAMS.nativeFilters);
let dataMask = nativeFilterKeyValue || {};
let activeTabs: string[] | undefined;
if (permalinkKey) {
const permalinkValue = await getPermalinkValue(permalinkKey);
if (permalinkValue) {
({ dataMask, activeTabs } = permalinkValue.state);
}
} else if (nativeFilterKeyValue) {
dataMask = await getFilterValue(id, nativeFilterKeyValue);
}
if (isOldRison) {
dataMask = isOldRison;
}
if (readyToRender) {
if (!isDashboardHydrated.current) {
isDashboardHydrated.current = true;
}
dispatch(
hydrateDashboard({
history,
dashboard,
charts,
activeTabs,
dataMask,
}),
);
}
return null;
}
if (id) getDataMaskApplied();
}, [readyToRender]);
This ensures that the activeTabs
state is correctly initialized and does not retain any previous state when hydrating the dashboard [3].
Furthermore, in the Tabs.jsx
component, ensure that the activeTabs
state is correctly managed and updated. Here is an example from the Tabs.jsx
file:
import React from 'react';
import PropTypes from 'prop-types';
import { styled, t } from '@superset-ui/core';
import { connect } from 'react-redux';
import { LineEditableTabs } from 'src/components/Tabs';
import { LOG_ACTIONS_SELECT_DASHBOARD_TAB } from 'src/logger/LogUtils';
import { AntdModal } from 'src/components';
import { Draggable } from '../dnd/DragDroppable';
import DragHandle from '../dnd/DragHandle';
import DashboardComponent from '../../containers/DashboardComponent';
import DeleteComponentButton from '../DeleteComponentButton';
import HoverMenu from '../menu/HoverMenu';
import findTabIndexByComponentId from '../../util/findTabIndexByComponentId';
import getDirectPathToTabIndex from '../../util/getDirectPathToTabIndex';
import getLeafComponentIdFromPath from '../../util/getLeafComponentIdFromPath';
import { componentShape } from '../../util/propShapes';
import { NEW_TAB_ID } from '../../util/constants';
import { RENDER_TAB, RENDER_TAB_CONTENT } from './Tab';
import { TABS_TYPE, TAB_TYPE } from '../../util/componentTypes';
const propTypes = {
id: PropTypes.string.isRequired,
parentId: PropTypes.string.isRequired,
component: componentShape.isRequired,
parentComponent: componentShape.isRequired,
index: PropTypes.number.isRequired,
depth: PropTypes.number.isRequired,
renderTabContent: PropTypes.bool,
editMode: PropTypes.bool.isRequired,
renderHoverMenu: PropTypes.bool,
directPathToChild: PropTypes.arrayOf(PropTypes.string),
activeTabs: PropTypes.arrayOf(PropTypes.string),
logEvent: PropTypes.func.isRequired,
setActiveTab: PropTypes.func,
availableColumnCount: PropTypes.number,
columnWidth: PropTypes.number,
onResizeStart: PropTypes.func,
onResize: PropTypes.func,
onResizeStop: PropTypes.func,
createComponent: PropTypes.func.isRequired,
handleComponentDrop: PropTypes.func.isRequired,
onChangeTab: PropTypes.func.isRequired,
deleteComponent: PropTypes.func.isRequired,
updateComponents: PropTypes.func.isRequired,
};
const defaultProps = {
renderTabContent: true,
renderHoverMenu: true,
availableColumnCount: 0,
columnWidth: 0,
activeTabs: [],
directPathToChild: [],
setActiveTab() {},
onResizeStart() {},
onResize() {},
onResizeStop() {},
};
const StyledTabsContainer = styled.div`
width: 100%;
background-color: ${({ theme }) => theme.colors.grayscale.light5};
.dashboard-component-tabs-content {
min-height: ${({ theme }) => theme.gridUnit * 12}px;
margin-top: ${({ theme }) => theme.gridUnit / 4}px;
position: relative;
}
.ant-tabs {
overflow: visible;
.ant-tabs-nav-wrap {
min-height: ${({ theme }) => theme.gridUnit * 12.5}px;
}
.ant-tabs-content-holder {
overflow: visible;
}
}
div .ant-tabs-tab-btn {
text-transform: none;
}
`;
export class Tabs extends React.PureComponent {
constructor(props) {
super(props);
const { tabIndex, activeKey } = this.getTabInfo(props);
this.state = {
tabIndex,
activeKey,
};
this.handleClickTab = this.handleClickTab.bind(this);
this.handleDeleteComponent = this.handleDeleteComponent.bind(this);
this.handleDeleteTab = this.handleDeleteTab.bind(this);
this.handleDropOnTab = this.handleDropOnTab.bind(this);
this.handleDrop = this.handleDrop.bind(this);
}
componentDidMount() {
this.props.setActiveTab(this.state.activeKey);
}
componentDidUpdate(prevProps, prevState) {
if (prevState.activeKey !== this.state.activeKey) {
this.props.setActiveTab(this.state.activeKey, prevState.activeKey);
}
}
UNSAFE_componentWillReceiveProps(nextProps) {
const maxIndex = Math.max(0, nextProps.component.children.length - 1);
const currTabsIds = this.props.component.children;
const nextTabsIds = nextProps.component.children;
if (this.state.tabIndex > maxIndex) {
this.setState(() => ({ tabIndex: maxIndex }));
}
if (nextProps.dashboardId !== this.props.dashboardId) {
const { tabIndex, activeKey } = this.getTabInfo(nextProps);
this.setState(() => ({
tabIndex,
activeKey,
}));
}
if (nextProps.isComponentVisible) {
const nextFocusComponent = getLeafComponentIdFromPath(
nextProps.directPathToChild,
);
const currentFocusComponent = getLeafComponentIdFromPath(
this.props.directPathToChild,
);
if (
nextFocusComponent !== currentFocusComponent ||
(nextFocusComponent === currentFocusComponent &&
currTabsIds !== nextTabsIds)
) {
const nextTabIndex = findTabIndexByComponentId({
currentComponent: nextProps.component,
directPathToChild: nextProps.directPathToChild,
});
if (nextTabIndex > -1 && nextTabIndex !== this.state.tabIndex) {
this.setState(() => ({
tabIndex: nextTabIndex,
activeKey: nextTabsIds[nextTabIndex],
}));
}
}
}
}
getTabInfo = props => {
let tabIndex = Math.max(
0,
findTabIndexByComponentId({
currentComponent: props.component,
directPathToChild: props.directPathToChild,
}),
);
if (tabIndex === 0 && props.activeTabs?.length) {
props.component.children.forEach((tabId, index) => {
if (tabIndex === 0 && props.activeTabs.includes(tabId)) {
tabIndex = index;
}
});
}
const { children: tabIds } = props.component;
const activeKey = tabIds[tabIndex];
return {
tabIndex,
activeKey,
};
};
showDeleteConfirmModal = key => {
const { component, deleteComponent } = this.props;
AntdModal.confirm({
title: t('Delete dashboard tab?'),
content: (
<span>
{t(
'Deleting a tab will remove all content within it. You may still ' +
'reverse this action with the',
)}{' '}
<b>{t('undo')}</b>{' '}
{t('button (cmd + z) until you save your changes.')}
</span>
),
onOk: () => {
deleteComponent(key, component.id);
const tabIndex = component.children.indexOf(key);
this.handleDeleteTab(tabIndex);
},
okType: 'danger',
okText: t('DELETE'),
cancelText: t('CANCEL'),
icon: null,
});
};
handleEdit = (event, action) => {
const { component, createComponent } = this.props. If (action === 'add') {
event?.stopPropagation?.();
createComponent({
destination: {
id: component.id,
type: component.type,
index: component.children.length,
},
dragging: {
id: NEW_TAB_ID,
type: TAB_TYPE,
},
});
} else if (action === 'remove') {
this.showDeleteConfirmModal(event);
}
};
handleClickTab(tabIndex) {
const { component } = this.props;
const { children: tabIds } = component;
if (tabIndex !== this.state.tabIndex) {
const pathToTabIndex = getDirectPathToTabIndex(component, tabIndex);
const targetTabId = pathToTabIndex[pathToTabIndex.length - 1];
this.props.logEvent(LOG_ACTIONS_SELECT_DASHBOARD_TAB, {
target_id: targetTabId,
index: tabIndex,
});
this.props.onChangeTab({ pathToTabIndex });
}
this.setState(() => ({ activeKey: tabIds[tabIndex] }));
}
handleDeleteComponent() {
const { deleteComponent, id, parentId } = this.props;
deleteComponent(id, parentId);
}
handleDeleteTab(tabIndex) {
if (this.state.tabIndex === tabIndex) {
this.handleClickTab(Math.max(0, tabIndex - 1));
}
}
handleDropOnTab(dropResult) {
const { component } = this.props;
const { destination } = dropResult;
if (destination) {
const dropTabIndex =
destination.id === component.id
? destination.index
: component.children.indexOf(destination.id);
if (dropTabIndex > -1) {
setTimeout(() => {
this.handleClickTab(dropTabIndex);
}, 30);
}
}
}
handleDrop(dropResult) {
if (dropResult.dragging.type !== TABS_TYPE) {
this.props.handleComponentDrop(dropResult);
}
}
render() {
const {
depth,
component: tabsComponent,
parentComponent,
index,
availableColumnCount,
columnWidth,
onResizeStart,
onResize,
onResizeStop,
renderTabContent,
renderHoverMenu,
isComponentVisible: isCurrentTabVisible,
editMode,
nativeFilters,
} = this.props;
const { children: tabIds } = tabsComponent;
const { tabIndex: selectedTabIndex, activeKey } = this.state;
let tabsToHighlight;
const highlightedFilterId =
nativeFilters?.focusedFilterId || nativeFilters?.hoveredFilterId;
if (highlightedFilterId) {
tabsToHighlight = nativeFilters.filters[highlightedFilterId]?.tabsInScope;
}
return (
<Draggable
component={tabsComponent}
parentComponent={parentComponent}
orientation="row"
index={index}
depth={depth}
onDrop={this.handleDrop}
editMode={editMode}
>
{({ dragSourceRef: tabsDragSourceRef }) => (
<StyledTabsContainer
className="dashboard-component dashboard-component-tabs"
data-test="dashboard-component-tabs"
>
{editMode && renderHoverMenu && (
<HoverMenu innerRef={tabsDragSourceRef} position="left">
<DragHandle position="left" />
<DeleteComponentButton onDelete={this.handleDeleteComponent} />
</HoverMenu>
)}
<LineEditableTabs
id={tabsComponent.id}
activeKey={activeKey}
onChange={key => {
this.handleClickTab(tabIds.indexOf(key));
}}
onEdit={this.handleEdit}
data-test="nav-list"
type={editMode ? 'editable-card' : 'card'}
>
{tabIds.map((tabId, tabIndex) => (
<LineEditableTabs.TabPane
key={tabId}
tab={
<DashboardComponent
id={tabId}
parentId={tabsComponent.id}
depth={depth}
index={tabIndex}
renderType={RENDER_TAB}
availableColumnCount={availableColumnCount}
columnWidth={columnWidth}
onDropOnTab={this.handleDropOnTab}
onHoverTab={() => this.handleClickTab(tabIndex)}
isFocused={activeKey === tabId}
isHighlighted={
activeKey !== tabId && tabsToHighlight?.includes(tabId)
}
/>
}
>
{renderTabContent && (
<DashboardComponent
id={tabId}
parentId={tabsComponent.id}
depth={depth}
index={tabIndex}
renderType={RENDER_TAB_CONTENT}
availableColumnCount={availableColumnCount}
columnWidth={columnWidth}
onResizeStart={onResizeStart}
onResize={onResize}
onResizeStop={onResizeStop}
onDropOnTab={this.handleDropOnTab}
isComponentVisible={
selectedTabIndex === tabIndex && isCurrentTabVisible
}
/>
)}
</LineEditableTabs.TabPane>
))}
</LineEditableTabs>
</StyledTabsContainer>
)}
</Draggable>
);
}
}
Tabs.propTypes = propTypes;
Tabs.defaultProps = defaultProps;
function mapStateToProps(state) {
return {
nativeFilters: state.nativeFilters,
activeTabs: state.dashboardState.activeTabs,
directPathToChild: state.dashboardState.directPathToChild,
};
}
export default connect(mapStateToProps)(Tabs);
This ensures that the activeTabs
state is correctly managed and updated within the Tabs
component, preventing the issue of incorrect nesting configurations when generating a new permalink [4].
Additionally, you can refer to the load_tabbed_dashboard
function in the superset/examples/tabbed_dashboard.py
file for an example of creating a dashboard with nested tabs. This function demonstrates how to structure the dashboard's JSON configuration to handle nested tabs correctly [5].
To continue the conversation, mention @dosu.
Bug description
When using permalinks to create views of a dashboard in a given tab state, generating a permalink will store the active tabs within state. When generating a new permalink, the previous state is not cleared, rather the new activeTab state is appended to the previous state resulting in strange behavior such as new nesting configurations such as those shown in the screen recording in which tabs nested under "Parent 1" are now visible underneath "Parent 2."
How to reproduce the bug
Select a dashboard that contains multiple nesting layers and at least two parent tabs ["Parent 1", "Parent 2"] Select a child tab from "Parent 1". ["P1 - T1"] should now be selected Generate a permalink and copy to clipboard. Select a child from "Parent 2". ["P2 - T1"] should now be selected Generate a permalink and copy to clipboard use the second permalink in the url and navigate to the dashboard dashboard state will load a combination of previously selected permalink tabs.
Screenshots/recordings
https://www.loom.com/share/60d642624a2b4f039572610723781987
Superset version
master / latest-dev
Python version
3.9
Node version
16
Browser
Chrome
Additional context
No response
Checklist