Skip to content

fix: keep agent modal inside viewport#2099

Merged
yottahmd merged 1 commit intomainfrom
codex/fix-agent-modal-zoom-bounds
May 6, 2026
Merged

fix: keep agent modal inside viewport#2099
yottahmd merged 1 commit intomainfrom
codex/fix-agent-modal-zoom-bounds

Conversation

@yottahmd
Copy link
Copy Markdown
Collaborator

@yottahmd yottahmd commented May 6, 2026

Summary

  • clamp restored and live-resized agent modal bounds against the actual viewport size
  • re-clamp draggable windows on resize and orientation changes so browser zoom keeps controls reachable
  • add regression tests for stale stored bounds and viewport shrink behavior

Root cause

Saved right and bottom offsets were clamped against the minimum modal size instead of the clamped actual modal size. A browser zoom or viewport shrink could therefore leave the fixed-position modal with a negative top or left offset, and already-open modals were not re-clamped on resize.

Testing

  • pnpm vitest run src/features/agent/hooks/__tests__/useResizableDraggable.test.ts src/features/agent/components/__tests__/AgentChatModal.test.tsx
  • pnpm typecheck
  • pnpm exec eslint --ext .ts,.tsx src/features/agent/hooks/useResizableDraggable.ts src/features/agent/hooks/__tests__/useResizableDraggable.test.ts
  • pnpm test
  • pnpm build
  • Playwright browser geometry check at 1100x760, then 900x520, with mocked API responses

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Draggable and resizable elements now remain within the visible viewport
    • Elements automatically adjust position when the window is resized or orientation changes
    • Improved persistence of element positions
  • Tests

    • Added test coverage for draggable and resizable functionality

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2f73fcd2-b8b8-41d3-b28c-ed7a21b0e2c0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

A new test suite validates viewport bounds clamping in the useResizableDraggable hook, which now enforces that draggable/resizable elements stay within the viewport, re-clamps on window resize/orientation changes, and validates stored bounds from localStorage with strict numeric checks and fallback behavior.

Changes

Viewport Bounds Clamping & Responsive Resizing

Layer / File(s) Summary
Utilities & Constants
ui/src/features/agent/hooks/useResizableDraggable.ts
Added viewport padding constants (VIEWPORT_HORIZONTAL_PADDING, VIEWPORT_VERTICAL_PADDING) and helper functions: finiteNumber() for fallback validation, clamp() for range enforcement, clampBounds() to enforce min/max sizes and viewport constraints, and areBoundsEqual() for equality checks.
Bounds Initialization
ui/src/features/agent/hooks/useResizableDraggable.ts
Loading stored bounds from localStorage now uses finiteNumber() for strict numeric validation and clampBounds() to ensure loaded values fit within the viewport; gracefully falls back to clamped defaults on errors.
Drag & Resize Logic
ui/src/features/agent/hooks/useResizableDraggable.ts
Dragging now computes viewport-aware maxWidth/maxHeight using padding constants and applies clampBounds() to enforce minimums and viewport constraints. Resizing logic updated similarly to clamp newWidth/newHeight/newRight/newBottom within viewport bounds.
Window Resize Watcher
ui/src/features/agent/hooks/useResizableDraggable.ts
New effect listens for resize and orientationchange events to re-clamp bounds when viewport changes, preventing off-screen panels; uses areBoundsEqual() to avoid redundant updates.
Refactored Callbacks
ui/src/features/agent/hooks/useResizableDraggable.ts
startDrag refactored with explicit useCallback dependency ([bounds]); saveBounds updated with clearer multi-line useCallback and dependency array ([storageKey]).
Tests & Validation
ui/src/features/agent/hooks/__tests__/useResizableDraggable.test.ts
New test suite with viewport simulation helpers; validates that stored bounds are clamped within the current viewport and re-clamped when viewport dimensions change (zoom/orientation). Includes proper setup (localStorage snapshot, window dimensions capture) and cleanup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 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 'fix: keep agent modal inside viewport' directly summarizes the main change: preventing the agent modal from moving outside the viewport bounds.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-agent-modal-zoom-bounds

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ui/src/features/agent/hooks/useResizableDraggable.ts (1)

1-1: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add the required GPL header before the imports.

This changed TypeScript file still starts directly with imports, so it misses the repository’s required license banner. Please run make addlicense on the touched file before merge.

As per coding guidelines, **/*.{go,ts,tsx,js}: Apply GPL v3 license headers on source files, managed via make addlicense.

🤖 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 `@ui/src/features/agent/hooks/useResizableDraggable.ts` at line 1, This file is
missing the required GPL v3 license header; run the repository tool to add it by
executing `make addlicense` for the modified file
(ui/src/features/agent/hooks/useResizableDraggable.ts) before merging so the GPL
banner is inserted above the imports; ensure the header is applied to the module
that contains the useResizableDraggable hook and its imports so the file
complies with the repository's licensing policy.
🤖 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 `@ui/src/features/agent/hooks/__tests__/useResizableDraggable.test.ts`:
- Around line 1-2: Add the GPL v3 license banner to this test file by running
the repository license tool: run make addlicense to apply the GPL header to the
TypeScript test (ensure the header appears at the very top of
useResizableDraggable.test.ts before the import lines like "import { act,
renderHook }" and "import { afterEach, describe, expect, it }"), then commit the
updated file so it matches the repository rule for **/*.{go,ts,tsx,js}.

---

Outside diff comments:
In `@ui/src/features/agent/hooks/useResizableDraggable.ts`:
- Line 1: This file is missing the required GPL v3 license header; run the
repository tool to add it by executing `make addlicense` for the modified file
(ui/src/features/agent/hooks/useResizableDraggable.ts) before merging so the GPL
banner is inserted above the imports; ensure the header is applied to the module
that contains the useResizableDraggable hook and its imports so the file
complies with the repository's licensing policy.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e821d2b3-ccb1-4846-a7e6-211253ef83d6

📥 Commits

Reviewing files that changed from the base of the PR and between 0f68cd3 and b460a12.

📒 Files selected for processing (2)
  • ui/src/features/agent/hooks/__tests__/useResizableDraggable.test.ts
  • ui/src/features/agent/hooks/useResizableDraggable.ts

Comment thread ui/src/features/agent/hooks/__tests__/useResizableDraggable.test.ts
@yottahmd yottahmd force-pushed the codex/fix-agent-modal-zoom-bounds branch from b460a12 to f709882 Compare May 6, 2026 08:51
@yottahmd yottahmd merged commit 928231d into main May 6, 2026
9 checks passed
@yottahmd yottahmd deleted the codex/fix-agent-modal-zoom-bounds branch May 6, 2026 09:23
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.

1 participant