Adding features to repo #333
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# This is the 1st commit message: update example's gradle, and compileSdkVersion remove enableR8 downgrade to beta.2, upgrade gradle to 7.3.0 release 1.2.7 remove unused fitEachPage prop add documentation release 1.2.8 Close endigo#230, release 1.2.9 chore: upgrade android-pdf-viewer to 3.2.0-beta.3 chore: bump version to 1.3.0 upgrade compileSdkVersion to 33 upgrade sdk version, release 1.3.1 update changelog use new api on test cleanup and check for plugin connection * Upgraded Gradle * Added: getCurrentPageSize() getViewSize() getCurrentViewportPosition() * Small formatting changes * Added: setScaleAndPosition() * Improved: setScaleAndPosition() * Added: getScreenshot() method * Added: onLoadComplete() callback * Added: onDraw method * renamed some methods to be more sensible * changed getCurrentPageSize() to return a map of width and height * removed redundant getViewSize() * renamed getCurrentViewportPosition() to getVpPositionAndScale() to reflect that it is returning scale as well, and now returns a Map() * renamed setScaleAndPosition() to more consistent setVpPositionAndScale() and now passes in Offset * added reload() method upgrade manifest and gradle to match endigo release upgrade manifest and gradle to match endigo release * Upgraded Gradle * Added: getCurrentPageSize() getViewSize() getCurrentViewportPosition() * Small formatting changes * Added: setScaleAndPosition() # This is the commit message #2: * Added: getScreenshot() method * Added: onLoadComplete() callback * Added: onDraw method * renamed some methods to be more sensible * changed getCurrentPageSize() to return a map of width and height * removed redundant getViewSize() * renamed getCurrentViewportPosition() to getVpPositionAndScale() to reflect that it is returning scale as well, and now returns a Map() * renamed setScaleAndPosition() to more consistent setVpPositionAndScale() and now passes in Offset * added reload() method upgrade manifest and gradle to match endigo release upgrade manifest and gradle to match endigo release - added safe unboxing Update pubspec.yaml Update CHANGELOG.md upgrade pubspec upgraded gradle plugin - added zoom limit setter for min, mid and max zoom levels - added zoom limit setter for min, mid and max zoom levels - added completers for get and set VP position to avoid contention and race conditions - upgraded AndroidPdfViewer to 3.2.4 - upgraded gradle - added iOS changes for displaying pdfs Update README.md Update README.md hotfix: Crash on iOS18 update deps on example Update README.md Update README.md Update README.md Update FUNDING.yml Update README.md add background color param Close endigo#289 release 1.3.4 release 1.3.4 pubspec.lock update - added iOS changes for displaying pdfs Build 1.3.5 - separated get/set position and get/set scale - fixed get/set position for iOS taking into account multi-page pdfs. Build 1.3.6 - removed header references to Chromium Authors copyright - updated .gitignore to remove .podspec and .sh files that should not be there. - in iOS, moved population of _currentPage and _pageNo to the layout phase and on page changed - it was running on every getPosition and setPosition. Not optimal. Build 1.3.7 - made more sensible type-safe returns using inbuilt classes instead of Maps. - removed deprecated Color.value Build 1.3.7 - made more sensible type-safe returns using inbuilt classes instead of Maps. - removed deprecated Color.value Close endigo#293 gradle upgrade update example to use SPM update example project to SPM pubspec.lock update add unit tests add error handling on backgroundColor remove default white background in iOS
* Added: onDraw() callback * Added: getCurrentPageSize() * Added: getScreenshot(String) * Added: getPosition() & setPosition(Offset) * Added: getScale() & setScale(double) * Added reload() * Added: setZoomLimits() * added safe unboxing * added completers for get and set VP position to avoid contention and race conditions * upgraded AndroidPdfViewer to 3.2.4 * fixed deprecated color .value
WalkthroughThis PR adds rendering options, lifecycle callbacks, controller methods (position/scale, screenshot, reload, zoom limits), and backgroundColor across Dart, Android, and iOS; updates build tooling, minimum platform versions, example app (iPad safe mode), and adds docs (CLAUDE.md). No breaking API removals. Changes
Sequence Diagram(s)sequenceDiagram
participant Flutter as Flutter UI
participant Controller as PDFViewController (Dart)
participant Channel as MethodChannel
participant Native as Native PDFView (Android / iOS)
participant FS as FileSystem / Renderer
Flutter->>Controller: user triggers screenshot / setScale / getPosition / open PDF
Controller->>Channel: invokeMethod("getScreenshot"/"setScale"/...)
Channel->>Native: deliver method call
Native->>Native: apply settings / update view / render
Native->>FS: write screenshot file (getScreenshot)
FS-->>Native: return file path / error
Native-->>Channel: method result / callback (onLoadComplete/onDraw)
Channel-->>Controller: result / callback event
Controller-->>Flutter: invoke widget callbacks (onLoadComplete/onDraw) / return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
example/ios/Flutter/flutter_export_environment.sh (1)
1-13: This generated file should not be checked into version control.The file header explicitly states "do not edit or check into version control." This file contains machine-specific paths (
/Users/jasonelder/...) that will break the build for other developers.Add this file to
.gitignoreand remove it from the repository:example/ios/Flutter/flutter_export_environment.shREADME.md (1)
58-60: Duplicatepasswordentry in options table.The
passwordoption is listed twice (lines 58 and 60). Remove the duplicate to keep the documentation clean.📝 Proposed fix
| nightMode | ✅ | ❌ | `false` | -| password | ✅ | ✅ | `null` | | autoSpacing | ✅ | ✅ | `true` |
🤖 Fix all issues with AI agents
In `@android/build.gradle`:
- Line 15: Update the Android Gradle configuration to use the required SDK
version by changing the compileSdk setting from 36 to 35; locate the compileSdk
declaration (the compileSdk property in android/build.gradle) and set its value
to 35 so it matches project guidelines.
In `@android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java`:
- Around line 164-166: The onMethodCall handler currently calls
setZoomLimits(methodCall) without passing the Flutter Result callback, causing
the Dart Future to never complete; update the switch branch ("setZoomLimits") to
call setZoomLimits(methodCall, result) and modify the setZoomLimits method
signature to accept a Result parameter (e.g., setZoomLimits(MethodCall call,
Result result)), ensure it validates inputs, invokes result.success(...) on
success or result.error(...) on failure, and mirror the same change for the
other occurrence referenced (the alternate call site around the 234-241 block)
so every invocation supplies and the method uses the Result callback.
- Around line 243-261: In getScreenshot, remove the unsafe assert on pdfFileName
and instead explicitly check pdfFileName for null and return a result.error when
null; ensure you don't call substring if pdfFileName is null. Also replace the
always-true Objects.equals(imageFileName, "") check with a real success
condition—e.g., verify bmp from loadBitmapFromPDFView() is not null and that the
image file was written or exists before calling result.success(imageFileName);
use the existing symbols pdfFileName, imageFileName, loadBitmapFromPDFView, bmp
and return appropriate result.error messages on failure.
- Around line 272-275: The reload(Result result) method can NPE if the instance
field configurator is null; change reload in FlutterPDFView to null-check
configurator before calling configurator.load() and return an appropriate Result
response instead of calling through on null (e.g., call
result.error("NO_CONFIGURATOR", "Configurator is null; cannot reload", null) or
result.success(false)); ensure you reference the configurator field and the
reload(Result) method when implementing the guard so code never invokes
configurator.load() when configurator == null.
In `@CHANGELOG.md`:
- Line 25: Update the CHANGELOG entry that currently reads "Revert `.toARGB32()`
to depricated `.value`" by correcting the typo to "deprecated" so it reads
"Revert `.toARGB32()` to deprecated `.value`", ensuring the wording and
punctuation around the `.toARGB32()` and `.value` tokens remain unchanged.
- Around line 49-51: Remove the orphan/incomplete bullet point in CHANGELOG.md:
delete the extraneous standalone dash that follows the "build.gradle namespace
[`#253`] ..." entry (the empty bullet shown in the diff) so the changelog has no
empty list items and formatting stays consistent.
In `@CLAUDE.md`:
- Around line 92-102: Update the documented platform/version values in CLAUDE.md
to match the actual project settings: change the iOS minimum version entry from
11.0 to the value declared in Package.swift (iOS 12.0) and update the Gradle
build tools version from 8.1.0 to the Gradle version referenced in CHANGELOG.md
(8.14.3), ensuring the text under the "iOS" and "Android" sections reflects
those exact values so docs and config stay consistent.
In `@example/android/settings.gradle`:
- Around line 1-27: Remove the duplicated pluginManagement block and ensure
flutterSdkPath is obtained exactly once as a String; consolidate into a single
pluginManagement { ... } block where flutterSdkPath is defined by invoking the
closure only once (e.g., assign settings.ext.flutterSdkPath = flutterSdkPath),
then use that String for
includeBuild("${settings.ext.flutterSdkPath}/packages/flutter_tools/gradle") and
the repositories block; specifically edit the duplicate top-level
pluginManagement and the redundant flutterSdkPath() invocation so flutterSdkPath
is not treated as a closure after being invoked.
In `@example/ios/Flutter/ephemeral/flutter_lldb_helper.py`:
- Around line 1-32: This generated LLDB helper (contains functions
handle_new_rx_page and __lldb_init_module in the ephemeral directory) should not
be committed; remove this file from the commit/PR and ensure generated ephemeral
files are ignored: add the ephemeral/ pattern (or ephemeral/*.py) to .gitignore,
and if the file is already tracked run git rm --cached on it before committing
so it remains out of version control. Ensure any CI or repo hooks that
regenerate ephemeral files don't accidentally re-add them to commits.
In
`@example/ios/Flutter/ephemeral/Packages/FlutterGeneratedPluginSwiftPackage/Package.swift`:
- Around line 17-19: The Package.swift in the generated ephemeral bundle
includes a hardcoded absolute path in the dependencies array (.package(name:
"flutter_pdfview", path: "/Users/jasonelder/.../flutter_pdfview")), which will
break other developers' builds; remove the committed generated file from source
control (or revert the change), ensure example/ios/Flutter/ephemeral/ is added
to .gitignore so generated files are not committed, and if you need a local
dependency for testing, replace the absolute path with a relative path or use a
proper package reference in the non-generated Package.swift (locate the .package
entry for "flutter_pdfview" to update or remove).
In
`@example/ios/Flutter/ephemeral/Packages/FlutterGeneratedPluginSwiftPackage/Sources/FlutterGeneratedPluginSwiftPackage/FlutterGeneratedPluginSwiftPackage.swift`:
- Around line 1-3: The generated ephemeral Flutter file
FlutterGeneratedPluginSwiftPackage.swift under the ephemeral directory should
not be committed; remove it from the repo and ensure ephemeral artifacts are
ignored by adding the ephemeral/ path (or the specific generated file pattern)
to .gitignore, then run git rm --cached on the committed file(s) and commit the
.gitignore change so future builds regenerate the file locally without being
tracked.
In `@example/lib/main.dart`:
- Line 250: The print uses "page ?? 0 + 1" which due to operator precedence
evaluates as page ?? (0 + 1) and yields inconsistent 0-based vs 1-based
numbering; update the print expression so the null-coalescing applies before
adding (e.g. change "page ?? 0 + 1" to "(page ?? 0) + 1") in the print statement
that references page and total to ensure correct 1-based page display.
In `@ios/flutter_pdfview/Package.swift`:
- Around line 8-10: The platforms entry in Package.swift currently specifies
.iOS("12.0") while the README claims iOS 11.0+ support; reconcile them by either
lowering the Package.swift platforms: [.iOS("12.0")] value to .iOS("11.0") to
match README or update the README to state iOS 12.0+ support; modify the
Package.swift platforms entry (.iOS("12.0")) or the README text accordingly and
ensure any other platform/version mentions in the repo are consistent with the
chosen minimum iOS version.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m`:
- Around line 91-92: The iOS handler never completes the Dart Future because
setZoomLimits currently doesn't accept or call the FlutterResult callback;
update the method signature to setZoomLimits:(FlutterMethodCall*)call
result:(FlutterResult)result (or add a second result parameter) and ensure it
invokes result(nil) on success or result([FlutterError errorWith...]) on
failure; then update the onMethodCall:result: branch that checks
isEqualToString:@"setZoomLimits" to call [_pdfView setZoomLimits:call
result:result] (and do the same for the duplicate setZoomLimits call around the
415-423 region) so every native path returns a result to Dart.
- Around line 342-345: The dispatch_async block captures weakSelf but
incorrectly reads self->_pageCount, risking a crash if self is deallocated;
change the block to safely strongify weakSelf (e.g. __strong typeof(weakSelf)
strongSelf = weakSelf), check strongSelf is non-nil, then call [strongSelf
handleRenderCompleted:strongSelf->_pageCount] (or otherwise read a local
pageCount from strongSelf before calling) so the async block never dereferences
deallocated self; update the block around handleRenderCompleted to use
strongSelf and its _pageCount instead of self.
- Around line 271-288: The dealloc currently calls stopObserving which
unconditionally removes a KVO observer and can crash if _scrollView was never
observed; add a BOOL ivar/property (e.g., _isObserving) and set it true in
startObserving immediately after addObserver succeeds, check _isObserving before
calling removeObserver in stopObserving and set it false after removal, and
ensure dealloc calls stopObserving as before—update startObserving,
stopObserving, and dealloc to use this flag to avoid removing an observer that
was never registered.
- Around line 263-268: The initializer that creates _pdfView has an extra
closing brace causing the observer registration (handlePageChanged:,
PDFViewPageChangedNotification) and [self addSubview:_pdfView] to be outside the
intended dispatch block; open the initializer that constructs _pdfView (the init
method where you call dispatch_async and create _pdfView), remove the stray
closing brace so the addObserver: and addSubview:_pdfView calls remain inside
the correct scope, and then re-audit matching braces in that initializer to
ensure opening and closing braces balance (112/112).
In `@lib/flutter_pdfview.dart`:
- Around line 390-393: The onDraw handler in the event switch (case 'onDraw')
force-unwraps widget.onDraw with widget.onDraw!(), which will crash when the
callback is not provided; change this to a null-safe invocation (use the same
pattern as other callbacks, e.g., widget.onDraw?.call()) so the event is ignored
when onDraw is null and keep returning null as before.
In `@README.md`:
- Around line 17-18: Update the README dependency example where it currently
lists "flutter_pdfview: 1.3.2" to match the project's current pubspec release by
changing it to "flutter_pdfview: 1.4.4"; locate the string "flutter_pdfview:
1.3.2" in the README and replace it with "flutter_pdfview: 1.4.4".
🧹 Nitpick comments (13)
example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme (1)
71-71: Consider the performance impact of GPU validation.
enableGPUValidationMode = "1"enables Metal GPU validation during debugging. While useful for catching GPU-related issues (potentially relevant for PDF rendering), it adds overhead during debug sessions. This is fine for development but ensure it doesn't mask performance issues during testing.example/android/gradle.properties (1)
4-5: Consider opting into non-transitive R/non-final IDs when ready.
If the example is compatible, enabling these flags can help catch resource coupling issues earlier and align with newer AGP behavior.example/ios/Podfile (1)
34-36: Commented-out RunnerTests target is acceptable.If unit tests aren't configured for the example iOS app, keeping this commented out is fine. Consider removing the commented block entirely to reduce noise, or uncommenting it when tests are added.
example/android/build.gradle (1)
16-23: Consider removing the commented resolutionStrategy block.It’s dead code now; trimming it would reduce noise.
♻️ Optional cleanup
-// configurations.all { -// resolutionStrategy { -// force 'androidx.core:core-ktx:1.6.0' -// } -// }android/settings.gradle (1)
1-9: Flutter SDK resolution will fail iflocal.propertiesis missing.The closure reads
local.propertieswithout checking if the file exists first. If a developer clones the repo without runningflutter pub getor creatinglocal.properties, the build will fail with an unclear error.Consider adding a file existence check for a more informative error message, though this is standard Flutter plugin boilerplate.
example/android/app/build.gradle (2)
25-25:compileSdkis set to 34, but coding guidelines specify 35.The coding guidelines state to use compile SDK 35. Consider updating for consistency across the project.
- compileSdk 34 + compileSdk 35
56-58: Remove commented-out test dependencies.Commented-out code adds noise. If these dependencies are not needed, remove them entirely. If they may be needed later, document why in a comment or track in an issue.
Suggested fix
dependencies { - // testImplementation 'junit:junit:4.12' - // androidTestImplementation 'androidx.test.ext:junit:1.1.1' - // androidTestImplementation 'androidx.test.espresso:espresso-core:3.1.0' }test/flutter_pdfview_test.dart (1)
49-139: Consider asserting the new callbacks.
These widget tests mostly verify build success; adding explicit expectations foronViewCreated,onRender, and the newly addedonLoadComplete/onDrawcallbacks would better exercise the new API surface. As per coding guidelines, please consider expanding test coverage for new features.example/lib/main.dart (1)
210-224: RedundantenableSwipeternary can be simplified.
widget.isIPadSafe ? true : trueis always true; consider simplifying unless the intent was to toggle it.♻️ Proposed simplification
- enableSwipe: widget.isIPadSafe ? true : true, + enableSwipe: true,android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java (2)
281-295: Redundant null check insetPage.The null check on line 282 (
call.argument("page") != null) makes the subsequent null check on lines 285-290 unnecessary.♻️ Simplified implementation
void setPage(MethodCall call, Result result) { - if (call.argument("page") != null) { - Integer pageObj = call.argument("page"); - int page; - if (pageObj != null) { - page = pageObj; // Safe unboxing - } else { - // Handle null case, e.g., assign a default value - page = 1; - } + Integer pageObj = call.argument("page"); + if (pageObj != null) { + int page = pageObj; pdfView.jumpTo(page); } - result.success(true); }
333-342:getBooleanhelper can be simplified.The current logic is correct but overly verbose. The
containsKeycheck on line 341 is redundant sinceparams.get(key)already returnsnullfor missing keys.♻️ Simplified implementation
-private boolean getBoolean(Map<String, Object> params, String key) { - Boolean keyObj = (Boolean) params.get(key); - boolean bKey; - if (keyObj != null) { - bKey = keyObj; - } else { - bKey = false; - } - return params.containsKey(key) && bKey; +private boolean getBoolean(Map<String, Object> params, String key) { + Boolean value = (Boolean) params.get(key); + return value != null && value; }lib/flutter_pdfview.dart (2)
493-500: ReturningnullinFuture<void>method is unconventional.Line 496 returns
nullin a method declared to returnFuture<void>. While technically valid, it's cleaner to use an empty return.♻️ Proposed fix
Future<void> _updateSettings(_PDFViewSettings setting) async { final Map<String, dynamic> updateMap = _settings.updatesMap(setting); if (updateMap.isEmpty) { - return null; + return; } _settings = setting; return _channel.invokeMethod('updateSettings', updateMap); }
403-406: Consider adding error handling for platform method calls.The new controller methods like
getCurrentPageSize,getPosition, andgetScaledon't handle potentialPlatformExceptionerrors. If the native side returns an error or unexpected format, the app may crash.♻️ Example with error handling
Future<Size> getCurrentPageSize() async { - return _channel - .invokeMethod('currentPageSize') - .then((pageSize) => Size(pageSize[0] ?? 0, pageSize[1] ?? 0)); + try { + final pageSize = await _channel.invokeMethod('currentPageSize'); + return Size( + (pageSize[0] as num?)?.toDouble() ?? 0, + (pageSize[1] as num?)?.toDouble() ?? 0, + ); + } on PlatformException catch (e) { + debugPrint('Failed to get page size: ${e.message}'); + return Size.zero; + } }
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
Comment on lines
+342
to
+345
| __weak __typeof__(self) weakSelf = self; | ||
| dispatch_async(dispatch_get_main_queue(), ^{ | ||
| [weakSelf handleRenderCompleted:self->_pageCount]; | ||
| }); |
There was a problem hiding this comment.
Unsafe self capture in async block may cause crash.
Line 344 captures weakSelf but then accesses self->_pageCount directly. If self is deallocated before the block executes, this will crash.
🔧 Proposed fix
__weak __typeof__(self) weakSelf = self;
dispatch_async(dispatch_get_main_queue(), ^{
- [weakSelf handleRenderCompleted:self->_pageCount];
+ __strong __typeof__(weakSelf) strongSelf = weakSelf;
+ if (strongSelf) {
+ [strongSelf handleRenderCompleted:strongSelf->_pageCount];
+ }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| __weak __typeof__(self) weakSelf = self; | |
| dispatch_async(dispatch_get_main_queue(), ^{ | |
| [weakSelf handleRenderCompleted:self->_pageCount]; | |
| }); | |
| __weak __typeof__(self) weakSelf = self; | |
| dispatch_async(dispatch_get_main_queue(), ^{ | |
| __strong __typeof__(weakSelf) strongSelf = weakSelf; | |
| if (strongSelf) { | |
| [strongSelf handleRenderCompleted:strongSelf->_pageCount]; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m` around lines
342 - 345, The dispatch_async block captures weakSelf but incorrectly reads
self->_pageCount, risking a crash if self is deallocated; change the block to
safely strongify weakSelf (e.g. __strong typeof(weakSelf) strongSelf =
weakSelf), check strongSelf is non-nil, then call [strongSelf
handleRenderCompleted:strongSelf->_pageCount] (or otherwise read a local
pageCount from strongSelf before calling) so the async block never dereferences
deallocated self; update the block around handleRenderCompleted to use
strongSelf and its _pageCount instead of self.
JaseElder
commented
Jan 27, 2026
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java`:
- Around line 239-265: getScreenshot may crash when pdfFileName has no "/" and
when pdfView has zero width/height; fix by validating pdfFileName (ensure not
null/empty and contains "/") before computing imageFileName and return
result.error with a clear message if invalid, and check pdfView.getWidth() and
getHeight() > 0 inside loadBitmapFromPDFView (or before calling it in
getScreenshot); if size is zero return result.error("FAIL", "view has zero
size", null) instead of creating the bitmap, and also guard against
loadBitmapFromPDFView returning null before attempting to write the file (return
an error if null).
In `@example/android/settings.gradle`:
- Around line 1-12: The pluginManagement block's inline flutterSdkPath closure
reads local.properties with file("local.properties").withInputStream which
throws FileNotFoundException without context; update the closure (the
flutterSdkPath def) to first check file("local.properties").exists() and if
missing throw or assert with a clear message like "local.properties not found;
please create it with flutter.sdk" (or log and fail), then proceed to load
properties and assert flutter.sdk is present before returning the path.
In `@example/ios/Runner.xcodeproj/project.pbxproj`:
- Line 370: The example project contains hardcoded DEVELOPMENT_TEAM entries
(DEVELOPMENT_TEAM = D8NS8ZJ3AA) in the Runner Xcode project; remove those keys
or set their values to empty so contributors aren’t forced to use a specific
team ID. Locate the DEVELOPMENT_TEAM occurrences in the Runner Xcode project
file (project.pbxproj) and either delete the DEVELOPMENT_TEAM = ... lines or
replace the right-hand value with an empty string, ensuring there are no
hardcoded team IDs remaining. After change, verify the project opens in Xcode
and prompts for a signing team selection.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m`:
- Around line 108-129: Add a one-time call to the Dart onLoadComplete from the
iOS side: introduce a BOOL ivar (e.g., _hasCalledOnLoadComplete) in FLTPDFView
and, inside the same place/method where the view currently fires the onRender
callback, check !_hasCalledOnLoadComplete and then call the controller's
onLoadComplete handler (via _controller) with the document info (use _pageCount
and current page index/_pageNo or _currentPageIndex), set
_hasCalledOnLoadComplete = YES, and ensure subsequent onRender calls do not call
onLoadComplete again so the callback is only invoked once.
- Around line 223-257: The local scrollView found in the dispatch_after block is
never assigned to the instance variable `_scrollView`, so `startObserving` and
`onDraw` never get triggered; update the block (where you search
self->_pdfView.subviews) to set `self->_scrollView = scrollView` after
discovery, then call `startObserving` (or restart observing) and ensure any
previous observer is stopped if needed, so the class-level `_scrollView` is used
for delegate/gesture handling and `onDraw` fires correctly.
- Around line 72-95: The onMethodCall:result: handler is missing branches for
"getScreenshot" and "reload", causing Dart calls to get
FlutterMethodNotImplemented; add explicit handling in that method: add else-if
clauses for @"getScreenshot" and @"reload" that either forward to corresponding
_pdfView methods (e.g., call [_pdfView getScreenshot:call result:result] and
[_pdfView reload:call result:result] if those methods exist) or, if not
implemented on iOS yet, return a structured FlutterError via
result([FlutterError errorWithCode:@"Unimplemented" message:@"getScreenshot not
supported on iOS" details:nil]) (and similarly for "reload") so callers receive
a clear platform-unsupported error instead of a generic not-implemented
response.
- Around line 131-135: The initializer initWithFrame:arguments:controller: calls
[super init] instead of the UIView designated initializer—replace that with
assigning self = [super initWithFrame:frame], check if self is non-nil before
setting instance variables (e.g., _controller) and any other setup, and return
self; ensure the frame parameter is respected and any existing logic that used
the previous super init is preserved inside the if (self) block.
In `@lib/flutter_pdfview.dart`:
- Around line 427-454: Both setPosition and setScale can leave their completers
pending if _channel.invokeMethod throws; wrap the invokeMethod call in a
try/catch/finally (or try/finally) inside setPosition and setScale so that
_setPositionCompleter and _setScaleCompleter are completed in the finally block
if not already completed, and then rethrow the caught exception (or return a
failure) to preserve error behavior; reference the methods setPosition and
setScale and the fields _setPositionCompleter, _setScaleCompleter and the
_channel.invokeMethod call when making this change.
In `@README.md`:
- Around line 70-82: Update the Controller Options table entry for setZoomLimits
so its Return column reflects the actual Dart API return type; change the
current "-" to `Future<bool>` for the setZoomLimits row (the symbol to locate is
the table row with "setZoomLimits" / the method name setZoomLimits).
🧹 Nitpick comments (1)
example/ios/Podfile (1)
34-36: Commented test target is fine but could be removed.This commented-out
RunnerTestsblock has no functional impact. Consider either uncommenting it if you plan to add iOS unit tests to the example app, or removing the commented lines entirely to keep the Podfile clean.
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
Comment on lines
+427
to
+454
| Future<bool> setPosition(Offset position) async { | ||
| if (_setPositionCompleter != null && !_setPositionCompleter!.isCompleted) { | ||
| await _setPositionCompleter!.future; | ||
| } | ||
| _setPositionCompleter = Completer<void>(); | ||
| final bool isSet = await _channel.invokeMethod('setPosition', <String, double>{ | ||
| 'xPos': position.dx, | ||
| 'yPos': position.dy, | ||
| }); | ||
| if (!_setPositionCompleter!.isCompleted) { | ||
| _setPositionCompleter!.complete(); | ||
| } | ||
| return isSet; | ||
| } | ||
|
|
||
| Future<bool> setScale(double scale) async { | ||
| if (_setScaleCompleter != null && !_setScaleCompleter!.isCompleted) { | ||
| await _setScaleCompleter!.future; | ||
| } | ||
| _setScaleCompleter = Completer<void>(); | ||
| final bool isSet = await _channel.invokeMethod('setScale', <String, double>{ | ||
| 'scale': scale, | ||
| }); | ||
| if (!_setScaleCompleter!.isCompleted) { | ||
| _setScaleCompleter!.complete(); | ||
| } | ||
| return isSet; | ||
| } |
There was a problem hiding this comment.
Complete _setPositionCompleter/_setScaleCompleter even on errors.
If the platform call throws, the completer remains pending and future calls will await forever.
🔧 Proposed fix
Future<bool> setPosition(Offset position) async {
if (_setPositionCompleter != null && !_setPositionCompleter!.isCompleted) {
await _setPositionCompleter!.future;
}
_setPositionCompleter = Completer<void>();
- final bool isSet = await _channel.invokeMethod('setPosition', <String, double>{
- 'xPos': position.dx,
- 'yPos': position.dy,
- });
- if (!_setPositionCompleter!.isCompleted) {
- _setPositionCompleter!.complete();
- }
- return isSet;
+ try {
+ final bool isSet = await _channel.invokeMethod('setPosition', <String, double>{
+ 'xPos': position.dx,
+ 'yPos': position.dy,
+ });
+ return isSet;
+ } finally {
+ if (!_setPositionCompleter!.isCompleted) {
+ _setPositionCompleter!.complete();
+ }
+ }
}
Future<bool> setScale(double scale) async {
if (_setScaleCompleter != null && !_setScaleCompleter!.isCompleted) {
await _setScaleCompleter!.future;
}
_setScaleCompleter = Completer<void>();
- final bool isSet = await _channel.invokeMethod('setScale', <String, double>{
- 'scale': scale,
- });
- if (!_setScaleCompleter!.isCompleted) {
- _setScaleCompleter!.complete();
- }
- return isSet;
+ try {
+ final bool isSet = await _channel.invokeMethod('setScale', <String, double>{
+ 'scale': scale,
+ });
+ return isSet;
+ } finally {
+ if (!_setScaleCompleter!.isCompleted) {
+ _setScaleCompleter!.complete();
+ }
+ }
}🤖 Prompt for AI Agents
In `@lib/flutter_pdfview.dart` around lines 427 - 454, Both setPosition and
setScale can leave their completers pending if _channel.invokeMethod throws;
wrap the invokeMethod call in a try/catch/finally (or try/finally) inside
setPosition and setScale so that _setPositionCompleter and _setScaleCompleter
are completed in the finally block if not already completed, and then rethrow
the caught exception (or return a failure) to preserve error behavior; reference
the methods setPosition and setScale and the fields _setPositionCompleter,
_setScaleCompleter and the _channel.invokeMethod call when making this change.
Comment on lines
+70
to
+82
| | Name | Description | Parameters | Return | | ||
| |:-------------------|:--------------------------------------------------------------------------------------------:|:------------------------------------------------:|:----------------:| | ||
| | getPageCount | Get total page count | - | `Future<int>` | | ||
| | getCurrentPage | Get current page | - | `Future<int>` | | ||
| | setPage | Go to/Set page | `int page` | `Future<bool>` | | ||
| | getCurrentPageSize | Return the width and height of the loaded page | - | `Future<Size>` | | ||
| | getScreenshot | Create a PNG of the contents of the PDFView and save to fileName | `String fileName` | `Future<String>` | | ||
| | getPosition | Get the position of the top left of the PDF with respect to the origin (top left of PDFView) | - | `Future<Offset>` | | ||
| | getScale | Get the PDF zoom value, for zooming | - | `Future<double>` | | ||
| | setPosition | Set the position of the top left of the PDF with respect to the origin (top left of PDFView) | `Offset position` | `Future<bool>` | | ||
| | setScale | Set the PDF zoom value, for zooming | `double scale` | `Future<bool>` | | ||
| | setZoomLimits | Set the minimum, maximum and mid bounds of the zoom limits | `double minZoom, double midZoom, double maxZoom` | - | | ||
| | reload | Reload the PDF document in the PDFView | - | `Future<bool>` | |
There was a problem hiding this comment.
Controller Options table: setZoomLimits return type is wrong.
The Dart API returns Future<bool>, not “-”.
📝 Proposed fix
-| setZoomLimits | Set the minimum, maximum and mid bounds of the zoom limits | `double minZoom, double midZoom, double maxZoom` | - |
+| setZoomLimits | Set the minimum, maximum and mid bounds of the zoom limits | `double minZoom, double midZoom, double maxZoom` | `Future<bool>` |🧰 Tools
🪛 LanguageTool
[style] ~77-~77: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...the position of the top left of the PDF with respect to the origin (top left of PDFView) | ...
(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
[style] ~79-~79: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ...the position of the top left of the PDF with respect to the origin (top left of PDFView) | ...
(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)
🤖 Prompt for AI Agents
In `@README.md` around lines 70 - 82, Update the Controller Options table entry
for setZoomLimits so its Return column reflects the actual Dart API return type;
change the current "-" to `Future<bool>` for the setZoomLimits row (the symbol
to locate is the table row with "setZoomLimits" / the method name
setZoomLimits).
Owner
|
Hey @JaseElder, great stuff. Can you resolve all those conflicts? Then I'm happy to merge |
Author
|
Hiya,
Yes, just working through them now.
Cheers
…On Fri, 30 Jan 2026 at 14:17, Tsogt Otgonbaatar ***@***.***> wrote:
*endigo* left a comment (endigo/flutter_pdfview#333)
<#333 (comment)>
Hey @JaseElder <https://github.com/JaseElder>, great stuff.
Can you resolve all those conflicts? Then I'm happy to merge
—
Reply to this email directly, view it on GitHub
<#333 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEQXDUZMRYJODODN4C7BRT4JNRWXAVCNFSM6AAAAACS67AL6CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQMRUGAYDMMJTG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java`:
- Around line 239-265: The getScreenshot method currently ignores the
caller-supplied output path and always writes "image.png" in the parent dir;
update getScreenshot to use the provided fileName as the output path (not a
hardcoded "image.png"), validate that the file's parent directory exists (create
it if necessary), open the FileOutputStream on the provided fileName, and
write/close the bitmap as before; apply the same change to the similar code
block at lines ~267-277 so both routines honor the caller-supplied path and
avoid unintended overwrites.
In `@example/android/settings.gradle`:
- Around line 23-28: The Android Gradle Plugin declaration (id
"com.android.application" version "8.11.1" in the plugins block) requires Gradle
8.13+, so update gradle-wrapper.properties to use a Gradle distribution URL
pointing to Gradle 8.13 or newer (e.g., 8.13/8.14) instead of the referenced
8.1.0; open gradle-wrapper.properties, change the distributionUrl to the
appropriate Gradle 8.13+ version, and commit the updated wrapper so the AGP and
Gradle versions are compatible.
In `@example/ios/Runner.xcodeproj/project.pbxproj`:
- Line 354: Update and align the iOS deployment target across the plugin
podspec, README, example Podfile, and the Xcode project by choosing a single
minimum target (e.g., 12.0 or 13.0) and applying it everywhere: change
IPHONEOS_DEPLOYMENT_TARGET occurrences in the Xcode project (symbol:
IPHONEOS_DEPLOYMENT_TARGET), update the s.platform value in the plugin podspec
(symbol: podspec s.platform/iOS), adjust the platform :ios value in the example
Podfile (symbol: Podfile platform :ios), and edit the README iOS support
statement to match the chosen version so all four sources are consistent.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m`:
- Around line 213-220: Guard against empty PDFs before calling pageAtIndex: by
checking pageCount == 0 and setting _defaultPage to nil (or otherwise handling
the empty document) instead of computing defaultPage = pageCount - 1; ensure you
only call [_document pageAtIndex: defaultPage] when pageCount > 0 and
defaultPage is clamped into [0, pageCount-1]. Update the logic around pageCount,
defaultPage and _defaultPage to return early or set a safe default when the
document has zero pages to avoid passing a negative index to pageAtIndex:.
- Around line 37-43: The initializer
initWithFrame:viewIdentifier:arguments:binaryMessenger: creates _pdfView by
calling [[FLTPDFView new] initWithFrame:...] which double-initializes the
object; replace that with an alloc/init pattern so _pdfView is created via
[[FLTPDFView alloc] initWithFrame:arguments:controller:] (i.e. construct
FLTPDFView with alloc then call initWithFrame:arguments:controller:) to avoid
calling -init twice and ensure proper initialization of FLTPDFView in the
initWithFrame:... method.
🧹 Nitpick comments (1)
example/android/settings.gradle (1)
12-12: Remove commented-out dead code.This line is a leftover from previous refactoring. Since
flutterSdkPathis now a String (not a closure), callingflutterSdkPath()would be invalid anyway. Remove it to avoid confusion.🧹 Proposed fix
}() -// settings.ext.flutterSdkPath = flutterSdkPath() - includeBuild("$flutterSdkPath/packages/flutter_tools/gradle")
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
* fixed image file naming * handle pagecount of 0
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java`:
- Around line 50-54: The code currently assigns a per-instance value to the
global constant Constants.THUMBNAIL_RATIO inside FlutterPDFView, causing
cross-instance side effects; change this by introducing an instance field (e.g.,
private Float thumbnailRatio) on FlutterPDFView and set that from
getFloat(params, "thumbnailRatio") instead of writing to
Constants.THUMBNAIL_RATIO, then update any internal code that reads
Constants.THUMBNAIL_RATIO to read the new instance field; if you must keep
modifying the static, capture the original value when setting it and restore it
in FlutterPDFView.dispose() (override dispose() to reset
Constants.THUMBNAIL_RATIO to the saved original) so other PDFView instances are
unaffected.
- Around line 172-176: The getCurrentPageSize method can throw a
NullPointerException because pdfView.getPageSize(pdfView.getCurrentPage()) may
return null; update getCurrentPageSize to check the returned SizeF (from
pdfView.getPageSize) for null before accessing getWidth()/getHeight(), and
handle the null case by returning an appropriate Result (e.g., result.error with
a clear message or result.success(null) / an empty array) so callers don't
crash; locate the check in the getCurrentPageSize method and use
pdfView.getCurrentPage and Result to implement the safe-return behavior.
- Around line 222-224: The current guard if (zoom != 1.0) prevents calling
pdfView.zoomTo((float) zoom) when the caller explicitly requests zoom 1.0, so an
explicit reset to default scale won't occur; update the logic in FlutterPDFView
where zoom is handled (the block using the zoom variable and calling
pdfView.zoomTo and loadPages()) to always call pdfView.zoomTo((float) zoom) when
a scale is provided (or use a comparison against the view's current zoom rather
than the literal 1.0), ensuring explicit setScale(1.0) actually resets the view;
adjust or remove the if (zoom != 1.0) check and keep loadPages() behavior
unchanged.
- Around line 253-265: The FileOutputStream created in getScreenshot leaks if
bmp.compress throws and the Bitmap is never recycled; update the getScreenshot
flow that calls loadBitmapFromPDFView() to use try-with-resources for the
FileOutputStream (wrap new FileOutputStream(outputFile, false) so it is always
closed) and ensure the Bitmap (bmp) is recycled after usage (call bmp.recycle()
in a finally block or after successful result.success(imageFileName)) while
preserving the null-check that already returns on bmp == null; reference the bmp
variable, the FileOutputStream created for outputFile, and the
result.success/result.error calls when implementing the fix.
In `@ios/flutter_pdfview.podspec`:
- Line 21: Update project documentation and release notes to reflect the raised
iOS minimum by documenting that s.ios.deployment_target has been changed to
'12.0' (see flutter_pdfview.podspec), noting this is a breaking change for iOS
11 users and listing any migration steps or impacts for dependent apps; ensure
README, CHANGELOG (or release-notes) and any integration/instructions pages
explicitly state the new minimum iOS version and call out required client
updates.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m`:
- Around line 427-433: The setPage handler currently calls [_pdfView.document
pageAtIndex:...] and then [_pdfView goToPage:] without validating the passed
page number; add a bounds check in setPage:result: to ensure the NSNumber* page
is non-nil and its unsigned integer value is less than
_pdfView.document.pageCount before calling pageAtIndex: and goToPage:. If the
page is out of range or invalid, return an appropriate Flutter result (e.g., a
FlutterError or result(`@NO`)) instead of calling goToPage:, and keep existing
success behavior when the index is valid; reference the setPage method,
_pdfView, pageAtIndex:, pageCount, and goToPage: to locate and change the code.
- Around line 261-264: The block captures self unsafely: change the async block
to avoid directly referencing self->_document; either capture the page count
before dispatch_async or use the weak->strong pattern with the captured weakSelf
(e.g., create __strong typeof(weakSelf) strongSelf = weakSelf; if (!strongSelf)
return;) and then call [strongSelf handleRenderCompleted:[NSNumber
numberWithUnsignedLong:[strongSelf->_document pageCount]]]; ensure you reference
weakSelf/strongSelf and handle the nil case to prevent dereferencing a
deallocated self in FlutterPDFView's render-completion dispatch.
🧹 Nitpick comments (2)
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java (1)
293-307: Redundant null check can be simplified.The inner null check (lines 297-302) is unnecessary since the outer
if (call.argument("page") != null)already guards the block.♻️ Optional simplification
void setPage(MethodCall call, Result result) { - if (call.argument("page") != null) { - Integer pageObj = call.argument("page"); - int page; - if (pageObj != null) { - page = pageObj; // Safe unboxing - } else { - // Handle null case, e.g., assign a default value - page = 1; - } + Integer pageObj = call.argument("page"); + if (pageObj != null) { + int page = pageObj; pdfView.jumpTo(page); } - result.success(true); }ios/flutter_pdfview.podspec (1)
17-18: Add Swift support to the podspec glob for future compatibility.The current glob pattern
'flutter_pdfview/Sources/flutter_pdfview/**/*.{h,m}'only includes Objective-C sources. If Swift implementation is planned or added later, it won't be automatically compiled. Consider adding.swiftto the pattern:'flutter_pdfview/Sources/flutter_pdfview/**/*.{h,m,swift}'to ensure Swift sources are included if needed.
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
* handle potential memory leak on image generation in Android * handle new doc size on reload in iOS * handle page range limit in set page in iOS
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java`:
- Around line 198-218: For setPosition, setScale, and setPage, replace the
current silent-default behavior with explicit argument validation: in
setPosition check that "xPos" and "yPos" are non-null Doubles and if either is
missing call result.error(...) and return; in setScale ensure the "scale"
argument is present, is a valid positive number (reject <=0) and call
result.error(...) if invalid; in setPage ensure the "page" argument is present,
is an integer within the PDF page range and call result.error(...) if out of
range or missing. Use the existing MethodCall/Result pattern to return a clear
error code and message, and only call
pdfView.moveTo/pdfView.loadPages/setScale/pdfView.jumpTo after validation
succeeds (also guard pdfView != null where used). Reference methods:
setPosition, setScale, setPage, and ensure the error uses result.error(...)
rather than silently defaulting values.
- Around line 236-244: In setZoomLimits, validate the zoom triplet before
applying: extract minZoom/midZoom/maxZoom from the MethodCall (as currently
done), check they are non-null (or apply sensible defaults), ensure each is > 0
and that minZoom ≤ midZoom ≤ maxZoom; if the values are invalid, do not call
pdfView.setMinZoom/setMidZoom/setMaxZoom and call result.error(...) with a clear
error code/message describing the ordering/positivity violation; only call
pdfView setters and result.success(true) when the validated constraints pass.
Reference: setZoomLimits, pdfView.setMinZoom, pdfView.setMidZoom,
pdfView.setMaxZoom, and result.error/result.success.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m`:
- Around line 283-300: The KVO observer on _scrollView is being registered
multiple times because startObserving is called from handleRenderCompleted
repeatedly; add a boolean guard (e.g., an instance ivar/property like
_isObserving or isObserving) to startObserving to only call addObserver if not
already observing, set the flag true after successful registration, and update
stopObserving (and dealloc) to only removeObserver if the flag is true and then
clear it; update both startObserving/stopObserving references (and any other
places that call them such as handleRenderCompleted) to use this guard so
registrations/removals stay balanced.
- Around line 197-199: The _pdfView.maxScaleFactor is being set to
[args[@"maxZoom"] doubleValue] which yields 0.0 when maxZoom is missing and
creates invalid zoom constraints; update the assignment in FlutterPDFView.m to
validate the parsed maxZoom (args[@"maxZoom"]) and only set
_pdfView.maxScaleFactor when the value is > 0, otherwise fall back to the safe
default (e.g., 4.0) and ensure _pdfView.minScaleFactor remains
_pdfView.scaleFactorForSizeToFit so min <= max is preserved.
- Around line 375-402: Guard against _currentPage (and
_pdfView/_pdfView.document) being nil in getPosition:result: and
setPosition:result: to avoid calling convertRect:toPage: or boundsForBox: on a
nil page; if any required object is missing, immediately return a FlutterError
via the FlutterResult (use a clear code like "PDF_NOT_LOADED" and a short
message) instead of performing the math or calling goToRect:. Update getPosition
and setPosition to check _currentPage (and optionally
_pdfView/_pdfView.document) at the top and return the error early so the
subsequent calculations (_pageSpaceRect, boundsForBox:, scale math, goToRect:)
only run when the PDF/page is initialized.
- Around line 404-408: The setScale handler currently assigns scale without
validation, causing collapse when scale is nil or invalid; update -
(void)setScale:(FlutterMethodCall*)call result:(FlutterResult)result to validate
call.arguments[@"scale"] (ensure it's an NSNumber and has a positive, non-zero
doubleValue) before assigning to _pdfView.scaleFactor; if validation fails,
return a FlutterError (similar to setPage's bounds checks) describing "Invalid
scale" and do not change the view, otherwise set _pdfView.scaleFactor and return
success.
🧹 Nitpick comments (1)
ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m (1)
320-333: Useboundsfor subview layout.Using
self.frameinsidelayoutSubviewscan offset the PDFView if the host view is positioned.boundskeeps the subview aligned to its own coordinate space.✅ Suggested change
- _pdfView.frame = self.frame; + _pdfView.frame = self.bounds;
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
Show resolved
Hide resolved
* handled min, mid, max zoom range limits in Android * added guards for get/set position * added guard for set scale
# Conflicts: # CLAUDE.md # README.md # android/build.gradle # android/gradle.properties # android/gradle/wrapper/gradle-wrapper.properties # android/gradlew.bat # android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java # example/android/app/build.gradle # example/android/gradle.properties # example/android/gradle/wrapper/gradle-wrapper.properties # example/android/settings.gradle # example/ios/Flutter/Flutter.podspec # example/ios/Flutter/ephemeral/Packages/FlutterGeneratedPluginSwiftPackage/Package.swift # example/ios/Podfile # example/ios/Podfile.lock # example/ios/Runner.xcodeproj/project.pbxproj # example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme # example/lib/main.dart # example/pubspec.lock # example/pubspec.yaml # ios/flutter_pdfview.podspec # ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m # lib/flutter_pdfview.dart # pubspec.lock
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m`:
- Around line 464-472: setZoomLimits currently computes and applies zoom factors
directly to _pdfView.minScaleFactor/_pdfView.maxScaleFactor which are later
clobbered by layoutSubviews; modify setZoomLimits to save the provided
minZoom/maxZoom into new instance variables (e.g. _customMinScale and
_customMaxScale) after converting by _pdfView.scaleFactorForSizeToFit, and then
apply them to _pdfView as before. In layoutSubviews, stop unconditionally
resetting _pdfView.minScaleFactor/_pdfView.maxScaleFactor to hard-coded values
and instead check if _customMinScale/_customMaxScale are set—if so, restore
those values; otherwise fall back to the size-to-fit and default (4.0) values.
Ensure the symbols to update are setZoomLimits, layoutSubviews, _customMinScale,
_customMaxScale, and the assignments to
_pdfView.minScaleFactor/_pdfView.maxScaleFactor.
- Around line 378-385: In getPosition:, replace the use of _pdfView.frame with
_pdfView.bounds when calling [_pdfView convertRect:toPage:_currentPage] to
ensure the rectangle is in the PDFView's local coordinate system; update the
assignment to _pageSpaceRect to call convertRect:toPage: with _pdfView.bounds
(same coordinate approach as used earlier) so _pageSpaceRect calculations that
follow use consistent PDFView-local coordinates.
…rgs[@"maxZoom"] doubleValue], where that arg is never present. * removed maxScaleFactor and minScaleFactor from layoutSubviews
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
I've added a number of features to the library, which may be useful to others.
I've squashed all the commits to hopefully make it easier to review. Can you have a look and maybe merge, as I have a plugin I'm writing that I want to release to pub.dev, which uses this library as a dependency - so for me to publish the other, these changes have to be merged in. Else I may have to publish my fork of this instead.
There shouldn't be any breaking changes here, as the two callbacks are optional.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores