Skip to content

Commit 12ce8b9

Browse files
authored
Fix resize repaint gaps and initial view sizing (#1039)
* Fix resize repaint gaps and initial view sizing The post-resize invalidate block repainted the title bar, content and env-select popup but skipped the progress view, even though it gets re-bounded on every resize. On the platforms that need the explicit invalidate that left the progress view stale until the next paint, so it now gets invalidated alongside the others. The title bar and welcome view were laid out from the compile-time DEFAULT_WIN_* constants at load time. A session restored at a larger or maximized size then rendered mis-sized for a frame until the first resize corrected it. Both now read the live size from getContentBounds, mirroring what _resizeViews already does. The update check parsed the release feed body with no status check, so a 5xx or an HTML error page went straight into yaml.load and could be mistaken for metadata. It now fails fast on a non-ok response and guards that the parsed version is a valid semver before comparing, which routes a bad response cleanly to the existing error dialog on the 'always' path instead of comparing against garbage. Also leave a note by the dialog toolkit script so nobody extends the CommonJS interop shim to it: the toolkit is a UMD bundle that branches on typeof exports and faking exports there takes down every jp-* component. I wrote and verified these with Claude Code: tsc clean, the full unit suite green, and a focused test that a non-ok update response never reaches the body parse (fails before the status check, passes after). * test: sort the vitest named imports to satisfy eslint The installer CI runs eslint:check before building and the sort-imports rule rejected the unsorted import in the new checkForUpdates test, failing the Linux job (and cancelling the others). Order the members alphabetically. * fix: share title bar bounds between load and resize The initial bounds in load() spanned the full content width with no inset, while _resizeViews() insets by 1px on non-macOS, so the title bar gapped by a pixel on the first paint until the first resize corrected it. Compute the bounds once in _titleBarBounds() and use it from both so the first paint matches every later resize. Note: AI-assisted (Claude Code). Manually verified: tsc and eslint clean, added a unit test for the macOS and non-macOS rects and confirmed it fails when the inset is removed.
1 parent 9023161 commit 12ce8b9

6 files changed

Lines changed: 111 additions & 13 deletions

File tree

src/main/app.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,9 +1257,15 @@ export class JupyterApplication implements IApplication, IDisposable {
12571257
)
12581258
.then(async response => {
12591259
try {
1260+
if (!response.ok) {
1261+
throw new Error(`Update check failed: ${response.status}`);
1262+
}
12601263
const data = await response.text();
12611264
const latestReleaseData = yaml.load(data);
12621265
const latestVersion = (latestReleaseData as any).version;
1266+
if (!latestVersion || !semver.valid(latestVersion)) {
1267+
throw new Error('No valid version in update metadata');
1268+
}
12631269
const currentVersion = app.getVersion();
12641270
const newVersionAvailable =
12651271
semver.compare(currentVersion, latestVersion) === -1;

src/main/dialog/themedwindow.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ export class ThemedWindow {
106106
overflow-y: auto;
107107
}
108108
</style>
109+
<!-- do not prepend the cjsInteropShim here: the toolkit is a UMD bundle that branches on typeof exports, so faking exports breaks every jp-* component -->
109110
<script type="module">${toolkitJsSrc}</script>
110111
<script type="module">${cjsInteropShim}${titlebarJsSrc}</script>
111112
<script>

src/main/sessionwindow/sessionwindow.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -170,12 +170,7 @@ export class SessionWindow implements IDisposable {
170170
load() {
171171
const titleBarView = new TitleBarView({ isDarkTheme: this._isDarkTheme });
172172
this._window.contentView.addChildView(titleBarView.view);
173-
titleBarView.view.setBounds({
174-
x: 0,
175-
y: 0,
176-
width: DEFAULT_WIN_WIDTH,
177-
height: titleBarHeight
178-
});
173+
titleBarView.view.setBounds(this._titleBarBounds());
179174

180175
this._window.on('focus', () => {
181176
titleBarView.activate();
@@ -324,11 +319,15 @@ export class SessionWindow implements IDisposable {
324319
isDarkTheme: this._isDarkTheme
325320
});
326321
this._window.contentView.addChildView(welcomeView.view);
322+
const {
323+
width: contentWidth,
324+
height: contentHeight
325+
} = this._window.getContentBounds();
327326
welcomeView.view.setBounds({
328327
x: 0,
329328
y: titleBarHeight,
330-
width: DEFAULT_WIN_WIDTH,
331-
height: DEFAULT_WIN_HEIGHT
329+
width: contentWidth,
330+
height: contentHeight - titleBarHeight
332331
});
333332

334333
welcomeView.load();
@@ -1225,16 +1224,23 @@ export class SessionWindow implements IDisposable {
12251224
}, 300);
12261225
}
12271226

1228-
private _resizeViews() {
1229-
const { width, height } = this._window.getContentBounds();
1230-
// add padding to allow resizing around title bar
1227+
// Title bar bounds, shared by load() and _resizeViews() so the first paint
1228+
// matches every later resize. Non-macOS insets by 1px so the frame stays
1229+
// grabbable for resizing around the title bar.
1230+
private _titleBarBounds(): Electron.Rectangle {
1231+
const { width } = this._window.getContentBounds();
12311232
const padding = process.platform === 'darwin' ? 0 : 1;
1232-
this._titleBarView.view.setBounds({
1233+
return {
12331234
x: padding,
12341235
y: padding,
12351236
width: width - 2 * padding,
12361237
height: titleBarHeight - padding
1237-
});
1238+
};
1239+
}
1240+
1241+
private _resizeViews() {
1242+
const { width, height } = this._window.getContentBounds();
1243+
this._titleBarView.view.setBounds(this._titleBarBounds());
12381244
const contentRect: Electron.Rectangle = {
12391245
x: 0,
12401246
y: titleBarHeight,
@@ -1260,6 +1266,7 @@ export class SessionWindow implements IDisposable {
12601266
};
12611267
invalidate(this._titleBarView?.view?.webContents);
12621268
invalidate(this.contentView?.webContents);
1269+
invalidate(this._progressView?.view?.view?.webContents);
12631270
invalidate(this._envSelectPopup?.view?.view?.webContents);
12641271
}, 200);
12651272
}

test/setup/electron-stub.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ export const BrowserWindow = vi.fn().mockImplementation(() => ({
5555

5656
export const shell = { openExternal: vi.fn(), openPath: vi.fn() };
5757

58+
export const net = { fetch: vi.fn() };
59+
5860
export const nativeTheme = { shouldUseDarkColors: false };
5961

6062
export const screen = {
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
import { net } from '../setup/electron-stub';
3+
import { JupyterApplication } from '../../src/main/app';
4+
5+
// checkForUpdates is a method on the large JupyterApplication class whose
6+
// constructor wires up the whole main process, so we exercise the method on a
7+
// prototype-only instance and stub the two collaborators it reaches: the
8+
// electron `net.fetch` boundary and the private _showUpdateDialog sink.
9+
function makeApp(): {
10+
app: JupyterApplication;
11+
showUpdateDialog: ReturnType<typeof vi.fn>;
12+
} {
13+
const app = Object.create(JupyterApplication.prototype) as JupyterApplication;
14+
const showUpdateDialog = vi.fn();
15+
((app as unknown) as {
16+
_showUpdateDialog: unknown;
17+
})._showUpdateDialog = showUpdateDialog;
18+
return { app, showUpdateDialog };
19+
}
20+
21+
describe('JupyterApplication.checkForUpdates', () => {
22+
beforeEach(() => {
23+
(net.fetch as ReturnType<typeof vi.fn>).mockReset();
24+
});
25+
26+
it('does not read the body when the release feed responds non-ok', async () => {
27+
const { app, showUpdateDialog } = makeApp();
28+
const text = vi.fn(() => Promise.resolve('<html>not yaml</html>'));
29+
(net.fetch as ReturnType<typeof vi.fn>).mockResolvedValue({
30+
ok: false,
31+
status: 503,
32+
text
33+
});
34+
35+
app.checkForUpdates('always');
36+
37+
await vi.waitFor(() => expect(showUpdateDialog).toHaveBeenCalled());
38+
expect(text).not.toHaveBeenCalled();
39+
expect(showUpdateDialog).toHaveBeenCalledWith('error');
40+
expect(showUpdateDialog).not.toHaveBeenCalledWith('updates-available');
41+
});
42+
});
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { afterEach, describe, expect, it } from 'vitest';
2+
import { SessionWindow } from '../../src/main/sessionwindow/sessionwindow';
3+
4+
// _titleBarBounds is the single source of truth shared by load() and
5+
// _resizeViews(); if the two ever diverge again the first paint gaps by a pixel
6+
// on non-macOS. Drive it directly without the heavy constructor.
7+
const realPlatform = process.platform;
8+
function setPlatform(p: string): void {
9+
Object.defineProperty(process, 'platform', { value: p, configurable: true });
10+
}
11+
12+
function windowWith(width: number): any {
13+
const win = Object.create(SessionWindow.prototype);
14+
win._window = { getContentBounds: () => ({ width, height: 600 }) };
15+
return win;
16+
}
17+
18+
describe('SessionWindow._titleBarBounds', () => {
19+
afterEach(() => setPlatform(realPlatform));
20+
21+
it('spans the full content width with no inset on macOS', () => {
22+
setPlatform('darwin');
23+
expect(windowWith(800)._titleBarBounds()).toEqual({
24+
x: 0,
25+
y: 0,
26+
width: 800,
27+
height: 29
28+
});
29+
});
30+
31+
it('insets by one pixel on non-macOS so the frame stays grabbable', () => {
32+
setPlatform('linux');
33+
expect(windowWith(800)._titleBarBounds()).toEqual({
34+
x: 1,
35+
y: 1,
36+
width: 798,
37+
height: 28
38+
});
39+
});
40+
});

0 commit comments

Comments
 (0)