[PdfKit] Implement manual bindings for PdfViewAnnotationHitEventArgs.AnnotationHit.#25068
[PdfKit] Implement manual bindings for PdfViewAnnotationHitEventArgs.AnnotationHit.#25068rolfbjarne wants to merge 2 commits intomainfrom
Conversation
…AnnotationHit. Because it's accessed using a literal string, not a constant string.
There was a problem hiding this comment.
Pull request overview
Adds a manual managed binding for PdfViewAnnotationHitEventArgs.AnnotationHit in PdfKit, because Apple’s userInfo key is a literal string ("PDFAnnotationHit") rather than an exported constant.
Changes:
- Added a new partial implementation of
PdfViewAnnotationHitEventArgs.AnnotationHitthat reads"PDFAnnotationHit"fromNSNotification.UserInfo. - Removed the incorrect generated
[Export ("PDFAnnotationHit")]property from the binding definition so it won’t be emitted as a (non-existent) native constant lookup. - Added monotouch tests to validate
AnnotationHitextraction (including a missing-UserInfocase).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/monotouch-test/PdfKit/PdfAnnotationTest.cs | Adds tests that build an NSNotification with userInfo["PDFAnnotationHit"] and validate AnnotationHit behavior. |
| src/PdfKit/PdfViewAnnotationHitEventArgs.cs | New manual partial binding that reads the literal userInfo key and returns a PdfAnnotation. |
| src/pdfkit.cs | Removes the autogenerated binding member for AnnotationHit and notes it’s manually bound. |
| src/frameworks.sources | Ensures the new manual binding source file is compiled into the PdfKit sources. |
| @@ -0,0 +1,18 @@ | |||
| namespace PdfKit; | |||
|
|
|||
| partial class PdfViewAnnotationHitEventArgs : NSNotificationEventArgs { | |||
There was a problem hiding this comment.
PdfViewAnnotationHitEventArgs is generated as a public partial class (see bgen's event-args generation). This partial declaration has no access modifier, so it defaults to internal and will cause a partial-type accessibility mismatch at compile time. Make this declaration public partial class ... to match the generated part.
| partial class PdfViewAnnotationHitEventArgs : NSNotificationEventArgs { | |
| public partial class PdfViewAnnotationHitEventArgs : NSNotificationEventArgs { |
| namespace PdfKit; | ||
|
|
||
| partial class PdfViewAnnotationHitEventArgs : NSNotificationEventArgs { | ||
| // This property needs a manual binding, because it's not using a constant string value, | ||
| // it's using a literal string value (as per Apple's documentation). |
There was a problem hiding this comment.
This new PdfKit source file doesn't enable nullable context (unlike the other PdfKit sources) and also uses a file-scoped namespace while the rest of src/PdfKit/*.cs uses block-scoped namespace PdfKit { ... }. Consider adding #nullable enable and matching the existing namespace style for consistency within the PdfKit sources.
| namespace PdfKit; | ||
|
|
||
| partial class PdfViewAnnotationHitEventArgs : NSNotificationEventArgs { |
There was a problem hiding this comment.
PdfViewAnnotationHitEventArgs is marked [NoiOS][NoTV][NoMacCatalyst] in src/pdfkit.cs, but this manual partial type is currently compiled unconditionally. If that platform restriction is intentional, wrap this file (or at least this type) in the corresponding #if MONOMAC/platform guard so the type/property doesn't appear on unsupported platforms.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #760f726] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #760f726] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #760f726] Build passed (Build macOS tests) ✅Pipeline on Agent |
🔥 [CI Build #760f726] Test results 🔥Test results❌ Tests failed on VSTS: test results 3 tests crashed, 1 tests failed, 149 tests passed. Failures❌ cecil tests🔥 Failed catastrophically on VSTS: test results - cecil (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (iOS)🔥 Failed catastrophically on VSTS: test results - dotnettests_ios (no summary found). Html Report (VSDrops) Download ❌ interdependent-binding-projects tests🔥 Failed catastrophically on VSTS: test results - interdependent-binding-projects (no summary found). Html Report (VSDrops) Download ❌ linker tests1 tests failed, 43 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Because it's accessed using a literal string, not a constant string.