core: add the preview screenshot scenario - #182
Merged
Conversation
The media kit needs a frame for the preview feature: an archive open with README.md selected. No demo archive looked like a source project, so 09-preview opens markdown-kit.zip — a small but genuinely working JavaScript repository, committed to MacPacker-TestArchives rather than generated, because it is also the fixture for archiving a checkout that honors its .gitignore. Quick Look runs in another process that SandboxPilot cannot capture, so the scenario produces only MacPacker's window with the file selected; FrameBison composes the preview panel on top. make_demo_archives.py copies that fixture into ~/Downloads/MacPacker-Demo alongside the archives it generates. It has to be copied rather than read in place: MacPacker is sandboxed to the downloads entitlement and cannot reach the submodule inside the repo. Also requires SandboxPilotKit 1.3.0, which applies a screenshot run's appearance at launch instead of flipping the running app — without it the search field keeps its dark-resolved white text through the light pass and draws white on white.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe project updates the ChangesArchive preview and search behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ArchiveWindowController
participant NSWindow
participant SearchField
participant MacPackerUITests
ArchiveWindowController->>NSWindow: Publish accessibility identifiers
NSWindow->>SearchField: Assign archive.searchField
MacPackerUITests->>SearchField: Locate and verify the search field
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/screenshots/make_demo_archives.py`:
- Around line 150-152: Update stage(FIXTURE) to fail when the required fixture
source is missing instead of printing “skip” and returning successfully; raise
an error or propagate a failure status that aborts the main flow, while
preserving skip behavior for non-required sources handled separately.
In `@MacPacker.xcodeproj/project.pbxproj`:
- Line 1385: Update the Package.resolved lockfile entry for SandboxPilotKit to a
resolved version compatible with the minimumVersion 1.3.0 requirement, including
its corresponding revision metadata; do not change the project setting itself.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 76074cbc-371c-4c4a-a0e2-94b3ca67b6cf
📒 Files selected for processing (3)
MacPacker.xcodeproj/project.pbxprojassets/screenshots/make_demo_archives.pysandboxpilot.json
Comment on lines
+150
to
+152
| if not os.path.exists(source): | ||
| print(f"skip {name} (no {relative_path} — run `git submodule update --init`)") | ||
| return |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fail when the required fixture is missing.
stage(FIXTURE) prints skip and returns successfully when the submodule is not initialized. The 09-preview scenario then has no markdown-kit.zip, but the script reports success.
Raise an error for this required fixture, or return a failure status and abort the main flow.
Proposed fix
if not os.path.exists(source):
- print(f"skip {name} (no {relative_path} — run `git submodule update --init`)")
- return
+ raise SystemExit(
+ f"missing {name} ({relative_path} — run `git submodule update --init`)"
+ )Also applies to: 161-161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@assets/screenshots/make_demo_archives.py` around lines 150 - 152, Update
stage(FIXTURE) to fail when the required fixture source is missing instead of
printing “skip” and returning successfully; raise an error or propagate a
failure status that aborts the main flow, while preserving skip behavior for
non-required sources handled separately.
1.3.0 traps on launch whenever a screenshot run left an appearance behind: NSApp does not exist yet when SandboxPilot.start() runs from the app's initializer. See LeanBytes/SandboxPilotKit#3.
FocusSearchWhenLaunchedIntoOne forced the cursor into the search field when a window was launched into a search, because an unfocused field appeared to draw its text in the placeholder's grey, unreadable in the light appearance. That grey was the stale-appearance bug, not a real unfocused style: the screenshot run relaunched the app in the system appearance and flipped it afterwards, so text colours stayed resolved against the previous one. With SandboxPilotKit 1.3.1 applying the appearance at launch, an unfocused field draws normal text in both appearances — verified in light and dark. Removing it takes the last piece of screenshot-only behaviour out of the window code, and the shots read better without a focus ring and a caret in them.
Splits the two things a screenshot scenario needs. Launch parameters keep deciding state — which archive, which path, which selection — and the plan's new `actions` drive the UI on top, naming a control by accessibility identifier for SandboxPilotKit to act on inside MacPacker. No Accessibility permission is involved, because a process may drive its own accessibility tree, and neither SandboxPilot nor the kit learns anything about MacPacker: sandboxpilot.json stays the only place that knows the control exists. So 04-search asks for the search field to be focused, where until now the app focused it itself on seeing -SearchQuery. SwiftUI gives the field behind `.searchable` no identifier — the modifier takes none, and applying one sets it on the container — so the window publishes it through the toolbar item that backs the field, the same route ⌘F already uses. That is ordinary accessibility work: VoiceOver and the UI tests want identifiers regardless, and the existing suite resorts to matching toolbar buttons by geometry for want of them. Requires SandboxPilotKit 1.4.0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The media kit needs a ninth frame: an archive open with
README.mdselected, to show the preview feature. No demo archive looked like a source project.09-previewQuick Look runs in another process SandboxPilot cannot capture, so the scenario produces only MacPacker's window with the file selected — FrameBison composes the preview panel on top. Verified by launching the debug build with those arguments and capturing the window:
Entries at the archive root:
src/,tests/,.gitignore,LICENSE,package.json,README.md— reads as a checkout at a glance, no-NavigatePathneeded.The archive is committed, not generated
markdown-kit.zipis a small but genuinely working repository —npm install,npm run buildandnpm testall succeed in it. That is deliberate: it is also the fixture for archiving a checkout while honoring its.gitignore, which needs thenode_modules/anddist/that.gitignorenames to actually exist. So it lives in MacPacker-TestArchives (sarensw/MacPacker-TestArchives#1) with azip/make_markdown_kit.shthat rebuilds it byte-identically.make_demo_archives.pytherefore gains a copy step next to its generate steps. It has to copy rather than read in place: MacPacker is sandboxed to the downloads entitlement and cannot reach the submodule inside the repo, andLaunchParameters.demoFile(named:)only looks in~/Downloads/MacPacker-Demo. Its docstring also now says out loud that nothing runs it for you — it is a manual pre-session step, which it always was.SandboxPilotKit 1.3.0
Bumped from 1.1.0. A screenshot run used to override the appearance on the already-running app; AppKit resolves some colors once, when a control is set up, so the search field focused at launch kept its dark-resolved white text through the flip to light and drew white on a light field. 1.3.0 applies the appearance at launch instead. See LeanBytes/SandboxPilotKit#2 and LeanBytes/SandboxPilot#32.
Sequencing
xcodebuildresolves again.swift test --package-path Modulesis unaffected, since the kit is an Xcode-project dependency only.No changelog entry: the plan file, a test fixture, the staging script and a DEBUG-only dependency pin are not user-visible. No existing issue covers this and I did not open one just to link.
Summary by CodeRabbit
New Features
markdown-kit.zipwithREADME.mdselected, enabling composited Quick Look screenshots.Bug Fixes
Documentation