Skip to content

joryirving/windowstead #54: fix popup clipping in bottom dock mode - #55

Merged
itsmiso-ai merged 1 commit into
mainfrom
fix/issue-54-popup-clip
Apr 18, 2026
Merged

joryirving/windowstead #54: fix popup clipping in bottom dock mode#55
itsmiso-ai merged 1 commit into
mainfrom
fix/issue-54-popup-clip

Conversation

@itsmiso-ai

Copy link
Copy Markdown
Contributor

Fixes #54. Partial fix — expanded dock width in bottom mode to account for the popup sidebar width, preventing the menu popup from being clipped or inaccessible. Added SIDEBAR_WIDTH constant and updated dock_size_for_anchor to include sidebar + gap width. Also updated position_popup_panel to use the SIDEBAR_WIDTH constant for bottom mode positioning.

@smurf-bot

smurf-bot Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Automated recommendation: APPROVE

Analysis engine: MiniMax-M2.7@https://api.minimax.io/v1

PR Review: fix popup clipping in bottom dock mode (PR #55)

Summary

This PR addresses issue #54 by expanding the dock width in bottom mode to accommodate the popup sidebar width, preventing the menu popup from being clipped or inaccessible.

Change-by-Change Findings

  1. New constant SIDEBAR_WIDTH := 240 (line 14)

    • Introduces a named constant for sidebar width.
    • Note: The value 240 is already hardcoded in apply_anchor_layout() at line 202 as sidebar_scroll.custom_minimum_size = Vector2(240, 200). Consider consolidating to avoid duplication.
  2. BOTTOM_DOCK_PADDING update (line 15)

    • Changed from Vector2i(32, 72) to Vector2i(48, 110).
    • Increases both horizontal and vertical padding for bottom dock.
  3. position_popup_panel fix (line 216)

    • Uses SIDEBAR_WIDTH constant instead of popup_size.x for positioning.
    • This is appropriate as it ensures consistent anchor positioning regardless of dynamic content sizing.
  4. dock_size_for_anchor update (lines 225-227)

    • Adds SIDEBAR_WIDTH + 16 to base.x for bottom mode.
    • The +16 appears to be a gap/padding value. This pattern matches WORLD_PANEL_PADDING but is not consolidated.

Standards Compliance

  • Tile-first dock sizing: The fix correctly derives the additional dock width from tile/UI chrome requirements rather than arbitrary sizing. ✓
  • Bottom dock primary mode: The fix supports bottom dock as the primary layout by ensuring popup accessibility. ✓
  • Popup as temporary overlay: The dock now accommodates the popup within its bounds, supporting the "shallow and wide" guideline. ✓

Minor Observations (Non-blocking)

  1. Duplicated sidebar width: The value 240 appears in two places (SIDEBAR_WIDTH constant and apply_anchor_layout() line 202). Future refactoring could consolidate these.

  2. Magic number +16: The gap value of 16 in dock_size_for_anchor is consistent with WORLD_PANEL_PADDING but could be a named constant for clarity.

Unknowns / Needs Verification

  • The fix assumes popup_size.x will not exceed SIDEBAR_WIDTH (240px). If the sidebar content or custom_minimum_size changes dynamically, positioning could become incorrect.
  • No test coverage visible for this specific edge case. Manual verification on affected platforms would confirm the fix.

Recommendation

Approve. The changes are targeted, logically consistent with the existing codebase patterns, and address the stated issue. The minor observations about value duplication and magic numbers are style concerns rather than functional defects.

@itsmiso-ai
itsmiso-ai force-pushed the fix/issue-54-popup-clip branch from 3a6ec58 to 4ca87a7 Compare April 18, 2026 13:31
@itsmiso-ai
itsmiso-ai merged commit df3b050 into main Apr 18, 2026
2 checks passed
@itsmiso-ai
itsmiso-ai deleted the fix/issue-54-popup-clip branch April 18, 2026 13:33
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.

Fix menu popup being clipped or inaccessible inside the dock

1 participant