Skip to content

Conversation

@guoyingtao
Copy link
Owner

@guoyingtao guoyingtao commented Jan 8, 2026

Fixes #486

Summary by CodeRabbit

  • Bug Fixes
    • Improved crop view animation behavior during rotation operations for smoother user experience.
    • Enhanced crop box confinement to work more reliably within image container boundaries.

✏️ Tip: You can customize this high-level summary in your review settings.

@guoyingtao
Copy link
Owner Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Jan 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link

coderabbitai bot commented Jan 8, 2026

📝 Walkthrough

Walkthrough

The changes address crop box resizing instability after 90-degree rotation by refactoring frame confinement logic in CropView and adding animation control to the image containment method.

Changes

Cohort / File(s) Summary
Demo Configuration
Example/DemoViewController.swift
Added commented-out line referencing rotateCropBoxFor90DegreeRotation configuration for testing purposes. No functional impact.
Core Crop View Logic
Sources/Mantis/CropView/CropView.swift
Modified updateCropBoxFrame(withTouchPoint:) to confine crop box frame directly within imageContainerRect instead of a computed imageFrame. Updated makeSureImageContainsCropOverlay() signature to accept optional animated: Bool parameter (default: true), delegating zoom behavior to cropWorkbenchView.zoomScaleToBound(animated:). Updated call sites to pass animated: false during rotation sequences.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hop and rotate, no more jumpy resize,
Frame confinement fixed before our eyes,
Animation flags to smooth the way,
Crop box glides through rotation's play!

🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning All changes are directly related to fixing the crop box jumping issue; the commented line in Example/DemoViewController.swift is demonstration code unrelated to the fix. Remove the commented-out configuration line in Example/DemoViewController.swift as it appears to be debugging code left in accidentally and is out of scope for this fix.
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 (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main fix: addressing a UI jumping issue when a specific configuration flag is set to false.
Linked Issues check ✅ Passed The changes fix the crop box resizing bug by updating coordinate confinement logic in updateCropBoxFrame() and adding animation control to makeSureImageContainsCropOverlay(), directly addressing the unstable interaction reported in issue #486.

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

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1cda2a8 and ee5c52f.

📒 Files selected for processing (2)
  • Example/DemoViewController.swift
  • Sources/Mantis/CropView/CropView.swift
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: guoyingtao
Repo: guoyingtao/Mantis PR: 462
File: Sources/Mantis/SwiftUIView/ImageCropper.swift:145-152
Timestamp: 2025-06-19T08:13:01.113Z
Learning: In the Mantis SwiftUI implementation, the user prefers to keep the CropAction enum minimal with only the .crop case active for now, rather than implementing all possible crop actions upfront.
📚 Learning: 2025-06-19T08:13:01.113Z
Learnt from: guoyingtao
Repo: guoyingtao/Mantis PR: 462
File: Sources/Mantis/SwiftUIView/ImageCropper.swift:145-152
Timestamp: 2025-06-19T08:13:01.113Z
Learning: In the Mantis SwiftUI implementation, the user prefers to keep the CropAction enum minimal with only the .crop case active for now, rather than implementing all possible crop actions upfront.

Applied to files:

  • Example/DemoViewController.swift
  • Sources/Mantis/CropView/CropView.swift
🧬 Code graph analysis (1)
Sources/Mantis/CropView/CropView.swift (4)
Sources/Mantis/CropView/ImageContainer.swift (1)
  • contains (41-50)
Tests/MantisTests/Mock/FakeImageContainer.swift (1)
  • contains (12-14)
Sources/Mantis/CropView/CropWorkbenchView.swift (1)
  • zoomScaleToBound (92-97)
Tests/MantisTests/Mock/FakeCropWorkbenchView.swift (1)
  • zoomScaleToBound (32-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Agent
🔇 Additional comments (4)
Example/DemoViewController.swift (1)

148-148: Good to keep this commented line for testing.

This commented configuration option is useful for reproducing and verifying the fix for issue #486. It aligns with the reproduction steps described in the linked issue.

Sources/Mantis/CropView/CropView.swift (3)

341-343: Excellent fix for the crop box jumping issue.

The change from using a computed imageFrame (based on cropWorkbenchView frame and offset) to using imageContainerRect directly addresses the root cause of issue #486. After 90-degree rotation with rotateCropBoxFor90DegreeRotation = false, the imageContainer.bounds remains accurate, ensuring touch points are confined to the correct rectangle and preventing the crop box from suddenly shrinking.


597-601: API enhancement with backward compatibility maintained.

The addition of the animated parameter allows fine-grained control over zoom animations when ensuring the image contains the crop overlay. The default value of true preserves the existing behavior for all current call sites (e.g., line 580), maintaining backward compatibility while enabling the fix for rotation-related visual glitches.


915-933: Proper animation control prevents visual glitches during rotation.

When rotateCropBoxFor90DegreeRotation is false, the crop box remains fixed while the image rotates. Calling makeSureImageContainsCropOverlay(animated: false) at line 932 ensures the image is adjusted to contain the crop overlay without introducing animation conflicts, which would cause the jumping behavior reported in issue #486. The disabled animation works correctly within the existing UIView.animate block at line 943.


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

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a UI jumping issue that occurs during 90-degree rotation when the rotateCropBoxFor90DegreeRotation configuration is set to false. The fix simplifies coordinate calculations and ensures proper image containment without animation during rotation.

  • Simplified touch point confinement logic by removing redundant frame and content offset calculations
  • Added animation control to the makeSureImageContainsCropOverlay method with backward-compatible default
  • Ensured the crop overlay is properly contained after rotation adjustments when not rotating the crop box

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
Sources/Mantis/CropView/CropView.swift Simplified touch point calculation, added animation control parameter, and fixed rotation jump by ensuring image contains crop overlay without animation
Example/DemoViewController.swift Added commented-out configuration line for testing the rotateCropBoxFor90DegreeRotation = false scenario

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.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.

rotateCropBoxFor90DegreeRotation = false causes abnormal crop box resizing after rotation

1 participant