-
-
Notifications
You must be signed in to change notification settings - Fork 0
chore: sync upstream PR #7599 - fix(ios): allow enabling gesture navigation via config (#3808) #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: plus
Are you sure you want to change the base?
Conversation
Signed-off-by: Martin Mazein <internet+github@mazein.net>
WalkthroughThis PR adds support for a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
2d31533 to
3f9d94a
Compare
3f9d94a to
4354ce2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ios/Capacitor/Capacitor/CAPBridgeViewController.swift(1 hunks)ios/Capacitor/Capacitor/CAPInstanceConfiguration.h(1 hunks)ios/Capacitor/Capacitor/CAPInstanceConfiguration.m(2 hunks)ios/Capacitor/Capacitor/CAPInstanceDescriptor.h(1 hunks)ios/Capacitor/Capacitor/CAPInstanceDescriptor.m(1 hunks)ios/Capacitor/Capacitor/CAPInstanceDescriptor.swift(1 hunks)
🧰 Additional context used
🪛 Clang (14.0.6)
ios/Capacitor/Capacitor/CAPInstanceConfiguration.h
[error] 22-22: expected identifier or '('
(clang-diagnostic-error)
⏰ 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). (4)
- GitHub Check: test-android
- GitHub Check: test-ios (/Applications/Xcode_26.0.app)
- GitHub Check: test-cli
- GitHub Check: test-core
🔇 Additional comments (6)
ios/Capacitor/Capacitor/CAPInstanceDescriptor.m (1)
42-42: LGTM! Default initialization is correct.The default value of
NOpreserves existing behavior (gestures disabled), preventing any breaking changes. The placement is consistent with surrounding properties.ios/Capacitor/Capacitor/CAPInstanceDescriptor.swift (1)
114-116: LGTM! Configuration parsing is correctly implemented.The implementation follows the established pattern for boolean configuration options, with proper optional unwrapping and assignment.
ios/Capacitor/Capacitor/CAPBridgeViewController.swift (1)
304-304: LGTM! WebView configuration is correctly applied.The configuration value is properly applied to the WKWebView during preparation, following the same pattern as
allowsLinkPreview.ios/Capacitor/Capacitor/CAPInstanceConfiguration.h (1)
22-22: LGTM! Property declaration is correct.The property declaration follows the correct Objective-C syntax and pattern. The static analysis error is a false positive—the syntax is valid.
ios/Capacitor/Capacitor/CAPInstanceConfiguration.m (2)
34-34: LGTM! Property propagation is correctly implemented.The value is properly copied from the descriptor during initialization.
74-74: LGTM! Configuration copying is correctly implemented.The value is properly propagated when creating a new configuration instance.
| /** | ||
| @brief Whether or not the web view will allow gesture navigation . | ||
| @discussion Set by @c ios.allowsBackForwardNavigationGestures in the configuration file. Corresponds to @c allowsBackForwardNavigationGestures on WKWebView. | ||
| */ | ||
| @property (nonatomic, assign) BOOL allowsBackForwardNavigationGestures; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor typo in documentation comment.
The property declaration is correct and follows the established pattern. However, there's an extra space before the period on line 97: "gesture navigation ." should be "gesture navigation."
Apply this diff to fix the typo:
/**
- @brief Whether or not the web view will allow gesture navigation .
+ @brief Whether or not the web view will allow gesture navigation.
@discussion Set by @c ios.allowsBackForwardNavigationGestures in the configuration file. Corresponds to @c allowsBackForwardNavigationGestures on WKWebView.
*/
@property (nonatomic, assign) BOOL allowsBackForwardNavigationGestures;📝 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.
| /** | |
| @brief Whether or not the web view will allow gesture navigation . | |
| @discussion Set by @c ios.allowsBackForwardNavigationGestures in the configuration file. Corresponds to @c allowsBackForwardNavigationGestures on WKWebView. | |
| */ | |
| @property (nonatomic, assign) BOOL allowsBackForwardNavigationGestures; | |
| /** | |
| @brief Whether or not the web view will allow gesture navigation. | |
| @discussion Set by @c ios.allowsBackForwardNavigationGestures in the configuration file. Corresponds to @c allowsBackForwardNavigationGestures on WKWebView. | |
| */ | |
| @property (nonatomic, assign) BOOL allowsBackForwardNavigationGestures; |
🤖 Prompt for AI Agents
In ios/Capacitor/Capacitor/CAPInstanceDescriptor.h around lines 96 to 100, the
documentation comment has an extra space before the period in "gesture
navigation ."; remove the stray space so the sentence reads "gesture
navigation." to match the established comment formatting and punctuation.
Upstream PR Sync
This PR syncs changes from an external contributor's PR on the official Capacitor repository.
Original PR
Automation
Synced from upstream by Capacitor+ Bot
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.