Skip to content

Commit 36f3e08

Browse files
CompuIvesCompuIves
authored
Preview Tabs Fixes (codesandbox#1695)
Add support for ?previewwindow and prevent duplicate tabs. Generally improves the function responsible for moving tabs since the mutations now happen in `immer`. There's now a smaller chance of mutation weirdness. Fixes codesandbox#1672 Fixes codesandbox#1651
1 parent 13ad591 commit 36f3e08

File tree

7 files changed

+320
-42
lines changed

7 files changed

+320
-42
lines changed

‎packages/app/package.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
"@types/codemirror": "^0.0.72",
4343
"@types/debug": "^4.1.1",
4444
"@types/gsap": "^1.20.1",
45+
"@types/jest": "^24.0.11",
4546
"@types/lodash-es": "^4.17.2",
4647
"@types/react": "^16.8.3",
4748
"@types/react-dom": "^16.0.11",
@@ -273,9 +274,18 @@
273274
"testPathIgnorePatterns": [
274275
"/create-zip\\/.*\\/files/"
275276
],
277+
"testRegex": "(test|spec)\\.(j|t)sx?$",
276278
"transformIgnorePatterns": [
277279
"node_modules/(?!(common|lodash-es|sandbox-hooks|react-icons)/.+\\.js$)"
278280
],
281+
"moduleFileExtensions": [
282+
"ts",
283+
"tsx",
284+
"js",
285+
"jsx",
286+
"json",
287+
"node"
288+
],
279289
"moduleNameMapper": {
280290
"\\.css$": "<rootDir>/__mocks__/styleMock.js",
281291
"\\.html$": "<rootDir>/__mocks__/styleMock.js",

‎packages/app/src/app/pages/Sandbox/Editor/Content/__snapshots__/utils.test.ts.snap‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`moveDevToolsTab can move a tab from one devtools to the other 1`] = `
4+
Array [
5+
Object {
6+
"views": Array [
7+
Object {
8+
"id": "codesandbox.tests",
9+
},
10+
],
11+
},
12+
Object {
13+
"views": Array [
14+
Object {
15+
"id": "codesandbox.console",
16+
},
17+
Object {
18+
"id": "codesandbox.browser",
19+
},
20+
Object {
21+
"id": "codesandbox.problems",
22+
},
23+
],
24+
},
25+
]
26+
`;
27+
28+
exports[`moveDevToolsTab can move a tab from one devtools to the other and move it back 1`] = `
29+
Array [
30+
Object {
31+
"views": Array [
32+
Object {
33+
"id": "codesandbox.browser",
34+
},
35+
Object {
36+
"id": "codesandbox.tests",
37+
},
38+
],
39+
},
40+
Object {
41+
"views": Array [
42+
Object {
43+
"id": "codesandbox.console",
44+
},
45+
Object {
46+
"id": "codesandbox.problems",
47+
},
48+
],
49+
},
50+
]
51+
`;
52+
53+
exports[`moveDevToolsTab can move a tab in same devtools 1`] = `
54+
Array [
55+
Object {
56+
"views": Array [
57+
Object {
58+
"id": "codesandbox.tests",
59+
},
60+
Object {
61+
"id": "codesandbox.browser",
62+
},
63+
],
64+
},
65+
Object {
66+
"views": Array [
67+
Object {
68+
"id": "codesandbox.console",
69+
},
70+
Object {
71+
"id": "codesandbox.problems",
72+
},
73+
],
74+
},
75+
]
76+
`;
77+
78+
exports[`moveDevToolsTab can move a tab in same devtools and move the tab back 1`] = `
79+
Array [
80+
Object {
81+
"views": Array [
82+
Object {
83+
"id": "codesandbox.browser",
84+
},
85+
Object {
86+
"id": "codesandbox.tests",
87+
},
88+
],
89+
},
90+
Object {
91+
"views": Array [
92+
Object {
93+
"id": "codesandbox.console",
94+
},
95+
Object {
96+
"id": "codesandbox.problems",
97+
},
98+
],
99+
},
100+
]
101+
`;
102+
103+
exports[`moveDevToolsTab can move the tab to the same position 1`] = `
104+
Array [
105+
Object {
106+
"views": Array [
107+
Object {
108+
"id": "codesandbox.browser",
109+
},
110+
Object {
111+
"id": "codesandbox.tests",
112+
},
113+
],
114+
},
115+
Object {
116+
"views": Array [
117+
Object {
118+
"id": "codesandbox.console",
119+
},
120+
Object {
121+
"id": "codesandbox.problems",
122+
},
123+
],
124+
},
125+
]
126+
`;

‎packages/app/src/app/pages/Sandbox/Editor/Content/index.js‎

Lines changed: 49 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import { Prompt } from 'react-router-dom';
55
import { reaction } from 'mobx';
66
import { TextOperation } from 'ot';
77
import { inject, observer } from 'mobx-react';
8+
import immer from 'immer';
89

10+
import { getSandboxOptions } from '@codesandbox/common/lib/url';
911
import getTemplateDefinition from '@codesandbox/common/lib/templates';
1012
import type { ModuleError } from '@codesandbox/common/lib/types';
1113
import { getPreviewTabs } from '@codesandbox/common/lib/templates/devtools';
@@ -18,6 +20,7 @@ import DevTools from 'app/components/Preview/DevTools';
1820
import Preview from './Preview';
1921
import preventGestureScroll, { removeListener } from './prevent-gesture-scroll';
2022
import Tabs from './Tabs';
23+
import { moveDevToolsTab } from './utils';
2124

2225
const settings = store =>
2326
({
@@ -376,29 +379,57 @@ class EditorPreview extends React.Component<Props, State> {
376379
});
377380
};
378381

379-
moveDevToolsTab = (prevPos, nextPos) => {
380-
const { store, signals } = this.props;
381-
const tabs = getPreviewTabs(store.editor.currentSandbox);
382-
383-
const prevDevTools = tabs[prevPos.devToolIndex];
384-
const nextDevTools = tabs[nextPos.devToolIndex];
385-
const prevTab = tabs[prevPos.devToolIndex].views[prevPos.tabPosition];
382+
getViews = () => {
383+
const sandbox = this.props.store.editor.currentSandbox;
384+
const views = getPreviewTabs(sandbox);
386385

387-
prevDevTools.views = prevDevTools.views.filter(
388-
(_, i) => i !== prevPos.tabPosition
389-
);
386+
// Do it in an immutable manner, prevents changing the original object
387+
return immer(views, draft => {
388+
const sandboxConfig = sandbox.modules.find(
389+
x => x.directoryShortid == null && x.title === 'sandbox.config.json'
390+
);
391+
let view = 'browser';
392+
if (sandboxConfig) {
393+
try {
394+
view = JSON.parse(sandboxConfig.code || '').view || 'browser';
395+
} catch (e) {
396+
/* swallow */
397+
}
398+
}
390399

391-
nextDevTools.views.splice(nextPos.tabPosition, 0, prevTab);
400+
const sandboxOptions = getSandboxOptions(location.href);
401+
if (
402+
sandboxOptions.previewWindow &&
403+
(sandboxOptions.previewWindow === 'tests' ||
404+
sandboxOptions.previewWindow === 'console')
405+
) {
406+
// Backwards compatibility for ?previewwindow=
392407

393-
const newTabs = tabs.map((t, i) => {
394-
if (i === prevPos.devToolIndex) {
395-
return prevDevTools;
396-
} else if (i === nextPos.devToolIndex) {
397-
return nextDevTools;
408+
view = sandboxOptions.previewWindow;
398409
}
399410

400-
return t;
411+
if (view !== 'browser') {
412+
// Backwards compatibility for sandbox.config.json
413+
if (view === 'console') {
414+
draft[0].views = draft[0].views.filter(
415+
t => t.id !== 'codesandbox.console'
416+
);
417+
draft[0].views.unshift({ id: 'codesandbox.console' });
418+
} else if (view === 'tests') {
419+
draft[0].views = draft[0].views.filter(
420+
t => t.id !== 'codesandbox.tests'
421+
);
422+
draft[0].views.unshift({ id: 'codesandbox.tests' });
423+
}
424+
}
401425
});
426+
};
427+
428+
moveDevToolsTab = (prevPos, nextPos) => {
429+
const { store, signals } = this.props;
430+
const tabs = this.getViews();
431+
432+
const newTabs = moveDevToolsTab(tabs, prevPos, nextPos);
402433

403434
const code = JSON.stringify({ preview: newTabs }, null, 2);
404435
const previousFile =
@@ -453,29 +484,7 @@ class EditorPreview extends React.Component<Props, State> {
453484
return false;
454485
};
455486

456-
const views = getPreviewTabs(sandbox);
457-
458-
const sandboxConfig = sandbox.modules.find(
459-
x => x.directoryShortid == null && x.title === 'sandbox.config.json'
460-
);
461-
462-
let view = 'browser';
463-
if (sandboxConfig) {
464-
try {
465-
view = JSON.parse(sandboxConfig.code || '').view || 'browser';
466-
} catch (e) {
467-
/* swallow */
468-
}
469-
}
470-
471-
if (view !== 'browser') {
472-
// Backwards compatibility for sandbox.config.json
473-
if (view === 'console') {
474-
views[0].views.unshift({ id: 'codesandbox.console' });
475-
} else if (view === 'tests') {
476-
views[0].views.unshift({ id: 'codesandbox.tests' });
477-
}
478-
}
487+
const views = this.getViews();
479488

480489
const browserConfig = {
481490
id: 'codesandbox.browser',

‎packages/app/src/app/pages/Sandbox/Editor/Content/utils.test.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { moveDevToolsTab } from './utils';
2+
3+
const BROWSER_FIXTURE = { id: 'codesandbox.browser' };
4+
const TESTS_FIXTURE = { id: 'codesandbox.tests' };
5+
const CONSOLE_FIXTURE = { id: 'codesandbox.console' };
6+
const PROBLEMS_FIXTURE = { id: 'codesandbox.problems' };
7+
8+
describe('moveDevToolsTab', () => {
9+
it('can move a tab in same devtools', () => {
10+
const tabs = [
11+
{ views: [{ ...BROWSER_FIXTURE }, { ...TESTS_FIXTURE }] },
12+
{ views: [{ ...CONSOLE_FIXTURE }, { ...PROBLEMS_FIXTURE }] },
13+
];
14+
const newTabs = moveDevToolsTab(
15+
tabs,
16+
{ devToolIndex: 0, tabPosition: 0 },
17+
{ devToolIndex: 0, tabPosition: 1 }
18+
);
19+
20+
expect(newTabs).toMatchSnapshot();
21+
});
22+
23+
it('can move a tab in same devtools and move the tab back', () => {
24+
const tabs = [
25+
{ views: [{ ...BROWSER_FIXTURE }, { ...TESTS_FIXTURE }] },
26+
{ views: [{ ...CONSOLE_FIXTURE }, { ...PROBLEMS_FIXTURE }] },
27+
];
28+
const from = { devToolIndex: 0, tabPosition: 0 };
29+
const to = { devToolIndex: 0, tabPosition: 1 };
30+
const newTabs = moveDevToolsTab(tabs, from, to);
31+
32+
expect(moveDevToolsTab(newTabs, to, from)).toMatchSnapshot();
33+
});
34+
35+
it('can move the tab to the same position', () => {
36+
const tabs = [
37+
{ views: [{ ...BROWSER_FIXTURE }, { ...TESTS_FIXTURE }] },
38+
{ views: [{ ...CONSOLE_FIXTURE }, { ...PROBLEMS_FIXTURE }] },
39+
];
40+
const from = { devToolIndex: 0, tabPosition: 0 };
41+
const to = { devToolIndex: 0, tabPosition: 0 };
42+
const newTabs = moveDevToolsTab(tabs, from, to);
43+
44+
expect(newTabs).toMatchSnapshot();
45+
});
46+
47+
it('can move a tab from one devtools to the other', () => {
48+
const tabs = [
49+
{ views: [{ ...BROWSER_FIXTURE }, { ...TESTS_FIXTURE }] },
50+
{ views: [{ ...CONSOLE_FIXTURE }, { ...PROBLEMS_FIXTURE }] },
51+
];
52+
const from = { devToolIndex: 0, tabPosition: 0 };
53+
const to = { devToolIndex: 1, tabPosition: 1 };
54+
const newTabs = moveDevToolsTab(tabs, from, to);
55+
56+
expect(newTabs).toMatchSnapshot();
57+
});
58+
59+
it('can move a tab from one devtools to the other and move it back', () => {
60+
const tabs = [
61+
{ views: [{ ...BROWSER_FIXTURE }, { ...TESTS_FIXTURE }] },
62+
{ views: [{ ...CONSOLE_FIXTURE }, { ...PROBLEMS_FIXTURE }] },
63+
];
64+
const from = { devToolIndex: 0, tabPosition: 0 };
65+
const to = { devToolIndex: 1, tabPosition: 1 };
66+
const newTabs = moveDevToolsTab(tabs, from, to);
67+
68+
expect(moveDevToolsTab(newTabs, to, from)).toMatchSnapshot();
69+
});
70+
71+
it('is immutable', () => {
72+
const tabs = [
73+
{ views: [{ ...BROWSER_FIXTURE }, { ...TESTS_FIXTURE }] },
74+
{ views: [{ ...CONSOLE_FIXTURE }, { ...PROBLEMS_FIXTURE }] },
75+
];
76+
const from = { devToolIndex: 0, tabPosition: 0 };
77+
const to = { devToolIndex: 1, tabPosition: 1 };
78+
const newTabs = moveDevToolsTab(tabs, from, to);
79+
80+
expect(newTabs).not.toBe(tabs);
81+
});
82+
});

0 commit comments

Comments
 (0)