Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- [FP-2787](https://movai.atlassian.net/browse/FP-2787): IDE - Topics - Can't open multiple Topics tabs #323

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- [FP-2879](https://movai.atlassian.net/browse/FP-2879): Closing last tab keeps bookmarks
- [FP-2882](https://movai.atlassian.net/browse/FP-2882): Clicking in the bar under the documents loses context, not easy to get back
- [FP-2787](https://movai.atlassian.net/browse/FP-2787): IDE - Topics - Can't open multiple Topics tabs

# v1.2.4

Expand Down
49 changes: 27 additions & 22 deletions src/plugins/views/Tabs/hooks/useTabLayout.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { getIconByScope } from "../../../../utils/Utils";
import PluginManagerIDE from "../../../../engine/PluginManagerIDE/PluginManagerIDE";
import Workspace from "../../../../utils/Workspace";
import { getToolTabData } from "../../../../tools";
import { freeToolId } from "../tabHelpers";
import useTabStack from "./useTabStack";

const useTabLayout = (props, dockRef) => {
Expand All @@ -26,7 +27,7 @@ const useTabLayout = (props, dockRef) => {
const activeTabId = useRef(null);
const firstLoad = useRef(true);
const preventReloadNewDoc = useRef(false);
const tabsById = useRef(new Map());
const tabsByIdRef = useRef(new Map());
const [layout, setLayout] = useState({ ...DEFAULT_LAYOUT });
const { addTabToStack, removeTabFromStack, getNextTabFromStack } =
useTabStack(workspaceManager);
Expand All @@ -47,14 +48,14 @@ const useTabLayout = (props, dockRef) => {
if (!tab.extension) {
const toolName = tab.id;
const tabData = getToolTabData(tab, tab.tabProps);
tabsById.current.set(tabData.id, tabData);
workspaceManager.setTabs(tabsById.current);
tabsByIdRef.current.set(tabData.id, tabData);
workspaceManager.setTabs(tabsByIdRef.current);

// This fixes an issue where on first load a non editor tab
// was being added to the tabStack (HomeTab) and that meant
// that the last tab to enter tabStack would always be
// HomeTab, instead of the correct last tab
if(!firstLoad.current){
if (!firstLoad.current) {
addTabToStack(tabData, DOCK_POSITIONS.DOCK);
}

Expand Down Expand Up @@ -210,10 +211,10 @@ const useTabLayout = (props, dockRef) => {
const tabIndex = box.tabs.findIndex(_el => _el.id === prevTabId);
box.tabs[tabIndex] = tabData;
box.activeId = tabData.id;
tabsById.current.delete(prevTabId);
tabsById.current.set(tabData.id, tabData);
tabsByIdRef.current.delete(prevTabId);
tabsByIdRef.current.set(tabData.id, tabData);
addTabToStack(tabData, location);
workspaceManager.setTabs(tabsById.current);
workspaceManager.setTabs(tabsByIdRef.current);
workspaceManager.setLayout(newLayout);
}
return { newLayout, box };
Expand Down Expand Up @@ -339,7 +340,8 @@ const useTabLayout = (props, dockRef) => {
*/
const _onLayoutRemoveTab = useCallback(
(newLayout, tabId, forceClose) => {
const { name, scope, isNew, isDirty } = tabsById.current.get(tabId);
const { name, scope, isNew, isDirty, tabIncrement } =
tabsByIdRef.current.get(tabId);

if (isDirty && !forceClose) {
const document = { id: tabId, name, scope, isNew };
Expand All @@ -355,8 +357,9 @@ const useTabLayout = (props, dockRef) => {
}

// Remove tab and apply new layout
tabsById.current.delete(tabId);
workspaceManager.setTabs(tabsById.current);
tabsByIdRef.current.delete(tabId);
freeToolId({ scope, tabIncrement });
workspaceManager.setTabs(tabsByIdRef.current);
const dock = getDockFromTabId(tabId);
removeTabFromStack(tabId, dock);
applyLayout(newLayout);
Expand Down Expand Up @@ -423,13 +426,13 @@ const useTabLayout = (props, dockRef) => {
data => {
const { instance: model, value: isDirty } = data;
const tabId = model.getUrl();
const currentTabData = tabsById.current.get(tabId);
const currentTabData = tabsByIdRef.current.get(tabId);
const currentDirtyState = Boolean(currentTabData?.isDirty);
// Doesn't trigger update if dirty state didn't change
if (!currentTabData || currentDirtyState === isDirty) return;
// Set new dirty state
const newTabData = { ...currentTabData, isDirty: isDirty };
tabsById.current.set(tabId, newTabData);
tabsByIdRef.current.set(tabId, newTabData);
// Trigger tab update
if (!dockRef.current) return;
const currentTab = findTab(tabId);
Expand Down Expand Up @@ -538,8 +541,8 @@ const useTabLayout = (props, dockRef) => {

emit(PLUGINS.TABS.ON.ACTIVE_TAB_CHANGE, { id: tabData.id });
addTabToStack(tabData, tabPosition);
tabsById.current.set(tabData.id, tabData);
workspaceManager.setTabs(tabsById.current);
tabsByIdRef.current.set(tabData.id, tabData);
workspaceManager.setTabs(tabsByIdRef.current);

const existingTab = findTab(tabData.id);
if (existingTab) {
Expand Down Expand Up @@ -631,7 +634,7 @@ const useTabLayout = (props, dockRef) => {
*/
const loadTab = useCallback(
data => {
const tabFromMemory = tabsById.current.get(data.id);
const tabFromMemory = tabsByIdRef.current.get(data.id);
if (!tabFromMemory && !data.content) return;
const {
id,
Expand All @@ -642,9 +645,10 @@ const useTabLayout = (props, dockRef) => {
extension,
isDirty,
isNew,
tabIncrement,
tabProps
} = tabFromMemory ?? data;
tabsById.current.set(id, {
tabsByIdRef.current.set(id, {
id,
scope,
name,
Expand All @@ -653,9 +657,10 @@ const useTabLayout = (props, dockRef) => {
extension,
isNew,
isDirty,
tabIncrement,
tabProps
});
const tabData = { id, scope, name, tabTitle, extension };
const tabData = { id, scope, name, tabTitle, tabIncrement, extension };
return {
id: id,
title: _getCustomTab(tabData, _closeTab, isDirty),
Expand All @@ -676,7 +681,7 @@ const useTabLayout = (props, dockRef) => {
(newLayout, tabId, direction) => {
const isActuallyTabChange = activeTabId.current !== tabId;
const dock = getDockFromTabId(tabId);
const tabData = tabsById.current.get(tabId);
const tabData = tabsByIdRef.current.get(tabId);
let newActiveTabId = tabId;

// Attempt to close tab
Expand All @@ -697,7 +702,7 @@ const useTabLayout = (props, dockRef) => {
// Emit new active tab id
if (!tabId) return;

if(isActuallyTabChange){
if (isActuallyTabChange) {
activeTabId.current = newActiveTabId;
emit(PLUGINS.TABS.ON.ACTIVE_TAB_CHANGE, { id: newActiveTabId });
}
Expand Down Expand Up @@ -743,7 +748,7 @@ const useTabLayout = (props, dockRef) => {
* @returns {string} active tab id
*/
const getActiveTab = useCallback(() => {
return tabsById.current.get(activeTabId.current);
return tabsByIdRef.current.get(activeTabId.current);
}, []);

/**
Expand Down Expand Up @@ -819,7 +824,7 @@ const useTabLayout = (props, dockRef) => {

layoutActiveIdIsValid(lastLayout);

tabsById.current = lastTabs;
tabsByIdRef.current = lastTabs;
// Install current tabs plugins
lastTabs.forEach(tab => {
const { content, ...others } = tab;
Expand All @@ -829,7 +834,7 @@ const useTabLayout = (props, dockRef) => {
Promise.allSettled(tabs).then(_tabs => {
_tabs.forEach(tab => {
tab.status === "fulfilled" &&
tabsById.current.set(tab.value.id, tab.value);
tabsByIdRef.current.set(tab.value.id, tab.value);
});
setLayout(lastLayout);

Expand Down
66 changes: 66 additions & 0 deletions src/plugins/views/Tabs/tabHelpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import Workspace from "./../../../utils/Workspace";

// This is not a new instance as Workspace is a singleton
const workspace = new Workspace();
// Extract only the tabs that have tabIncrement
const tabsMap = [...workspace.getTabs().values()].filter(x => x.tabIncrement);
// Compute the tool tab map
export const toolIds = computeTabMap(tabsMap);

/* get the structure from the tabsMap that will allow us
* to efficently organize tool ids.
*/
function computeTabMap(tabsMap) {
const computedToolTabMap = {};

/* Compute a map of this format:
* {
* Topics: { last: 3, busy: [2] },
* VarsDebug: { last 2, busy: [1] },
* }
*/
for (const tabData of tabsMap) {
const { scope, tabIncrement } = tabData;

const tabIncrementStatusData = computedToolTabMap[scope] ?? {
last: 1,
busy: {},
free: []
};
tabIncrementStatusData.busy[tabIncrement] = 1;

if (tabIncrement > tabIncrementStatusData.last)
tabIncrementStatusData.last = tabIncrement;

computedToolTabMap[scope] = tabIncrementStatusData;
}

/* turn it into:
* {
* Topics: { last: 3, free: [1] },
* VarsDebug: { last 2, free: [] },
* }
*/
for (const toolScope in computedToolTabMap) {
const tabIncrementStatusData = computedToolTabMap[toolScope];

for (let i = 1; i < tabIncrementStatusData.last; i++)
if (!tabIncrementStatusData.busy[i]) tabIncrementStatusData.free.push(i);

delete tabIncrementStatusData.busy;
tabIncrementStatusData.last++;
}

return computedToolTabMap;
}

/* free a tool id */
export function freeToolId(tabData) {
if (!tabData.tabIncrement) return;

const toolIdData = toolIds[tabData.scope];
if (toolIdData) {
toolIdData.free.unshift(tabData.tabIncrement);
toolIdData.free.sort();
}
}
1 change: 1 addition & 0 deletions src/stories/components/Logs/Logs.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export const getLogsToolTab = () => {
name: LOGS_PROFILE.title,
tabTitle: LOGS_PROFILE.title,
scope: LOGS_PROFILE.name,
multiple: true,
extension: ""
};
};
2 changes: 1 addition & 1 deletion src/tools/NotInstalled/NotInstalled.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const NotInstalled = props => {

export default NotInstalled;

export const getTabData = (id, name) => {
export const getNotInstalledTabData = (id, name) => {
const tabTitle = name || id;
return {
id: id,
Expand Down
12 changes: 6 additions & 6 deletions src/tools/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from "react";
import PluginManagerIDE from "../engine/PluginManagerIDE/PluginManagerIDE";
import { getTabData } from "./NotInstalled/NotInstalled";
import { getNotInstalledTabData } from "./NotInstalled/NotInstalled";

const APP_TOOLS = {};

Expand All @@ -9,12 +9,12 @@ export const addTool = (name, tool) => {
};

export const getToolTabData = (tab, props = {}) => {
const { id, name } = tab;
const notInstalledTab = getTabData(id, name);
const data = id in APP_TOOLS ? APP_TOOLS[id].tabData : notInstalledTab;
const { name, scope } = tab;
const notInstalledTab = getNotInstalledTabData(scope, name);
const data = scope in APP_TOOLS ? APP_TOOLS[scope].tabData : notInstalledTab;
// Sanitize tab data to avoid TypeError: Converting circular structure to JSON
if (!data.content) {
const plugin = PluginManagerIDE.getPlugin(id);
const plugin = PluginManagerIDE.getPlugin(scope);
data.content = plugin.render(props) || notInstalledTab.content;
}
if (props.tabTitle) data.name = props.tabTitle;
Expand All @@ -24,7 +24,7 @@ export const getToolTabData = (tab, props = {}) => {
data.content = React.cloneElement(data.content, mergedProps);
}

return data;
return { ...data, ...tab };
};

export const hasTool = name => {
Expand Down
28 changes: 24 additions & 4 deletions src/utils/generalFunctions.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { i18n } from "@mov-ai/mov-fe-lib-react";
import AppSettings from "../App/AppSettings";
import { PLUGINS } from "../utils/Constants";
import movaiIconWhite from "../Branding/movai-logo-white.png";
import { PLUGINS } from "../utils/Constants";
import { toolIds } from "../plugins/views/Tabs/tabHelpers";
import { getHomeTab } from "../tools/HomeTab/HomeTab";
import { getShortcutsTab } from "../tools/AppShortcuts/AppShortcuts";
import { getToolTabData } from "../tools";
Expand Down Expand Up @@ -70,10 +71,29 @@ export function aboutPopup(call, classes = {}) {
/**
* Open Tool tab
* @param {function} call : Plugin call method
* @param {string} toolName : Tool Unique Name
* @param {string} toolScope : Tool Unique Scope
*/
export function openTool(call, toolName = getHomeTab().id, props = {}) {
const tabData = getToolTabData({ id: toolName }, props);
export function openTool(call, toolScope = getHomeTab().scope, props = {}) {
const tabData = getToolTabData({ scope: toolScope }, props);

if (tabData.tabIncrement) {
const toolIdData = toolIds[toolScope] ?? { last: 1, free: [] };
const currentIncrement = toolIdData.free.shift() ?? toolIdData.last++;

tabData.id += "_" + currentIncrement;

// We don't want to have a Topics 1.
// First opened tool should be just the tool name.
// From then on, yes, increment, so Topics, Topics 2, etc.
if (currentIncrement > 1) {
tabData.name += " " + currentIncrement;
tabData.title += " " + currentIncrement;
tabData.tabIncrement = currentIncrement;
}

toolIds[toolScope] = toolIdData;
}

call(PLUGINS.TABS.NAME, PLUGINS.TABS.CALL.OPEN, tabData);
}

Expand Down
Loading