Skip to content

Preserve window width ratio on cross-workspace move#365

Open
biswadip-paul wants to merge 1 commit into
BarutSRB:mainfrom
biswadip-paul:fix/preserve-column-width-on-cross-workspace-move
Open

Preserve window width ratio on cross-workspace move#365
biswadip-paul wants to merge 1 commit into
BarutSRB:mainfrom
biswadip-paul:fix/preserve-column-width-on-cross-workspace-move

Conversation

@biswadip-paul
Copy link
Copy Markdown

@biswadip-paul biswadip-paul commented May 27, 2026

Summary

  • Replaces initializeNewColumnWidth() with copyColumnWidthState() in moveWindowToWorkspace()
  • Preserves the source column's width proportions, preset index, full-width state, and manual override flag when moving a window to a different workspace

Details

When moving a single window to another workspace (including cross-monitor moves), moveWindowToWorkspace() was calling initializeNewColumnWidth() which resets the target column to the workspace's default width. This discards any manual resizing the user had done.

The fix uses the existing copyColumnWidthState() helper (already used for same-workspace column moves in moveColumnTo* operations) to carry over the source column's width state. Note that moveColumnToWorkspace() already preserves width correctly since it transfers the entire column without re-initializing.

Test plan

  • Resize a column to a custom width, then move it to a workspace on a different monitor — width ratio should be preserved
  • Move a full-width column across workspaces — full-width state should be preserved
  • Move a window from a preset-width column — preset index should transfer
  • Existing layout engine tests continue to pass

Fixes #295

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Improved window column width state handling when moving windows between workspaces for more consistent sizing behavior.

Review Change Stack

… width when moving window across workspaces moveWindowToWorkspace() was calling initializeNewColumnWidth() which resets the column to the workspace default width. Replace with copyColumnWidthState() to preserve the source column's width proportions, preset index, and full-width state. Fixes #295 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d093df3c-6ecc-49c0-9d6e-8d650fa40867

📥 Commits

Reviewing files that changed from the base of the PR and between 917e270 and 41480dd.

📒 Files selected for processing (1)
  • Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine+WorkspaceOps.swift

📝 Walkthrough

Walkthrough

This PR fixes a bug where windows lost their width when moved to a different workspace on another screen. The moveWindowToWorkspace method now copies the source column's width state to the target column instead of initializing it to defaults.

Changes

Window width preservation on workspace moves

Layer / File(s) Summary
Width state copying in moveWindowToWorkspace
Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine+WorkspaceOps.swift
When moving a window to a target workspace, the method replaces initializeNewColumnWidth with copyColumnWidthState calls. Width state is copied from the source column to either an existing empty column or a newly created column, ensuring the window maintains its width after the move.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 A window's width is now preserved,
As between workspaces it's swerved,
Copy state, not reset—hooray!
Width stays the same, come what may! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Preserve window width ratio on cross-workspace move' clearly and concisely describes the main change, matching the objective to maintain window width proportions when moving windows between workspaces.
Linked Issues check ✅ Passed The code change directly addresses issue #295 by replacing initializeNewColumnWidth() with copyColumnWidthState(), which preserves column width proportions, preset index, full-width state, and manual override flag during single-window moves.
Out of Scope Changes check ✅ Passed The changes are limited to the moveWindowToWorkspace() method in the Niri layout engine and directly address the scope of issue #295 regarding cross-workspace window moves.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Guria
Copy link
Copy Markdown

Guria commented May 28, 2026

I also notice when window lost it width when moved between columns

@biswadip-paul
Copy link
Copy Markdown
Author

Good catch @Guria — that's a related but separate bug. This PR fixes width loss on cross-workspace moves (moveWindowToWorkspace), but the same pattern exists for within-workspace column moves:

  • createColumnAndMove() in NiriLayoutEngine+ColumnOps.swift (line ~98) calls initializeNewColumnWidth() instead of copyColumnWidthState() — this is the move-left/move-right path when extracting a window from a stacked column.
  • insertWindowInNewColumn() (line ~149) has the same issue during drag reordering.

Interestingly, expelWindow() and expelWindowRight() already use copyColumnWidthState() correctly — so the fix pattern is proven, it's just not applied consistently.

I can open a follow-up PR for the column move case if the maintainer prefers to keep the scope separate, or expand this one. @BarutSRB what's your preference?

@BarutSRB
Copy link
Copy Markdown
Owner

I also notice when window lost it width when moved between columns

Omni is literally doing what Niri does, as you consume/expel it inherits the width of the column it got expelled from, unless you mean something else?

@biswadip-paul
Copy link
Copy Markdown
Author

You're right — I traced this more carefully and the move-left/move-right path (moveWindowconsumeOrExpelWindowexpelWindow) already uses copyColumnWidthState, so width is correctly preserved there. My earlier mention of createColumnAndMove() was wrong for that path; it's defined but not called from any production code path (only tests).

The one place I do see initializeNewColumnWidth in a live code path is insertWindowInNewColumn(), which is called from overview drag reordering and window summon — but resetting to default width may be intentional for those operations since there's no obvious "source column" to inherit from.

@Guria — could you clarify what operation triggers the width loss you're seeing? Is it consume/expel (Mod+Shift+Left/Right), or something else like dragging in the overview?

@Guria
Copy link
Copy Markdown

Guria commented May 28, 2026

I will monitor it and tell later what exactly confuses me. I have no yet exact steps. Feel free to ignore it for now

bispaul added a commit to bispaul/OmniWM that referenced this pull request Jun 4, 2026
…kspace move (PR BarutSRB#365)

Cherry-picked from biswadip-paul PR BarutSRB#365. Uses copyColumnWidthState
instead of initializeNewColumnWidth when moving windows between
workspaces, preserving the source column's width proportions.

Also changed copyColumnWidthState from private to internal for
cross-extension access.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

[Bug] [Niri] Windows do not keep their current width when moved to another workspace on another screen

4 participants