Conversation
WalkthroughAdds a 2.2.2 release entry and version bump across package metadata and SDK identifiers, and tightens the Safari web-push migration prompt by guarding the Safari path with a runtime PushManager availability check that causes an early return when absent. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant SDK as Web SDK (v2.2.2)
participant B as Browser (Safari)
participant PM as PushManager
U->>SDK: Initialize / trigger push prompt
SDK->>B: Detect Safari + migration state
alt PushManager not available
SDK->>B: Check 'PushManager' in window? — false
Note over SDK,B #FDEBD0: Early return from Safari migration path\n(no prompt shown)
SDK-->>U: Exit prompt flow
else PushManager available
SDK->>PM: Proceed with migration/prompt logic
PM-->>SDK: Push flow continues
SDK-->>U: Prompt shown / migration proceeds
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 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)
✨ Finishing Touches🧪 Generate unit tests
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 |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) ✅ code/snyk check is complete. No issues have been found. (View Details) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/modules/webPushPrompt/prompt.js (2)
330-338: Same issue here: guard Notification before use.
Without the check this can throw on unsupported browsers/contexts.- bellIcon.addEventListener('click', () => { - if (Notification.permission === 'denied') { + bellIcon.addEventListener('click', () => { + if (!('Notification' in window) || Notification == null) { + // No native notifications support; show guidance GIF instead. + toggleGifModal(bellWrapper) + return + } + if (Notification.permission === 'denied') {
255-256: Guard allNotification.permissionchecks with a window‐object guard
AccessingNotification.permissionwhenNotificationis undefined (e.g. certain Safari contexts) throws a ReferenceError. Wrap every instance ofNotification.permissionin a check for'Notification' in window. For example:- if (Notification.permission === 'granted') { … } + if (('Notification' in window) && Notification.permission === 'granted') { … }Occurrences to update:
- src/modules/webPushPrompt/prompt.js at lines 214, 218, 255, 330, 335
- src/modules/notification.js at lines 464, 468
🧹 Nitpick comments (5)
CHANGELOG.md (1)
4-6: Fix typos/casing in 2.2.2 entry.
Polish the wording.-## [2.2.2] 9th Sept 2025 -- Fixed hndling of safari web push notifications in iPhone. +## [2.2.2] 9th Sept 2025 +- Fixed handling of Safari web push notifications on iPhone.src/modules/webPushPrompt/prompt.js (1)
240-241: Good guard to avoid showing soft prompt when PushManager is unavailable on Safari iPhone.
This early return prevents an impossible opt-in path from being shown.For readability, consider hoisting a single
const isPushSupported = 'PushManager' in windowand reuse it where checked (Lines 211, 233, 241).clevertap.js (3)
16331-16332: Deduplicate SDK version string.The literal 'web-sdk-v2.2.2' is baked in here; it’s also used elsewhere. Prefer a single source of truth.
Apply:
- lib: 'web-sdk-v2.2.2', + lib: `web-sdk-v${SDK_VERSION}`,And add once (near top-level bootstrapping, after IIFE header or alongside other consts):
// SDK Version const SDK_VERSION = '2.2.2';
18253-18253: Use shared SDK_VERSION constant.Avoid drift between getSDKVersion() and af.lib.
Apply:
- return 'web-sdk-v2.2.2'; + return `web-sdk-v${SDK_VERSION}`;
13852-13852: Keep Builder SDK check in sync with SDK_VERSION.Prevents mismatches shown in Visual Builder.
Apply:
- const sdkVersion = '2.2.2'; + const sdkVersion = SDK_VERSION;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
clevertap.js.mapis excluded by!**/*.mapclevertap.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)clevertap.js(4 hunks)package.json(1 hunks)src/modules/webPushPrompt/prompt.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/modules/webPushPrompt/prompt.js (1)
clevertap.js (3)
vapidSupportedAndMigrated(11381-11381)vapidSupportedAndMigrated(11835-11835)fcmPublicKey(11568-11568)
clevertap.js (2)
src/modules/webPushPrompt/prompt.js (2)
vapidSupportedAndMigrated(211-211)fcmPublicKey(19-19)src/modules/visualBuilder/pageBuilder.js (1)
sdkVersion(33-33)
🔇 Additional comments (2)
package.json (1)
3-3: LGTM: version bump aligned with release.
No issues spotted here.clevertap.js (1)
11869-11870: Good Safari iPhone guard — prevents soft prompt when unsupported or already migrated.Early return on missing PushManager/FCM key and when migration prompt was already shown avoids duplicate/invalid prompts on iOS Safari. Looks correct given VAPID/APNs differences.
Please sanity-check on:
- iOS Safari <16.4 (no PushManager): prompt should never render in the “not-expired” branch.
- iOS Safari ≥16.4 with VAPID key: first-time render allowed, subsequent renders (timer not expired) suppressed.
- APNs-only Safari flow (no VAPID key): ensure normal prompt still renders only under timer-expired path.
3 participants
Changes
Added the condition for handlin web push soft prompt rendering in safari for iPhone
Changes to Public Facing API if any
No impact
How Has This Been Tested?
Tested in Safari iPhone using browserstack and on actual device
Checklist
Link to Deployed SDK
Use these url for testing :
https://static.wizrocket.com/staging/task/WEB-3937/safari_web_push/js/clevertap.min.jshttps://static.wizrocket.com/staging/task/WEB-3937/safari_web_push/js/sw_webpush.min.jsHow to trigger Automations
Just add a empty commit after all your changes are done in the PR with the command
git commit --allow-empty -m "[run-test] Testing Automation"This will trigger the automation suite
Summary by CodeRabbit
Bug Fixes
Chores
Documentation