Skip to content

fix: preserve locked aspect ratio within size constraints - #278

Open
sameerdeolalikar wants to merge 1 commit into
react-grid-layout:masterfrom
sameerdeolalikar:fix/aspect-ratio-size-constraints
Open

sameerdeolalikar wants to merge 1 commit into
react-grid-layout:masterfrom
sameerdeolalikar:fix/aspect-ratio-size-constraints

Conversation

@sameerdeolalikar

@sameerdeolalikar sameerdeolalikar commented Sep 26, 2026 •

Copy link
Copy Markdown

Closes #277

When lockAspectRatio is combined with independent size constraints, dragging a 300 × 150 component toward 600 × 300 with a maximum width of 420 produces 420 × 300. This patch intersects the width and height bounds on the aspect-ratio line, producing 420 × 210 and preserving the ratio through subsequent drag callbacks.

If no size can satisfy both the ratio and the constraints, existing size-constraint precedence is preserved. Unlocked resizing retains the existing clamp path. No dependency, generated-file or public API changes.

Validation:

  • New regression tests: five failures on unchanged master; all six pass with the fix.
  • Full Jest suite: 72 tests and 2 snapshots pass.
  • ESLint: no errors; one existing warning in ResizableBox.test.tsx.
  • Type-check, build and built-package smoke test pass.

The regression tests exercise bound overshoot, return and a subsequent gesture through the component's drag callback, following the existing test conventions. They are not a claim of browser/device coverage.

Implementation and regression tests were prepared with AI assistance. The checks above were executed against this branch.

Summary by CodeRabbit

  • Bug Fixes
    • Resizing with a locked aspect ratio now better respects minimum and maximum width and height constraints across successive drag movements, including when dimensions are adjusted beyond a limit and then returned.
    • When the configured bounds cannot be satisfied while preserving the aspect ratio, the bounds remain authoritative. Non-integer aspect ratios are also handled.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: e064d0cf-eaae-46f8-9db8-5ff80a4174f3

📥 Commits

Reviewing files that changed from the base of the PR and between c6fd4ab and 3eb7bf1.

📒 Files selected for processing (2)
  • __tests__/aspectConstraints.test.tsx
  • lib/Resizable.tsx

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Resizable now applies compatible minimum and maximum constraints along the locked aspect-ratio line. When the bounds cannot satisfy the ratio, it retains independent minimum-then-maximum clamping. Tests cover drag sequences and incompatible bounds.

Changes

Aspect-ratio constraints

Layer / File(s) Summary
Apply bounds to locked aspect ratios
lib/Resizable.tsx, __tests__/aspectConstraints.test.tsx
runConstraints clamps dimensions along the aspect-ratio line when compatible bounds exist. Otherwise, it retains independent clamping. Tests cover overshoot, return, subsequent drags, a non-integer ratio, and bounds that cannot satisfy the ratio.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 3eb7b

The change appears ready to merge after normal checks; no unresolved issue is identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 3eb7b

The change is confined to resize calculations and their tests. It improves bounded aspect-ratio behavior without an identified new security path, but downstream consumer behavior has not been verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The behavioral change can reach consumers of Resizable that enable aspect locking and supply size bounds; the evidence does not establish a wider security-sensitive downstream use.

Trust Boundaries and Controls

  • observed — Drag deltas and component props feed the constraint calculation before callback delivery. The inspected path shows a sizing boundary, not an authentication or privilege transition.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving the locked aspect ratio when size constraints apply.
Linked Issues check ✅ Passed The implementation addresses [#277]. runConstraints projects constrained dimensions onto the current width / height ratio, intersects width bounds derived from both dimensions, and derives height …
Out of Scope Changes check ✅ Passed The pull request changes only lib/Resizable.tsx and __tests__/aspectConstraints.test.tsx. The source change implements [#277], and the tests verify its required behavior. No unrelated dependency, …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

This branch has not been deployed

No deployments
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.

lockAspectRatio still breaks at independent min/max constraints

1 participant