ReviewOS
Reload

also looking at this

craft-native/craft

feat(window): --frame-autosave, so windows come back where they were

#56
Merged glennmichael123 wants to merge feat/window-frame-autosave into main
8 files +181 -0
packages/typescript/src/__tests__/frame-autosave.test.tsadded+56-0
@@ -0,0 +1,56 @@
1/**
2 * `frameAutosave` — windows that remember where they were (craft-native/craft#52).
3 *
4 * There was no `setFrameAutosaveName:` anywhere in the tree, so every Craft
5 * window forgot its geometry on every launch. An app that wanted the standard
6 * behaviour had to wire `onResize`/`onMove` to its own storage and restore on
7 * next start, reimplementing badly what AppKit does in one call.
8 */
9
10import type { AppConfig } from '../types'
11import { describe, expect, it } from 'bun:test'
12
13async function argsFor(config: AppConfig): Promise<string[]> {
14 const { CraftApp } = await import('../index')
15 const app = new (CraftApp as unknown as { new (c: AppConfig): { buildArgs: () => string[] } })(config)
16 return (app as unknown as { buildArgs: () => string[] }).buildArgs()
17}
18
19function valueOf(args: string[], flag: string): string | undefined {
20 const index = args.indexOf(flag)
21 return index === -1 ? undefined : args[index + 1]
22}
23
24describe('frameAutosave', () => {
25 it('passes the autosave name through to the binary', async () => {
26 const args = await argsFor({ window: { frameAutosave: 'main' } })
27 expect(valueOf(args, '--frame-autosave')).toBe('main')
28 })
29
30 it('is absent when not configured, so nothing is remembered', async () => {
31 // The default has to stay "forgets its geometry": silently starting to
32 // persist frames would move existing apps' windows on upgrade.
33 const args = await argsFor({ window: { width: 900, height: 700 } })
34 expect(args).not.toContain('--frame-autosave')
35 })
36
37 it('coexists with explicit geometry rather than replacing it', async () => {
38 // The two are not alternatives. `width`/`height`/`x`/`y` are what the
39 // window opens at the first time, before there is any saved frame to
40 // restore — so both must reach the binary.
41 const args = await argsFor({
42 window: { frameAutosave: 'main', width: 900, height: 700, x: 40, y: 60 },
43 })
44 expect(valueOf(args, '--frame-autosave')).toBe('main')
45 expect(valueOf(args, '--width')).toBe('900')
46 expect(valueOf(args, '--height')).toBe('700')
47 expect(valueOf(args, '--x')).toBe('40')
48 expect(valueOf(args, '--y')).toBe('60')
49 })
50
51 it('survives a name containing spaces as a single argument', async () => {
52 const args = await argsFor({ window: { frameAutosave: 'Main Window' } })
53 expect(valueOf(args, '--frame-autosave')).toBe('Main Window')
54 expect(args.filter(a => a === 'Main Window')).toHaveLength(1)
55 })
56})
packages/typescript/src/index.tsmodified+4-0
@@ -431,6 +431,10 @@ export class CraftApp {
431431 }
432432 if (window?.icon)
433433 args.push('--icon', window.icon)
434 // After the geometry flags, and independent of them: those are what the
435 // window opens at until there is a saved frame, which then wins.
436 if (window?.frameAutosave)
437 args.push('--frame-autosave', window.frameAutosave)
434438
435439 if (window?.nativeSidebar) {
436440 args.push('--native-sidebar')
packages/typescript/src/types.tsmodified+16-0
@@ -57,6 +57,22 @@ export interface WindowOptions {
5757 */
5858 title?: string
5959
60 /**
61 * Remember this window's size and position across launches, under this name
62 * (macOS).
63 *
64 * `width`/`height`/`x`/`y` become **first-launch defaults**: they are what
65 * the window opens at until there is a saved frame to restore, and the saved
66 * frame wins from then on. Without this the window forgets its geometry
67 * every launch, and an app that wants the standard behaviour has to wire
68 * `onResize`/`onMove` to its own storage and reimplement what AppKit does in
69 * one call.
70 *
71 * The name is the key AppKit stores under, so it must be stable across
72 * launches and distinct per window.
73 */
74 frameAutosave?: string
75
6076 /**
6177 * Window width in pixels
6278 * @default 800
packages/zig/src/cli.zigmodified+28-0
@@ -46,6 +46,9 @@ pub const WindowOptions = struct {
4646 // — so every app launched through the shared `craft` binary called itself
4747 // "craft". macOS only.
4848 app_name: ?[]const u8 = null,
49 // Name AppKit remembers this window's size and position under, so the app
50 // opens where the user last left it. Null means it forgets every launch.
51 frame_autosave: ?[]const u8 = null,
4952};
5053
5154pub const CliError = error{
@@ -73,6 +76,7 @@ pub fn freeOptionStrings(allocator: std.mem.Allocator, options: *WindowOptions)
7376 if (options.sidebar_config) |s| allocator.free(s);
7477 if (options.icon) |s| allocator.free(s);
7578 if (options.app_name) |s| allocator.free(s);
79 if (options.frame_autosave) |s| allocator.free(s);
7680 if (options.eval_source) |s| allocator.free(s);
7781 if (options.eval_file) |s| allocator.free(s);
7882 options.* = WindowOptions{};
@@ -141,6 +145,7 @@ const BundleConfig = struct {
141145 url: ?[]const u8 = null,
142146 title: ?[]const u8 = null,
143147 appName: ?[]const u8 = null,
148 frameAutosave: ?[]const u8 = null,
144149 width: ?u32 = null,
145150 height: ?u32 = null,
146151 icon: ?[]const u8 = null,
@@ -228,6 +233,7 @@ fn loadBundleConfig(allocator: std.mem.Allocator) ?WindowOptions {
228233 if (cfg.url) |v| options.url = allocator.dupe(u8, v) catch null;
229234 if (cfg.title) |v| options.title = allocator.dupe(u8, v) catch options.title;
230235 if (cfg.appName) |v| options.app_name = allocator.dupe(u8, v) catch null;
236 if (cfg.frameAutosave) |v| options.frame_autosave = allocator.dupe(u8, v) catch null;
231237 if (cfg.icon) |v| options.icon = allocator.dupe(u8, v) catch null;
232238 applyScalarConfig(cfg, &options);
233239
@@ -325,6 +331,19 @@ test "a manifest can name the app without naming the window" {
325331 try std.testing.expectEqualStrings("Hush", parsed.value.appName.?);
326332}
327333
334test "a manifest can ask for its window frame to be remembered" {
335 const source =
336 \\{"title":"Hush","frameAutosave":"main"}
337 ;
338 const parsed = try std.json.parseFromSlice(BundleConfig, std.testing.allocator, source, .{
339 .ignore_unknown_fields = true,
340 .allocate = .alloc_always,
341 });
342 defer parsed.deinit();
343
344 try std.testing.expectEqualStrings("main", parsed.value.frameAutosave.?);
345}
346
328347test "unknown manifest keys are ignored rather than failing the launch" {
329348 const source =
330349 \\{"title":"Hush","somethingNewer":true}
@@ -485,6 +504,10 @@ pub fn parseArgs(allocator: std.mem.Allocator, args: []const [:0]const u8) !Wind
485504 i += 1;
486505 if (i >= args.len) return CliError.MissingValue;
487506 options.app_name = try allocator.dupe(u8, args[i]);
507 } else if (std.mem.eql(u8, arg, "--frame-autosave")) {
508 i += 1;
509 if (i >= args.len) return CliError.MissingValue;
510 options.frame_autosave = try allocator.dupe(u8, args[i]);
488511 } else if (!std.mem.startsWith(u8, arg, "--")) {
489512 // Treat as positional URL argument
490513 if (options.url == null) {
@@ -551,6 +574,10 @@ fn printHelp() void {
551574 \\ --icon <PATH> Path to dock icon image (PNG/JPG/ICNS)
552575 \\ --app-name <NAME> Name in the menu bar and in About/Hide/Quit
553576 \\ (macOS; defaults to the executable name)
577 \\ --frame-autosave <NAME>
578 \\ Remember window size and position across
579 \\ launches under NAME (macOS). --width/--height/
580 \\ --x/--y become first-launch defaults.
554581 \\
555582 \\Debugging:
556583 \\ --debug Enable debug output
@@ -575,6 +602,7 @@ fn printHelp() void {
575602 \\ craft http://localhost:3000 --system-tray --light
576603 \\ craft http://localhost:3000 --native-sidebar --sidebar-width 250
577604 \\ craft http://localhost:3000 --app-name "My App"
605 \\ craft http://localhost:3000 --frame-autosave main
578606 \\
579607 \\For more information, visit: https://github.com/home-lang/craft
580608 \\
packages/zig/src/macos.zigmodified+33-0
@@ -92,6 +92,10 @@ pub const WindowStyle = struct {
9292 web_sidebar_width: u32 = 286,
9393 web_sidebar_material_opacity: f64 = 0.78,
9494 benchmark: bool = false, // Benchmark mode: skip bridge setup for fastest window creation
95 /// Name under which AppKit remembers this window's frame across launches.
96 /// Null leaves the window forgetting its geometry every time, which is
97 /// what craft did until now.
98 frame_autosave: ?[]const u8 = null,
9599};
96100
97101// Helper functions for Objective-C runtime
@@ -525,6 +529,29 @@ fn getBorderlessWindowClass() objc.Class {
525529 return BorderlessWindowClass;
526530}
527531
532/// Remember this window's size and position across launches, under `name`.
533///
534/// Two calls, in this order, and the order is the whole design:
535///
536/// 1. `setFrameUsingName:` restores a saved frame if there is one. It
537/// returns NO and leaves the window alone if there is not — which is
538/// what makes `--width/--height/--x/--y` behave as first-launch defaults
539/// rather than as an override that would defeat the point.
540/// 2. `setFrameAutosaveName:` starts saving on every subsequent change.
541///
542/// Measured rather than assumed: `setFrameAutosaveName:` does *not* write the
543/// current frame when it is set, so enabling it after the restore cannot
544/// clobber what was restored.
545///
546/// Called after the centring above, so a remembered frame wins over being
547/// centred — an app the user has placed should come back where they put it.
548fn applyFrameAutosave(window: objc.id, name: []const u8) void {
549 if (name.len == 0) return;
550 const key = createNSString(name);
551 _ = msgSend1(window, "setFrameUsingName:", key);
552 _ = msgSend1(window, "setFrameAutosaveName:", key);
553}
554
528555pub fn createWindow(title: []const u8, width: u32, height: u32, html: []const u8) !objc.id {
529556 return createWindowWithStyle(title, width, height, html, null, .{});
530557}
@@ -910,6 +937,8 @@ pub fn createWindowWithStyle(title: []const u8, width: u32, height: u32, html: ?
910937 msgSendVoid0(window, "center");
911938 }
912939
940 if (style.frame_autosave) |name| applyFrameAutosave(window, name);
941
913942 // Enter fullscreen if requested
914943 if (style.fullscreen) {
915944 msgSendVoid0(window, "toggleFullScreen:");
@@ -2136,6 +2165,8 @@ pub fn createWindowWithSidebar(
21362165 msgSendVoid0(window, "center");
21372166 }
21382167
2168 if (style.frame_autosave) |name| applyFrameAutosave(window, name);
2169
21392170 // Store references
21402171 if (webview) |wv| {
21412172 setGlobalWebView(wv);
@@ -3327,6 +3358,8 @@ pub fn createWindowWithSidebarURL(
33273358 msgSendVoid0(window, "center");
33283359 }
33293360
3361 if (style.frame_autosave) |name| applyFrameAutosave(window, name);
3362
33303363 // Store references
33313364 if (webview) |wv| {
33323365 setGlobalWebView(wv);
packages/zig/src/main.zigmodified+4-0
@@ -104,6 +104,10 @@ pub const WindowStyle = if (builtin.os.tag == .macos) macos.WindowStyle else str
104104 web_sidebar_width: u32 = 286,
105105 web_sidebar_material_opacity: f64 = 0.78,
106106 benchmark: bool = false,
107 /// Accepted so `minimal.zig` can pass one `WindowStyle` on every platform,
108 /// and ignored: remembering a window frame is `setFrameAutosaveName:`, an
109 /// AppKit facility with no counterpart wired up on GTK or Win32 yet.
110 frame_autosave: ?[]const u8 = null,
107111};
108112
109113pub const Window = struct {
packages/zig/src/minimal.zigmodified+6-0
@@ -112,6 +112,7 @@ pub fn main(init: std.process.Init) !void {
112112 .dev_tools = effective_dev_tools,
113113 .native_sidebar = true,
114114 .benchmark = options.benchmark,
115 .frame_autosave = options.frame_autosave,
115116 },
116117 );
117118 } else if (options.native_sidebar and options.html != null) {
@@ -149,6 +150,7 @@ pub fn main(init: std.process.Init) !void {
149150 .dev_tools = effective_dev_tools,
150151 .native_sidebar = true,
151152 .benchmark = options.benchmark,
153 .frame_autosave = options.frame_autosave,
152154 },
153155 );
154156 } else if (options.url) |url| {
@@ -188,6 +190,7 @@ pub fn main(init: std.process.Init) !void {
188190 .system_tray = options.system_tray,
189191 .dev_tools = effective_dev_tools,
190192 .benchmark = options.benchmark,
193 .frame_autosave = options.frame_autosave,
191194 .web_sidebar_material = options.web_sidebar_material,
192195 .web_sidebar_width = options.web_sidebar_width,
193196 .web_sidebar_material_opacity = options.web_sidebar_material_opacity,
@@ -221,6 +224,7 @@ pub fn main(init: std.process.Init) !void {
221224 .system_tray = options.system_tray,
222225 .dev_tools = effective_dev_tools,
223226 .benchmark = options.benchmark,
227 .frame_autosave = options.frame_autosave,
224228 .web_sidebar_material = options.web_sidebar_material,
225229 .web_sidebar_width = options.web_sidebar_width,
226230 .web_sidebar_material_opacity = options.web_sidebar_material_opacity,
@@ -412,6 +416,7 @@ fn runWithSystemTray(allocator: std.mem.Allocator, options: cli.WindowOptions) !
412416 .titlebar_hidden = options.titlebar_hidden,
413417 .system_tray = options.system_tray,
414418 .dev_tools = options.dev_tools,
419 .frame_autosave = options.frame_autosave,
415420 },
416421 );
417422 } else if (options.html) |html| {
@@ -434,6 +439,7 @@ fn runWithSystemTray(allocator: std.mem.Allocator, options: cli.WindowOptions) !
434439 .titlebar_hidden = options.titlebar_hidden,
435440 .system_tray = options.system_tray,
436441 .dev_tools = options.dev_tools,
442 .frame_autosave = options.frame_autosave,
437443 },
438444 );
439445 }
packages/zig/test/cli_test.zigmodified+34-0
@@ -278,3 +278,37 @@ test "parseArgs - a name with spaces survives as one argument" {
278278 defer freeOptions(&options);
279279 try testing.expectEqualStrings("My Great App", options.app_name.?);
280280}
281
282test "parseArgs - --frame-autosave is read and owned" {
283 var options = try parse(&.{ "craft", "--frame-autosave", "main" });
284 defer freeOptions(&options);
285 try testing.expectEqualStrings("main", options.frame_autosave.?);
286}
287
288test "parseArgs - --frame-autosave defaults to absent" {
289 // Absent means the window forgets its geometry, which is what craft did
290 // before this flag existed and must keep doing for apps that never ask.
291 var options = try parse(&.{ "craft", "http://localhost:3000" });
292 defer freeOptions(&options);
293 try testing.expect(options.frame_autosave == null);
294}
295
296test "parseArgs - --frame-autosave leaves the explicit geometry alone" {
297 // The two are not alternatives: the geometry flags are what the window
298 // opens at the first time, before anything has been saved to restore.
299 var options = try parse(&.{
300 "craft", "--frame-autosave", "main", "--width", "900",
301 "--height", "700", "--x", "40", "--y",
302 "60",
303 });
304 defer freeOptions(&options);
305 try testing.expectEqualStrings("main", options.frame_autosave.?);
306 try testing.expectEqual(@as(u32, 900), options.width);
307 try testing.expectEqual(@as(u32, 700), options.height);
308 try testing.expectEqual(@as(i32, 40), options.x.?);
309 try testing.expectEqual(@as(i32, 60), options.y.?);
310}
311
312test "parseArgs - --frame-autosave without a value is an error, not a silent skip" {
313 try testing.expectError(cli.CliError.MissingValue, parse(&.{ "craft", "--frame-autosave" }));
314}