Skip to content

fix(columns): share the right pane between child columns and the preview - #1405

Merged
l0gicgate merged 11 commits into
mainfrom
fix/1404-columns-preview-right-pane
Oct 3, 2026
Merged

l0gicgate merged 11 commits into
mainfrom
fix/1404-columns-preview-right-pane

Conversation

@ogarza

@ogarza ogarza commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

Description

In Columns, keyboard mirroring appended the child column inside the columns scroller beside the reserved preview slot. Once the window was deep enough that the slot sat at its minimum width, every Up/Down onto a folder scrolled the columns left to fit the child, and every Up/Down onto a file closed the child and let GTK clamp the scroll back. The focused column bounced by one column width on every keypress.

The area right of the focused column is now one right pane shared by the child column and the preview:

  • The preview geometry counts only the navigated columns (root through the active column). While nothing is displayed, the empty reserved slot lends trailing columns exactly their width, so the scroller and its content grow and shrink together and the scroll offset never changes.
  • A focused folder hides the drawer instead of showing the placeholder and hands the space to its child column. Files with no preview and empty selections keep the "No preview for this selection" placeholder.
  • A preview closed on purpose (Space, i, Esc, close button, Appearance → Preview panel) stays closed while keyboard mirroring continues, until it is opened explicitly again. Previously a Down onto the next previewable file reopened it.
  • A pointer preview of a file in a parent column closes deeper columns first, matching the keyboard mirror.
  • Switching Appearance → Preview panel off is an explicit dismissal even when nothing was previewed at that moment. Previously the next keyboard step onto a previewable file reopened the panel, re-reserved the space and shifted the columns (and in a narrow window collapsed the sidebar).
  • Dragging the divider on an empty slot accounts for the lent width so the next frame does not shrink it again.
  • Priority in space-constrained windows: focused column, then preview, then peek. Nothing is reserved for a peek any more (the old budget depended on the viewport size and neighbour count, so it toggled with the panel state and moved columns by 48 px). A fully visible column never moves; otherwise one rule aligns the end of the strip with the right pane without scrolling past the focused column's left edge. A trailing child that does not fit in the lent slot is clipped instead of pushing the focused column; revealing a column beyond the active one reveals the active column.
  • Sidebar squeeze: a divider position pinned by a narrow window was being recorded as the user's sidebar width, and the preview was then sized against that clamped width, locking the squeeze in. The handler ignores pinned positions (judged by the pane's own geometry, since the child allocation can be stale), the preview geometry uses the intended width, and the sidebar is restored as soon as the content has room.
  • Windows too narrow for any preview: the reserved slot no longer disappears. It shrinks to whatever remains beside the focused column, down to zero, and only the preview content hides below 240 px. Without this, the mirrored child column extended the scroller and closing it let GTK clamp the offset, so stepping from a folder onto a file shifted the column. A narrower viewport now also reveals a clipped focused column immediately instead of on the next keypress.
  • reveal_column measures the viewport one frame later, after the slot has resized for the new active column, so entering a mirrored child column scrolls it into view instead of leaving it clipped.
  • E2E harness: wait_for_directory settles the pane it waited for, and view_mode waits for a pane instead of asserting on a transient empty tree after a window resize.

The complete sizing, reservation, dismissal, and peek-strip rules now live in docs/preview-panel-layout.md, with the owning test for each rule, so later changes have a contract to check against.

Visual evidence

Pending: the owner will upload the before/after captures through the GitHub editor.

  • Before: mirroring onto a folder at depth three pushed the focused level3 column left and clipped the child beside the placeholder.
  • After (folder): level3 stays in place and the branch child column takes the preview's space.
  • After (file): level3 still in place with the preview meeting its right edge.

How to test

  1. Open Strata in Columns with the defaults (single-click previews and Mirror columns selection on).
  2. Press Right into nested folders three or four times until the focused column sits beside the minimum-width preview slot.
  3. Press Up/Down to alternate between a folder and a file in that column.
  4. Press Space to close the preview, then Down onto another previewable file, then Space again.
  5. With deeper columns open, press Left and click a file in the parent column.

Expected result: In step 3 the focused column never moves; the right pane alternates between the child column and the preview, both starting at the focused column's right edge. In step 4 the preview stays closed after Down and reopens on Space, after which it follows the selection again. In step 5 the deeper columns close and the preview opens beside the clicked file's column.

Related issue

Closes #1404

ogarza added 2 commits October 3, 2026 17:07
Keyboard mirroring appended the child column inside the columns scroller
beside the reserved preview slot, so at the slot's minimum width every
folder/file step scrolled the focused column back and forth. The empty
slot now lends trailing columns their width, the preview geometry counts
only the navigated columns, and a focused folder hides the drawer instead
of showing a placeholder.

An explicit close marks the drawer dismissed so the keyboard mirror stops
reopening it, a pointer preview closes deeper columns first, and column
reveals keep one constant peek sliver instead of a viewport-dependent
budget.

Refs #1404
@ogarza
ogarza marked this pull request as draft October 3, 2026 21:23
ogarza and others added 6 commits October 3, 2026 17:41
Nothing is reserved for a peek sliver any more: the preview takes what
remains beside the focused column, and column reveals use one rule that
aligns the strip with the right pane without scrolling past the focused
column. A trailing child that does not fit the lent slot is clipped
instead of moving the focused column.

A divider position pinned by a narrow window is no longer recorded as
the user's sidebar width; the preview is sized against the intended
width and the sidebar is restored once the content has room.

Refs #1404
Windows too narrow for a preview hid the reserved slot, so the mirrored
child column extended the scroller and closing it let GTK clamp the
offset. The slot now shrinks to whatever remains beside the focused
column and only the preview content hides below its threshold. A
narrower viewport reveals a clipped focused column immediately, and a
divider position pinned by the window is judged by the pane's own
geometry so a stale child allocation cannot record it.

Refs #1404
Switching Appearance → Preview panel off while nothing was previewed left
the keyboard mirror free to reopen the panel on the next previewable
file, re-reserving the space and shifting the columns.

Refs #1404
Replace added geometry assertions with manual verification, remove the event-setter echo test, and audit all added comments. Retain folder/file lifecycle, dismissal, pointer preview ordering, and archive key-routing coverage. Refs #1404
@ogarza

ogarza commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

Handoff

Branch fix/1404-columns-preview-right-pane is complete from my side; someone else is picking it up from here.

Commits

  1. fix(columns): share the right pane between child columns and the preview — the core fix for Columns: focused column bounces when keyboard mirroring alternates folders and previews #1404 plus dismissal and the pointer-preview truncation.
  2. docs(preview): document the preview panel and column layout contract — docs/preview-panel-layout.md, the contract and the owning test per rule.
  3. fix(columns): keep the focused column first in narrow windows — priority order focused column > preview > peek, no peek reservation, sidebar squeeze fix.
  4. fix(columns): keep the reserved slot at every window width — the slot never disappears in Columns; resize reveals a clipped focused column; squeeze guard uses the pane geometry.
  5. fix(preview): treat releasing the panel as an explicit dismissal — Appearance → Preview panel off blocks keyboard-mirror reopening in every state.

Validation

  • Commits 1 to 4: full pinned Rust suite (2073 passed, 9 ignored), pinned fmt and clippy, full E2E suite (901 passed). Container engine was Docker.
  • Commit 5 (one-line behaviour change): drawer unit tests, plus the test_quick_preview.py, test_preview_session.py and test_preferences.py E2E files (59 passed), pinned fmt and clippy. Full suites were not rerun for this commit.
  • The stationary scenario fails on the unfixed source and passes on the branch, in both wide and narrow (820 px) cases.

Open items

  • Visual evidence is pending. Before/after captures exist locally only and must be uploaded through the GitHub editor, or re-recorded with this build: a folder mirrored at depth three (before: focused column pushed left; after: child column takes the preview's space) and a file selected (preview meets the focused column).
  • Drag auto-scroll in windows about one column wide starts as soon as a row drag nears the strip edge, then the focused column snaps back on release. Left as is pending a product decision: keep, or start auto-scroll only over a visible parent sliver (small change in src/ui/browser/columns/drag_scroll.rs).
  • Columns: keyboard focus moves to a panel when the sidebar rails with a preview open #1406 (pre-existing): keyboard focus moves to an unnamed panel when the sidebar rails while a preview is open. Reproduced on main; not caused by this PR.
  • E2E harness changes in this PR: wait_for_directory settles the pane it waited for, and view_mode waits for a pane instead of asserting on a transient empty tree.

Manual check (also in the doc): Columns, defaults, Right three or four times into nested folders until the preview slot is at its minimum, then Up/Down across folders and files. The focused column must not move; the right pane alternates between the child column and the preview. Space closes the preview and Down must leave it closed; Space reopens it. Appearance → Preview panel off must also stay off while stepping onto files.

Bind the GTK fixture to a Columns view so the initial reservation is present while the drawer is disabled. Keep startup release and content dismissal as distinct E2E input routes; remove duplicate layout assertions. Refs #1404
@l0gicgate
l0gicgate marked this pull request as ready for review October 3, 2026 22:41
@l0gicgate

l0gicgate commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Reviewed and pushed merge preparation, without merging the PR:

  • Merged verified lgse/strata main f2051aa76fe3f08d3b8d0d25567e9b5de0a70293, retaining both independent preview regressions at the conflict; integrated the author's concurrent dismissal fix without force-pushing.
  • Strengthened startup Appearance dismissal coverage using an actual reserved Columns drawer. Kept Space dismissal/reopen, folder/file lifecycle, pointer-preview ordering, and archive key-routing coverage.
  • Fixed a reproducible CI fixture race: Send to tests now wait for enumeration and refresh completion using the existing menu helper, not fixed 80 ms delays. All hot-plug/reopen/lifetime assertions remain.
  • Audited every added/modified test and comment. Removed geometry-only Rust/E2E assertions and the event-setter echo test, tightened comments to non-obvious rationale, and corrected documentation. No submitted reviews or inline threads exist; no high-confidence security vulnerabilities identified.

Pinned rootless-Podman validation, with private displays/buses: ./scripts/quality.sh fmt, clippy, and test passed (2,162 passed; 9 existing ignores). Full canonical E2E: 905 passed, one copy-conflict Return focus timeout. Its exact reproduction and all 9 copy-conflict cases then passed; the timeout is not claimed fixed. Affected coverage passed: 1 GTK dismissal test, 8 preview E2E cases, and all 6 context-menu tests. After the CI fixture fix, the complete formatting/Clippy/Rust suite passed again (2,162 passed; 9 existing ignores). git diff --check and documentation/link review passed.

Manually inspected current captures at widths 1440, 820, and 500: the focused column stays put, folder/file selection exchanges the right pane, narrow preview content hides, and dismissal survives selection.

Visual evidence pending: attachment-upload tooling is unavailable. Please upload these files through GitHub's editor; no media was committed. All are under /home/l0gicgate/.cache/pi-tmp/pr1405-review.lQwPdq/media/:

  • 820-folder.png: focused level3 with child branch to its right.
  • 820-file-return.png: same column with the file preview to its right.
  • startup-panel-off-file.png: startup panel release survives keyboard selection of a previewable file.

All required checks are green on f196e443: final PR CI, attempt 2 verified all 906 E2E tests passed exactly once, with no skips, and complete Rust coverage. Earlier resize/rename and drag-setup timeouts passed their exact/affected local reruns; they are not claimed fixed, and their logs/captures remain retained. The reproducible Send to race was fixed before this final run.

Author handoff reconciled: independently reproduced #1406 on the fetched main baseline (focus moved from b.txt to an unnamed panel after railing; Up left selection unchanged). It is pre-existing, not introduced here, and remains a separate non-blocking issue. The narrow-window drag-scroll policy is a pending product choice, not a merge blocker; left unchanged, with both drag-edge regressions passing. Visual upload remains pending, and GitHub still requires human approval. The PR has not been merged.

@blacksmith-sh

This comment has been minimized.

Replace fixed 80 ms delays with the existing bounded menu-condition helper. Preserve hot-plug, unplug, reopening, navigation, and subscription-lifetime assertions. This fixes the source-entry race reproduced from PR 1405 CI attempt 2. Refs #1404
@blacksmith-sh

This comment has been minimized.

@l0gicgate
l0gicgate merged commit 2c32c54 into main Oct 3, 2026
22 of 24 checks passed
@l0gicgate
l0gicgate deleted the fix/1404-columns-preview-right-pane branch October 5, 2026 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Columns: focused column bounces when keyboard mirroring alternates folders and previews

2 participants