feat: add maxZoom support for iOS and Android #326
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an optional maxZoom parameter in Dart, propagates it via creation params, and conditionally applies it in native views: Android multiplies by 2 and calls setMaxZoom, iOS sets maxScaleFactor only when maxZoom is an NSNumber. Changes
Sequence Diagram(s)sequenceDiagram
participant W as Flutter Widget (PDFView)
participant P as Platform Channel
participant A as Android PDFView
participant I as iOS PDFView
rect rgba(230,245,255,0.6)
Note over W: Widget built with maxZoom
W->>P: send creation params { maxZoom, ... }
end
alt Android path
P->>A: onCreate(params)
A->>A: if maxZoom present\nsetMaxZoom(maxZoom * 2)
else iOS path
P->>I: onCreate(params)
I->>I: if maxZoom is NSNumber\nset maxScaleFactor
end
Note over A,I: Other configuration steps unchanged
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/flutter_pdfview.dart (1)
42-45: Clarify API semantics: “relative to fit” and creation‑time only.Document that maxZoom is a multiple of the fit-to-screen scale and is applied at creation time (not via updates). This matches the intended cross‑platform behavior and prevents confusion.
Apply this doc update:
- /// Use to change the maximal zoom. Must be greater than 1.0, default 3.0. + /// Max zoom multiple relative to the fit-to-screen scale (iOS and Android). + /// Must be > 1.0. Default is 3.0. Note: Currently applied only at creation time.Also applies to: 125-127, 245-246, 261-262, 277-278, 292-292
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java(1 hunks)ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m(1 hunks)lib/flutter_pdfview.dart(6 hunks)
🔇 Additional comments (1)
lib/flutter_pdfview.dart (1)
42-45: LGTM: API surface and guard.Adding maxZoom with a >1.0 assert is a good safety net.
| Object maxZoom = params.get("maxZoom"); | ||
| if (maxZoom != null) { | ||
| float max = ((Number) maxZoom).floatValue(); | ||
| pdfView.setMaxZoom(max * 2); | ||
| } | ||
|
|
There was a problem hiding this comment.
Compute Android maxZoom relative to the initial fit; avoid arbitrary x2.
Multiplying by 2 makes semantics unclear and diverges from iOS (“relative to fit”). Align by setting maxZoom after first render based on the initial fit zoom.
Apply this diff within the shown lines to capture the requested multiple (don’t set max yet):
- Object maxZoom = params.get("maxZoom");
- if (maxZoom != null) {
- float max = ((Number) maxZoom).floatValue();
- pdfView.setMaxZoom(max * 2);
- }
+ final Object maxZoomObj = params.get("maxZoom");
+ final Float maxZoomMultiple =
+ (maxZoomObj instanceof Number) ? ((Number) maxZoomObj).floatValue() : null;Then, set the absolute max after the first render (relative to the fit zoom). Insert the following inside onInitiallyRendered(...) right after invoking the "onRender" channel method:
// After: methodChannel.invokeMethod("onRender", args);
if (maxZoomMultiple != null) {
float fitZoom = pdfView.getZoom(); // zoom at initial render ~= fit scale
pdfView.setMaxZoom(fitZoom * maxZoomMultiple);
// Optional: clamp current zoom if needed (if your PDFView version exposes helpers).
}🤖 Prompt for AI Agents
In android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
around lines 57-62, the code currently multiplies the provided maxZoom by 2
immediately which is arbitrary and inconsistent with iOS; instead, capture the
provided maxZoom as a multiple (e.g. store it in a Float field named
maxZoomMultiple) but do NOT call pdfView.setMaxZoom() at these lines; then,
inside onInitiallyRendered(...) immediately after invoking the "onRender"
channel method, if maxZoomMultiple is not null read the initial fit zoom from
pdfView.getZoom() and call pdfView.setMaxZoom(fitZoom * maxZoomMultiple)
(optionally clamp the current zoom if your PDFView supports it).
| NSNumber *maxZoom = args[@"maxZoom"]; | ||
| if (maxZoom != nil && [maxZoom isKindOfClass:[NSNumber class]]) { | ||
| _pdfView.maxScaleFactor = [maxZoom doubleValue]; | ||
| } | ||
|
|
There was a problem hiding this comment.
iOS: maxZoom should be relative to fit and not get overwritten on layout; clamp double‑tap to max.
- Current code sets maxScaleFactor to the raw number, then layout sets it back to 4.0, breaking the new API.
- Implement “relative to fit” semantics consistently and clamp double‑tap to respect max.
Apply this diff in the changed block to compute max relative to fit and remember the multiple:
- NSNumber *maxZoom = args[@"maxZoom"];
- if (maxZoom != nil && [maxZoom isKindOfClass:[NSNumber class]]) {
- _pdfView.maxScaleFactor = [maxZoom doubleValue];
- }
+ NSNumber *maxZoom = args[@"maxZoom"];
+ if ([maxZoom isKindOfClass:[NSNumber class]]) {
+ _maxZoomMultiple = MAX(1.0, [maxZoom doubleValue]); // store the multiple
+ _pdfView.maxScaleFactor = _pdfView.scaleFactorForSizeToFit * _maxZoomMultiple;
+ }Add an ivar and a sensible default (outside the changed lines):
// Near other ivars
double _maxZoomMultiple;
// In -initWithFrame:... before using it
_maxZoomMultiple = 3.0;Update layout to recompute absolute max from the current fit scale and clamp the current zoom (outside the changed lines):
// In -layoutSubviews (replace the hard-coded 4.0)
_pdfView.maxScaleFactor = _pdfView.scaleFactorForSizeToFit * (_maxZoomMultiple > 1.0 ? _maxZoomMultiple : 3.0);
if (_pdfView.scaleFactor > _pdfView.maxScaleFactor) {
_pdfView.scaleFactor = _pdfView.maxScaleFactor;
}Respect max on double‑tap zoom (outside the changed lines):
// Where scaleFactor is set to scaleFactorForSizeToFit * 2
CGFloat target = MIN(self->_pdfView.scaleFactorForSizeToFit * 2.0, self->_pdfView.maxScaleFactor);
self->_pdfView.scaleFactor = target;Also applies to: 257-264, 341-348
🤖 Prompt for AI Agents
In ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m around lines
168-172, the code currently assigns maxScaleFactor directly from the raw maxZoom
number which gets overwritten on layout; change it to store a relative multiple
(add a double _maxZoomMultiple ivar near other ivars and initialize it to 3.0 in
initWithFrame:), when handling args compute and save the multiple (e.g.
_maxZoomMultiple = maxZoomValue; do not set absolute max here), update
layoutSubviews to recompute the absolute max as scaleFactorForSizeToFit *
(_maxZoomMultiple > 1.0 ? _maxZoomMultiple : 3.0) and clamp current scaleFactor
to that max, and update the double-tap zoom logic to cap the target zoom with
the recomputed maxScaleFactor so double-tap respects the configured max; apply
the same pattern to the other mentioned blocks (lines 257-264, 341-348).
|
@BugBot review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if (maxZoom != null) { | ||
| float max = ((Number) maxZoom).floatValue(); | ||
| pdfView.setMaxZoom(max * 2); | ||
| } |
There was a problem hiding this comment.
Bug: Android Zoom Bug Causes Inconsistency
Android's maxZoom parameter is multiplied by 2, leading to inconsistent maximum zoom levels compared to iOS. The implementation also casts maxZoom to Number without a type check, which could cause a ClassCastException at runtime.
There was a problem hiding this comment.
Note on Android maxZoom implementation
On Android, the underlying PDFView (from android-pdf-viewer) uses a different internal scaling model compared to iOS PDFKit.
If we pass the same numeric value for maxZoom directly, the perceived zoom level on Android will look significantly smaller than on iOS.
To make the behavior consistent across platforms, the Android implementation multiplies maxZoom by 2.
This way, when developers pass the same maxZoom value in Dart, the visual zoom level on Android and iOS appears similar.
Added an inline comment in FlutterPDFView.java to explain that Android's PDFView uses a different zoom scale than iOS PDFKit. Multiplying by 2 makes zoom levels more visually consistent across platforms.
2 participants
What I did
maxZoomparameter to control the maximum zoom level in PDFViewPDFView.maxScaleFactor(relative toscaleFactorForSizeToFit)setMaxZoom()Why
Currently, flutter_pdfview does not provide a way to limit zoom scale.
This feature gives developers better control over user experience and performance.
Related Issues
Notes
maxZoom= 3.0 (relative to fit scale on iOS)maxZoomparameter will not breakSummary by CodeRabbit
New Features
Bug Fixes