Skip to content

Commit

Permalink
fix(dashboard): Race condition when setting activeTabs with nested ta…
Browse files Browse the repository at this point in the history
…bs (#17007)

* fix(dashboard): Race condition when setting activeTabs with nested tabs

* Remove activeTabs prop from Tabs
  • Loading branch information
kgabryje committed Oct 7, 2021
1 parent 6cdb324 commit 45908ff
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 11 deletions.
4 changes: 2 additions & 2 deletions superset-frontend/src/dashboard/actions/dashboardState.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,8 @@ export function setDirectPathToChild(path) {
}

export const SET_ACTIVE_TABS = 'SET_ACTIVE_TABS';
export function setActiveTabs(tabIds) {
return { type: SET_ACTIVE_TABS, tabIds };
export function setActiveTabs(tabId, prevTabId) {
return { type: SET_ACTIVE_TABS, tabId, prevTabId };
}

export const SET_FOCUSED_FILTER_FIELD = 'SET_FOCUSED_FILTER_FIELD';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ const propTypes = {
editMode: PropTypes.bool.isRequired,
renderHoverMenu: PropTypes.bool,
directPathToChild: PropTypes.arrayOf(PropTypes.string),
activeTabs: PropTypes.arrayOf(PropTypes.string),

// actions (from DashboardComponent.jsx)
logEvent: PropTypes.func.isRequired,
Expand All @@ -74,7 +73,6 @@ const defaultProps = {
availableColumnCount: 0,
columnWidth: 0,
directPathToChild: [],
activeTabs: [],
setActiveTabs() {},
onResizeStart() {},
onResize() {},
Expand Down Expand Up @@ -133,15 +131,12 @@ export class Tabs extends React.PureComponent {
}

componentDidMount() {
this.props.setActiveTabs([...this.props.activeTabs, this.state.activeKey]);
this.props.setActiveTabs(this.state.activeKey);
}

componentDidUpdate(prevProps, prevState) {
if (prevState.activeKey !== this.state.activeKey) {
this.props.setActiveTabs([
...this.props.activeTabs.filter(tabId => tabId !== prevState.activeKey),
this.state.activeKey,
]);
this.props.setActiveTabs(this.state.activeKey, prevState.activeKey);
}
}

Expand Down Expand Up @@ -410,7 +405,6 @@ function mapStateToProps(state) {
return {
nativeFilters: state.nativeFilters,
directPathToChild: state.dashboardState.directPathToChild,
activeTabs: state.dashboardState.activeTabs,
};
}
export default connect(mapStateToProps)(Tabs);
9 changes: 8 additions & 1 deletion superset-frontend/src/dashboard/reducers/dashboardState.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,16 @@ export default function dashboardStateReducer(state = {}, action) {
};
},
[SET_ACTIVE_TABS]() {
const prevActiveTabs = state.activeTabs ?? [];
const newActiveTabs = action.prevTabId
? [
...prevActiveTabs.filter(tabId => tabId !== action.prevTabId),
action.tabId,
]
: [...prevActiveTabs, action.tabId];
return {
...state,
activeTabs: action.tabIds,
activeTabs: newActiveTabs,
};
},
[SET_FOCUSED_FILTER_FIELD]() {
Expand Down

0 comments on commit 45908ff

Please sign in to comment.