You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "fisjac (via GitHub)" <gi...@apache.org> on 2024/05/28 15:05:25 UTC
[I] Permalinks add all tabs ever added to state [superset]
fisjac opened a new issue, #28741:
URL: https://github.com/apache/superset/issues/28741
### 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
- [X] I have searched Superset docs and Slack and didn't find a solution to my problem.
- [ ] I have searched the GitHub issue tracker and didn't find a similar bug report.
- [ ] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] New permalinks retain previous permalink activeTab state [superset]
Posted by "dosubot[bot] (via GitHub)" <gi...@apache.org>.
dosubot[bot] commented on issue #28741:
URL: https://github.com/apache/superset/issues/28741#issuecomment-2135484688
<!-- Greeting -->
Hey @fisjac! 👋 I'm here to help you with any bugs, questions, or contributions you have for the repository. Let's tackle this together!
<!-- Answer -->
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:
```python
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`:
```js
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 <sup>[[1]](https://github.com/apache/superset/blob/master/superset/dashboards/permalink/schemas.py)</sup><sup>[[2]](https://github.com/apache/superset/blob/master/superset-frontend/src/dashboard/reducers/dashboardState.js)</sup>.
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:
```tsx
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 <sup>[[3]](https://github.com/apache/superset/blob/master/superset-frontend/src/dashboard/containers/DashboardPage.tsx)</sup>.
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:
```jsx
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 <sup>[[4]](https://github.com/apache/superset/blob/master/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx)</sup>.
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 <sup>[[5]](https://github.com/apache/superset/blob/master/superset/examples/tabbed_dashboard.py)</sup>.
<!-- Dosu Comment Footer -->
<details open>
<summary>
#### Details
</summary>
**About Dosu**
This response is meant to be useful and save you time. It is **not meant to be a precise solution**, but rather a starting point for your own research.
Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.
</details>
*To continue the conversation, mention @dosu.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] New permalinks retain previous permalink activeTab state [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido closed issue #28741: New permalinks retain previous permalink activeTab state
URL: https://github.com/apache/superset/issues/28741
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org