Repository navigation
cmux-next: collages: arrange several pictures as one background - #21789
teamleaderleo wants to merge 58 commits into
Conversation
…t chat Every wide painting framed the same fixed point (0.5, 0.45), so with the sidebar and a docked agent chat open the subject often sat under them, and wide or short windows cut faces off. Each work now names its subject box in art/manifest.json, and the art zooms, slides or shrinks (over a blurred fill) so that box lies in the part of the window the columns leave visible. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…out, not the App CmuxNextApp may not grow (check-app-size). The sidebar and each workspace's layout root now tell the window's backdrop where they cover the art (WindowMaterialView.setCover), and the App is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Settings > Background gets Add from a Link (https only, judged by its bytes, 50 MB image / 500 MB video caps), a remove button on each of your images (not the one in use), what they take on disk, and Show in Finder. A picked, dropped or linked file must decode or play before it is copied in. Dropping a file on Settings no longer needs experimental appearance (the page already said to). The ops live in BackdropImagesPageProvider (CmuxNextPages); CmuxNextApp shrinks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r edge correctly Review fixes for the subject crop: - keepsSubject is measured on the final placement, never assumed. - A shrunk or slid image keeps the subject in the span first (the cover clamp overrode it), and the shrink fit counts the subject's height too. - No vertical fallback that cannot fail; portrait subjects are the head and upper body (top 70%%), which fit a 21:9 window. - The catalog test asserts the subject frame lies in the visible span. - Only a column that comes or goes springs the art; resizes and live window resizes move it at once. Fill filters are made per view. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eir name in their folder Review fixes for Add from a Link: - The link's name is sanitized (no folders, colons or leading dots), and the renamed file must stay in the download's own folder (an encoded ../ climbed out). - A delegate-driven data task replaces download(from:delegate:), whose task delegate never saw progress: the declared length and the running count are checked against the cap for the declared type, so a huge body never lands. - Redirects off https are refused; timeouts are a minute idle, ten in all. - The temporary folder is removed on every failure. - Remove refuses the background in use natively, not just on the page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…leo/next-place-background
Settings > Background > Adjust Placement… fades the window's content and puts an overlay over it: drag moves the picture, scroll or pinch zooms (1x-4x), Reset goes back to the automatic subject framing, Done or Escape ends. The placement (the point at the middle of the visible span, and the zoom) is kept per background in background-placement.json, and every window follows it. It starts from the framing on screen, so the first drag does not jump. CmuxNextApp is unchanged: the op rides BackdropImagesPageProvider. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…re they start Re-review fixes: LinkDownload keeps its state in a Mutex and is plainly Sendable (no new unchecked conformance for the crash ratchet). A cancel that comes before the task exists marks the download, and the task is cancelled as soon as it is made, instead of invalidating the session first (making a task on an invalidated session raises). A body that starts like a picture gets the picture cap whatever type the server declared. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eo/next-place-background
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ace window, saved in order Review fixes for Adjust Placement: - Compile: the store notification's object is compared by identity (the batch Swift run stopped on 'sending store risks causing data races'). - Saves run on one serial queue, in order; an unreadable file is kept beside. - The overlay opens in the workspace window (WindowManager.active), never the Settings window; one at a time app-wide; it ends when its window shows another background, and fades views added while it is open. - Keyboard: arrows nudge (Shift: further), + and - zoom, 0 resets. - A drag past an edge has no dead travel; windows only re-place for their own background's change; removing a picture drops its placement; the page's 'no picture' note clears when the background changes. - init(coder:) returns nil instead of fatalError (crash ratchet). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ace-background # Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextDesign/BackdropArtPlacement.swift # Packages/macOS/CmuxNext/Sources/CmuxNextDesign/WindowMaterialView.swift # Packages/macOS/CmuxNext/Tests/CmuxNextDesignTests/BackdropArtPlacementTests.swift
appearance.background takes collage:<id>, a collage saved as Backgrounds/Collages/<id>.json and drawn at 3200x2000 to <id>.png, which the window shows like any picture. Settings > Background > New Collage… opens an editor: add pictures from the grid, drag to move, drag the corner handle to resize (edges snap to halves and thirds), arrows nudge, layer order, rounded corners, a fill color and a name, starter layouts (side by side, three columns, mosaic). Saving draws it natively and makes it the background; your collages can be edited (a new id each save) or removed (never the one in use). The page ops ride BackdropImagesPageProvider; CmuxNextApp is unchanged. Still pictures only for now; video layers are a follow-up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Unclear: CI failed on
Failures:
Matched log linesNot re-run automatically: Written by red: cmux-next CmuxNextApp size cap; NEW: green on the base c740963; do not merge until green; see lane-rules
Agents: |
…dow comes forward Re-review fix: the center's clamp had its bounds swapped, so for art that covers the window every drag or arrow pinned it to an edge. The bounds are now [middle / size, (middle - view + size) / size], and the test checks the exact travel of a drag and the stop at the edge. Placing brings the target window forward again, so from a separate Settings window keys reach it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Bead |
|
Found 33 test failures on Blacksmith runners: Failures
...and 23 more test failures. View all on Blacksmith: run 39395662303, run 39395662251
|
The fleet compile found JSONValue (CmuxNextSettings) and revealInBackground (CmuxNextCompat) used without their imports. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eo/next-place-background
… switches Re-review fix: saving no longer deletes the collage it replaces. The page saves the edit under a new id, switches appearance.background to it, and only then asks to delete the old one, which the host refuses while the app or any workspace background (workspace-backgrounds.json) still shows it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sion, cache names Re-review and fleet-compile fixes: saving never deletes (the page removes an edited collage's old files after the switch), the record is written before the drawing lands through a temporary file, a drawing without its record is not listed, names are read once per id, and list() no longer trips the type checker's time limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…leo/next-collages
A public initializer's default argument may only use publicly imported API. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eo/next-place-background
…leo/next-collages
…moved Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
Re-review fix: when workspace-backgrounds.json exists but does not read, a collage or image is taken to be shown, so removing it is refused rather than risking a blank workspace. The test covers a workspace showing a collage and an image, a malformed file, and a file that names neither. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
| window.makeKeyAndOrderFront(nil) | ||
| window.makeFirstResponder(overlay) | ||
| active = overlay |
There was a problem hiding this comment.
Placement loses keyboard focus
BackdropAdjustOverlay.begin(in:) gives the overlay keyboard focus without telling the window's focus owner that an overlay opened. In an app window, ShellWindow.makeFirstResponder immediately reports this view as windowOrNone. The focus owner then puts focus back on the selected page or terminal.
The overlay stays visible, but arrow keys, zoom keys and Escape reach the content underneath instead. Register and close this overlay through the window's focus owner, as the palette does.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| return name | ||
| } | ||
|
|
||
| private static let names = Mutex<[String: String]>([:]) |
There was a problem hiding this comment.
Name cache breaks locking rule
BackdropCollages adds a shared name cache guarded by Mutex. The repository's blocking-runtime rule forbids new manual locks around shared state unless a documented low-level boundary prevents an actor from owning it. This cache has no such boundary or explanation.
Keep cached names on an owned, isolated catalog or store instead of adding another lock-backed global cache.
Rule Used: Flag new blocking or timing-based synchronization in production Swift: semaphores, DispatchGroup.wait, sleeps, Task.sleep, asyncAfter, timers or polling for synchronization, DispatchQueue.main.sync, or manual locks where actor isolation or a real sig... (source)
| Self.writes.async { | ||
| try? FileManager.default.createDirectory(at: file.deletingLastPathComponent(), withIntermediateDirectories: true) | ||
| try? data.write(to: file, options: .atomic) | ||
| } |
There was a problem hiding this comment.
BackdropPlacementStore adds a custom background queue for ordinary file writes. The repository's Swift concurrency rule requires this work to use Swift concurrency instead. Move ordered writes into an owned actor or a stored task chain so the store can await their completion.
Rule Used: Flag new legacy async patterns in cmux-owned Swift where Swift concurrency is the correct shape: DispatchQueue.global for ordinary async work, new Combine app state, completion-handler APIs fully under cmux control, or fire-and-forget Tasks with mean... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| /// `subviews`, so each window update (after every event it handles) looks for new ones. | ||
| private var updateObserver: NSObjectProtocol? | ||
| /// The one overlay placing now, in any window. | ||
| private static weak var active: BackdropAdjustOverlay? |
There was a problem hiding this comment.
Windows share hidden overlay state
BackdropAdjustOverlay.active keeps app-wide mutable state on the view type. Opening an overlay in one window closes another through this hidden reference. This violates the repository's requirement that runtime state belong to a constructable, injected owner. Move the active overlay to a coordinator owned by the app's window services.
Rule Used: Flag new ambient global state in production Swift: a top-level (file-scope) func used as API, a top-level mutable var or a stub class/struct holding a global flag/once-token, a caseless enum/empty struct used purely as a static func/static let namesp... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| {index === chosen && ( | ||
| <button | ||
| type="button" | ||
| className="collage-handle" |
There was a problem hiding this comment.
Pictures cover the resize handle
A picture in front can cover the selected picture's resize handle. The handle is rendered beside its layer inside the back-to-front loop, and collage-handle has no higher stacking order. Clicking the covered corner selects the front picture instead, so users must move or reorder pictures before resizing. Render the selected handle above all picture layers.
|
|
| <button | ||
| type="button" | ||
| className="collage-handle" | ||
| aria-label={t("settingsWindow.collage.resize")} | ||
| style={{ | ||
| left: `calc(${(item.x + item.width) * 100}% - 12px)`, | ||
| top: `calc(${(item.y + item.height) * 100}% - 12px)`, | ||
| }} | ||
| onPointerDown={grab("resize")} | ||
| onPointerMove={pointer} | ||
| onPointerUp={release} | ||
| onPointerCancel={release} |
There was a problem hiding this comment.
Keyboard resizing does nothing
collage-handle is a focusable button, but it only responds to pointer events. Enter, Space and arrow keys do nothing, and the layer's keyboard handler only moves or removes pictures. Keyboard-only users cannot resize a picture. Add keyboard resizing to the handle or provide width and height controls.
| const save = async () => { | ||
| setBusy(true); | ||
| setRefusal(null); | ||
| const result = await store.saveCollage(collage, replacing); | ||
| setBusy(false); | ||
| if ("id" in result) onClose(); |
There was a problem hiding this comment.
The editor stays editable while saveCollage() runs. If a user changes the name, layout or pictures during a slow save, those changes update the preview, but Save still writes the earlier collage value and then closes the editor. Disable editing while saving, or keep the editor open when newer changes exist.
| const switched = await this.set("appearance.background", `collage:${reply.value.id}`); | ||
| if (switched.ok && replacing) await this.request("cmux.settings.backdrop.image.collage.delete", { id: replacing }); |
There was a problem hiding this comment.
saveCollage() switches to a new id without copying the old placement, then deletes the old collage and its placement. Anyone who positioned or zoomed the background loses that choice, even when changing only its name. Copy the placement to the new id before switching backgrounds.
- Packages/macOS/CmuxNext/ci-target-graph.json Autofix-Run: https://github.com/manaflow-ai/cmux/actions/runs/39391164578
| <button | ||
| type="button" | ||
| className="icon-button backdrop-remove backdrop-edit" | ||
| aria-label={t("settingsWindow.collage.edit", choice.title)} | ||
| onClick={() => { | ||
| const id = choice.id.slice("collage:".length); | ||
| void store.collage(id).then((start) => start && setEditing({ start, replacing: id })); | ||
| }} |
There was a problem hiding this comment.
This comment has been minimized.
This comment has been minimized.
|
owner: 01a15c08-af2e-7471-8e77-d62753886e4f |
|
Owned by nx-settings on big-red |
|
Rebuilt as #25433. The fresh current-base slice starts from |
Phase 7 of the backgrounds plan: collages, approved by Leo through the coordinator.
Stacked on #21743, which carries #21731's Settings page provider. Review only 8afeb67.
What changes
appearance.backgroundtakescollage:<id>(BackdropSelection.collage).BackdropSelectionSetting.acceptsand the page'svalidate.tsboth take it. A schema sample and a refused sample (collage:../evil) are added, and the base-autofix regenerates the export.Backgrounds/Collages/<id>.jsonholds the record (BackdropCollage: name, fill and layers back to front, each with a source, frame, opacity and corner radius).<id>.pngis the drawing, which the window shows like any picture.BackdropCollages.renderdraws the fill, then each layer.cmux.settings.backdrop.image.collage.save | get | delete, onBackdropImagesPageProvider. The page setsappearance.backgrounditself through the normal settings write. CmuxNextApp is unchanged.Strings
19 page keys and 3 native keys, all in 21 locales.
Tests
CmuxNextDesignTests/BackdropCollagesTests:CmuxNextPagesTests: collage save, get and delete; a malformed collage is refused; the in-use one can't be deleted.collageLayout.test.ts: snapping, staying on the canvas, minimum size, layouts and restackinghostCards.test.tsx: New Collage adds two pictures, applies Side by Side, saves and setscollage:<id>, the in-use one can't be removed, and an edit saves withreplacingvp lint,vp fmtandtscare clean.Verification
Not in this PR (follow-ups)
Changelog
Added: Collages: arrange several pictures as one background, with snapping, layouts, rounded corners and a fill color.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Adds collages as a background: several pictures arranged on one 16:10 canvas, drawn natively to a PNG at 3200×2000 and set via
appearance.background = collage:<id>.Collages
Backgrounds/Collages/as a JSON record plus a drawing; a saved edit gets a new id, and old files are removed only after the background switches.cmux.settings.backdrop.image.collage.save | get | delete.Carried from earlier phases
cmux.settings.backdrop.image.adjust), saved per background and removed with the picture.Written for commit de6d12f. Summary will update on new commits.