feat(expose): inset opaque panel over a dimmed, correctly-sized backdrop#169
Merged
Conversation
Make exposé a proper inset modal over the dimmed backdrop: the popup goes borderless-fullscreen so exposé paints an edge-to-edge dimmed host backdrop, then floats a centered ~90% opaque bordered panel holding the title, tiles, and help — so the body is solid (no see-through) and inset like before. Wrap each frame in synchronized output and only clear on resize so the timer-driven repaint lands atomically without flashing the panel body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Render the backdrop, panel, and tile chrome before capturing tile previews so entry shows the framed exposé at once instead of a blank wait; previews fill in on the next tick. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Opening the fullscreen popup transiently reflows the host pane, so capturing the backdrop from inside exposé caught a mis-sized, off-centre frame. Capture the host in the launcher before display-popup, pass it via --backdrop-file, and have exposé read then delete that snapshot (in-popup capture kept as a fallback). The dimmed backdrop now mirrors the host at its real width. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Owner
Author
|
@coderabbitai review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a pre-popup host pane snapshot mechanism to the exposé feature. The shell launcher captures the current pane to a temp file and passes it via ChangesExposé backdrop snapshot and panel overlay
Sequence Diagram(s)sequenceDiagram
participant Shell as show_local_expose
participant tmux_cmd as tmux capture-pane
participant CLI as aimux expose
participant Core as runTmuxExpose
Shell->>tmux_cmd: capture-pane -p to tempfile
tmux_cmd-->>Shell: exit 0 (success) or error
alt capture succeeded
Shell->>CLI: --backdrop-file <tempfile>
CLI->>Core: backdropFile: opts.backdropFile
else capture failed
Shell->>Shell: rm tempfile
Shell->>CLI: (no --backdrop-file)
end
Core->>Core: readFileSync(backdropFile)
Core->>Core: unlinkSync(backdropFile)
Core->>Core: panelGeometry(cols, rows) → geo
Core->>Core: buildPanelFrame(geo) → frame with borders
Core->>Core: render() immediately with panel overlay
Core->>Core: refreshCaptures() updates previews in panel
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
✅ Action performedReview finished.
|
![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
🧹 Nitpick comments (1)
src/tmux/control-script.test.ts (1)
1359-1359: ⚡ Quick winAlso assert
--backdrop-fileis present in the generated expose command.This test now checks
-B, but the new cross-layer contract in this PR is the backdrop-file plumbing. Adding that assertion would catch regressions earlier.✅ Suggested assertion extension
- entry.includes("display-popup -c /dev/live -T aimux exposé -x C -y C -w 100% -h 100% -B -E exec"), + entry.includes("display-popup -c /dev/live -T aimux exposé -x C -y C -w 100% -h 100% -B -E exec") && + entry.includes(" --backdrop-file "),🤖 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 `@src/tmux/control-script.test.ts` at line 1359, The test assertion at line 1359 in the control-script.test.ts file currently verifies that the display-popup command includes the `-B` flag, but it does not assert that the `--backdrop-file` argument is present in the generated expose command. Add an additional assertion (or extend the existing entry.includes check) to verify that `--backdrop-file` is included in the command string to ensure the backdrop-file plumbing is working correctly and catch any regressions in this cross-layer contract.
🤖 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 `@scripts/tmux-control.sh`:
- Around line 412-425: The temporary backdrop file stored in expose_backdrop
variable is not being cleaned up if the tmux display-popup command fails. Add
cleanup logic to remove the expose_backdrop file before each return 1 statement
that follows a failed tmux display-popup call. Since there are two tmux
display-popup invocations (one with the -c option and one without), ensure the
cleanup happens for both failure paths. Use rm -f on the expose_backdrop
variable to safely remove the file if it exists before the function returns.
In `@src/tmux/expose.ts`:
- Around line 110-113: The panelGeometry function's width and height
calculations can exceed the actual viewport dimensions on very small terminals
because the Math.max operations with MIN_TILE_WIDTH and MIN_TILE_HEIGHT are
applied after clamping to cols and rows, allowing the minimums to override the
bounds. Add an additional Math.min operation to clamp the final width value to
cols and the final height value to rows after all other calculations in the
panelGeometry function, ensuring the panel dimensions never exceed the actual
viewport bounds.
---
Nitpick comments:
In `@src/tmux/control-script.test.ts`:
- Line 1359: The test assertion at line 1359 in the control-script.test.ts file
currently verifies that the display-popup command includes the `-B` flag, but it
does not assert that the `--backdrop-file` argument is present in the generated
expose command. Add an additional assertion (or extend the existing
entry.includes check) to verify that `--backdrop-file` is included in the
command string to ensure the backdrop-file plumbing is working correctly and
catch any regressions in this cross-layer contract.
🪄 Autofix (Beta)
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
Run ID: 4f784250-708c-4162-bc63-cbc48d1dcbc3
📒 Files selected for processing (5)
scripts/tmux-control.shsrc/main.tssrc/tmux/control-script.test.tssrc/tmux/expose-tile.test.tssrc/tmux/expose.ts
…lure - panelGeometry: clamp width/height to the viewport last so the minimum-size floor can't make the panel larger than a tiny screen. - tmux-control.sh: remove the captured backdrop temp file if display-popup fails (exposé otherwise reads then deletes it). - prefer the exact host window id over the session in the capture fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Concurrent sub-agent review (orphan findings):
Geometry, BSU ( |
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.
What
Iterates the exposé "dimmed real backdrop" into a proper inset modal and fixes its rough edges.
display-popup, passed via--backdrop-file, and read+deleted by exposé — so it mirrors the host at its real width.How
src/tmux/expose.ts— newpanelGeometry/buildPanelFrame(inset opaque frame); render insets title/tiles/help into the panel over a full-screenbuildBackdrop; synchronized-output + resize-only clear; render-before-capture; read the launcher's--backdrop-filesnapshot (in-popup capture kept as fallback).scripts/tmux-control.sh— exposé popup is borderless fullscreen (-w 100% -h 100% -B); capture the host (capture-pane -e -S 0) to a temp file before opening the popup and pass--backdrop-file.src/main.ts—--backdrop-fileoption plumbed torunTmuxExpose.Tests
panelGeometry(centered inset, clamps),buildPanelFrame(opaque bordered box).control-script.test.tsfor the fullscreen-Bpopup.yarn buildclean. Verified the centering fix by rendering a real pre-popup dashboard capture (title lands centered at col 77 of 195).Notes
aimux exposeNode cold-start blank.🤖 Generated with Claude Code
Summary by CodeRabbit
--backdrop-file <path>option to allow using a pre-supplied backdrop snapshot.