-
Notifications
You must be signed in to change notification settings - Fork 3
feat(push): bridge open web URL actions to Dart #116
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: rel/0.3.0
Are you sure you want to change the base?
Changes from all commits
201ec12
edc4203
d970376
c73651f
a57f7f1
3e13236
2ad64d7
2b26013
73d673f
d671b8f
3e9e620
0e58931
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,7 @@ import 'models/klaviyo_event.dart'; | |||||||||||||||||||||||||||||||||||||
| import 'models/in_app_form_config.dart'; | ||||||||||||||||||||||||||||||||||||||
| import 'models/geofence.dart'; | ||||||||||||||||||||||||||||||||||||||
| import 'models/form_lifecycle_event.dart'; | ||||||||||||||||||||||||||||||||||||||
| import 'models/klaviyo_push_action.dart'; | ||||||||||||||||||||||||||||||||||||||
| import 'enums/klaviyo_log_level.dart'; | ||||||||||||||||||||||||||||||||||||||
| import 'services/klaviyo_native_wrapper.dart'; | ||||||||||||||||||||||||||||||||||||||
| import 'package:logging/logging.dart'; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -423,6 +424,28 @@ class KlaviyoSDK { | |||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /// Get typed push action events stream. | ||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||
| /// Emits a [KlaviyoPushAction] when a user taps a push whose action opens an | ||||||||||||||||||||||||||||||||||||||
| /// external web URL ([OpenWebUrl]), or taps an action button on a push | ||||||||||||||||||||||||||||||||||||||
| /// ([ActionButtonTapped]). Filters for the `push_open_web_url` and | ||||||||||||||||||||||||||||||||||||||
| /// `push_action_button_tapped` event types and parses the native map. | ||||||||||||||||||||||||||||||||||||||
| /// Malformed events are logged and dropped rather than crashing the stream. | ||||||||||||||||||||||||||||||||||||||
| Stream<KlaviyoPushAction> get onPushAction => _nativeWrapper.onPushActionEvent | ||||||||||||||||||||||||||||||||||||||
| .where( | ||||||||||||||||||||||||||||||||||||||
| (event) => | ||||||||||||||||||||||||||||||||||||||
| event['type'] == 'push_open_web_url' || | ||||||||||||||||||||||||||||||||||||||
| event['type'] == 'push_action_button_tapped', | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| .expand((event) { | ||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||
| return [KlaviyoPushAction.fromMap(event)]; | ||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||
| _logger.warning('Dropping malformed push action event: $e'); | ||||||||||||||||||||||||||||||||||||||
| return []; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+440
to
+447
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Locate klaviyo_sdk.dart =="
fd -a 'klaviyo_sdk\.dart$' .
echo "== Extract relevant lines =="
if [ -f lib/src/klaviyo_sdk.dart ]; then
sed -n '425,455p' lib/src/klaviyo_sdk.dart | cat -n
fi
echo "== Check require_trailing_commas config references =="
rg -n "require_trailing_commas|trailing_commas|avoid_trailing_commas|dart format" . -g '!build/**' -g '!dist/**' -g '!node_modules/**' || true
echo "== Check local analyzer/dart configuration files =="
for f in analysis_options.yaml pubspec.yaml pubspec.yaml.lock .dart_tool/package_config.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,220p' "$f"
fi
done
echo "== Extract surrounding stream chain with line numbers =="
nl -ba lib/src/klaviyo_sdk.dart | sed -n '420,455p'Repository: klaviyo/klaviyo-flutter-sdk Length of output: 4125 🌐 Web query:
💡 Result: The require_trailing_commas lint rule in Dart is designed to enforce the use of trailing commas in multi-line parameter and argument lists [1]. Key details regarding its usage and behavior: How to Enable: To enable this lint, add it to your analysis_options.yaml file under the linter rules section [1]: linter: rules: - require_trailing_commas Behavior and Limitations: - The rule encourages using trailing commas for all multi-line parameter and argument lists. If a list fits on a single line, a trailing comma is not required [1]. - It is intended to complement the Dart formatter (dart format), which also automatically manages trailing commas [1][2]. - NOTE: The rule assumes code has already been processed by dart format; it may produce false positives on unformatted code [1]. - For projects targeting Dart language version 3.7 or later (using the new "tall" style formatter), the require_trailing_commas lint rule is automatically disabled, as the formatter's automated behavior takes precedence [3]. Interaction with Formatter Configuration: In Dart 3.7 and later, the default formatter behavior is to automatically add or remove trailing commas based on whether a construct spans multiple lines [2][4]. Users who prefer manual control over line breaks—where the presence of a trailing comma forces a construct to split across lines—can opt into this behavior by configuring the formatter in analysis_options.yaml [2][5][6]: formatter: trailing_commas: preserve When this preserve setting is used, it restores the ability to force line breaks via trailing commas, which was a standard feature of the older Dart formatter [5][6]. Using the lint rule alongside this setting helps maintain consistent application of that style across the codebase [7]. Citations:
🌐 Web query:
💡 Result: The Flutter 3.24 SDK release is paired with Dart 3.5 [1][2]. This release was officially announced on August 6, 2024 [1][2]. Citations:
Add the required trailing comma to the multiline The repository enforces Proposed formatting fix- .expand((event) {
+ .expand(
+ (event) {
try {
return [KlaviyoPushAction.fromMap(event)];
} catch (e) {
_logger.warning('Dropping malformed push action event: $e');
return [];
}
- });
+ },
+ );📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /// Private methods | ||||||||||||||||||||||||||||||||||||||
| void _ensureInitialized() { | ||||||||||||||||||||||||||||||||||||||
| if (!_isInitialized) { | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
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.
Stale iOS push action cache
Medium Severity
When Flutter is not ready,
forwardPushActionstores a push action incachedPushActionbut returns early when the latest tap has no typed action without clearing that cache. After a second notification open overwritescachedOpenedNotification,onListencan deliver a mismatchedpush_open_web_urlor button event from an earlier tap.Reviewed by Cursor Bugbot for commit 0e58931. Configure here.