Skip to content

Respect window min size constraints in Niri column width#384

Open
biswadip-paul wants to merge 2 commits into
BarutSRB:mainfrom
biswadip-paul:fix/respect-min-size-in-niri-column-width
Open

Respect window min size constraints in Niri column width#384
biswadip-paul wants to merge 2 commits into
BarutSRB:mainfrom
biswadip-paul:fix/respect-min-size-in-niri-column-width

Conversation

@biswadip-paul
Copy link
Copy Markdown

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

Summary

  • Apps with minimum size constraints (e.g., WhatsApp) triggered resize placeholders or got parked offscreen when the Niri column width preset was narrower than the app's minimum width
  • Root cause: layoutConstraints were unconditionally relaxed via relaxedForResizePlaceholder(), dropping minWidth/minHeight to 1 for all windows — preventing the layout engine from clamping column widths to respect app minimum sizes
  • Fix: only relax constraints for windows already in a resize placeholder state. For all other windows, full constraints propagate to the layout engine, allowing resolveSpan() to clamp column widths up to the app's minimum size
  • Updated existing test to verify the new behavior and added a test for the resize placeholder persistence path

Fixes #383

Test plan

  • Open an app with known minimum width (e.g., WhatsApp) with a column preset of 50% or less
  • Verify the column auto-expands to the app's minimum width instead of showing a resize placeholder
  • Verify resize placeholders still work for windows already in that state (e.g., after AX frame apply fallback)
  • Run existing test suite — NiriLayoutEngineTests

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Column widths now correctly clamp to respect large minimum window sizes during layout.
    • Resize placeholder handling improved so window constraints are preserved during layout changes.
  • Tests

    • Added/updated regression tests to validate clamped widths and correct resize-placeholder behavior.

Review Change Stack

…size constraints in Niri column width resolution Previously, layoutConstraints were unconditionally relaxed via relaxedForResizePlaceholder(), dropping minWidth/minHeight to 1 for all windows. This meant the layout engine couldn't clamp column widths to respect app minimum sizes, causing apps like WhatsApp to trigger resize placeholders (or get parked offscreen) when the column preset was narrower than the app's minimum width. Now, constraints are only relaxed for windows already in a resize placeholder state. For all other windows, the full constraints propagate to the layout engine, allowing resolveSpan() to clamp column widths up to the app's minimum size. This prevents unnecessary resize placeholders and keeps apps usable at any column width preset. Fixes BarutSRB#383 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 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: aacb7d82-8ad8-45eb-aa44-66ac24af7525

📥 Commits

Reviewing files that changed from the base of the PR and between 1cbd606 and 8f8119c.

📒 Files selected for processing (1)
  • Tests/OmniWMTests/NiriLayoutEngineTests.swift

📝 Walkthrough

Walkthrough

buildWindowSnapshots now reads resizePlaceholderState first and applies relaxed constraints only when that state exists; otherwise it uses mergedConstraints. Tests add a regression verifying large minimum sizes clamp widths without emitting a placeholder and seed a placeholder in another test to validate the relaxed-constraint path.

Changes

Conditional Constraint Relaxation for Resize Placeholders

Layer / File(s) Summary
Constraint selection in snapshot builder
Sources/OmniWM/Core/Controller/LayoutRefreshController.swift
buildWindowSnapshots now computes resizePlaceholderState before deriving layoutConstraints; it applies mergedConstraints.relaxedForResizePlaceholder() only when a placeholder state exists and otherwise uses mergedConstraints directly.
Regression: large-minimum-size clamps column width
Tests/OmniWMTests/NiriLayoutEngineTests.swift
Adds windowWithLargeMinSizeClampsColumnWidthInNiri: enables Niri layout, sets very large minimum window size constraints for a token, requests a layout plan, asserts a frame change with width >= constraint minimum, and asserts no resize placeholder is emitted for that token.
Seeded placeholder test setup
Tests/OmniWMTests/NiriLayoutEngineTests.swift
Updates existingResizePlaceholderKeepsRelaxedConstraintsInNiri to call workspaceManager.setResizePlaceholderState(...) for the target token with a placeholder frame and minimumSize: constraints.minSize before generating the layout plan.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
I peeked at windows, tiny and grand,
Held constraints tight in my little hand,
No ghostly resizer to muddle the view,
Widths now obey the minimum true,
A happy hop for a layout anew!

🚥 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 title clearly and specifically summarizes the main change: respecting window minimum size constraints in Niri column width calculations.
Linked Issues check ✅ Passed The PR addresses all coding requirements from issue #383: it modifies constraint relaxation logic to respect app-declared minimum sizes, prevents unnecessary resize placeholders, and preserves existing behavior for windows already in resize state.
Out of Scope Changes check ✅ Passed All changes are directly related to the stated objective of respecting window minimum size constraints in Niri layout; no unrelated modifications detected.

✏️ 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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@Tests/OmniWMTests/NiriLayoutEngineTests.swift`:
- Around line 4093-4096: The test force-unwraps frameChange after a prior
`#expect`, which can crash and hide the real failure; instead safely unwrap the
optional returned from plan.diff.frameChanges.first { $0.token == token } (e.g.,
use guard let or if let to bind frameChange) and then run the subsequent
assertions on the bound value (replace direct use of frameChange! with the
unwrapped variable before checking .frame.width), keeping the original `#expect`
check or converting it into an explicit failure path that reports a clear test
error if the optional is nil.
🪄 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: 44d831ee-8217-4912-8222-891a1626bd10

📥 Commits

Reviewing files that changed from the base of the PR and between 777f1c7 and 1cbd606.

📒 Files selected for processing (2)
  • Sources/OmniWM/Core/Controller/LayoutRefreshController.swift
  • Tests/OmniWMTests/NiriLayoutEngineTests.swift

Comment thread Tests/OmniWMTests/NiriLayoutEngineTests.swift Outdated
…th guard-let in test assertion Avoids crashing the test runner if frameChange is nil, surfacing the failure via Issue.record instead. 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.

[0.4.9.6] Minimum size issue in applications like WhatsApp when using presets of 50% or less

2 participants