Skip to content

Conversation

@belleklaviyo
Copy link
Contributor

@belleklaviyo belleklaviyo commented Sep 24, 2025

Description

Updating the sample app(s) in this repo and prepping them for geofencing updates. Big goal of getting rid of using the Storyboard and moving it all (or as much as possible) to SwiftUI. Fairly straightforward change for the SPMExample, but since Cocoapods relies on the actual pods, I had to create a new podspec for the upcoming KlaviyoLocation pod - kind of a placeholder for now (as that will actually be coming) but the Cocoapods example still has trouble building as it is somewhat self referential and unpublished but still felt like a good update to do.

Still kept the main AppDelegate where we had a lot of instructional comments showing how to write calls to our SDK.

Due Diligence

  • I have tested this on a simulator or a physical device.
  • I have added sufficient unit/integration tests of my changes.
  • I have adjusted or added new test cases to team test docs, if applicable.
  • I am confident these changes are compatible with all iOS and XCode versions the SDK currently supports.

Release/Versioning Considerations

  • Patch Contains internal changes or backwards-compatible bug fixes.
  • Minor Contains changes to the public API.
  • Major Contains breaking changes.
  • Contains readme or migration guide changes.
    • If so, please merge to a feature branch so documentation updates only go live upon official release.
  • This is planned work for an upcoming release.
    • If no, author or reviewer should account for this in a release plan, or describe why not below.

Changelog / Code Overview

new.app.mov
map.portion.mov

Test Plan

Related Issues/Tickets

https://klaviyo.atlassian.net/browse/CHNL-25115


Note

Migrates example apps to SwiftUI with a new geofencing map flow, updates app/ project configs (Pods/SPM, Info.plists), and adds an internal API to query currently monitored geofences.

  • Examples (SPM & CocoaPods):
    • SwiftUI Migration: Add CocoapodsExampleApp/KLMuncheryApp, ContentView, LoginView, MenuView, CheckoutView; remove Main.storyboard and related table view cells.
    • Geofencing UI: New MapView to register/unregister geofencing and visualize monitored regions with annotations and radius overlays.
    • State & Persistence: Introduce AppState to manage auth, profile, and cart; persist via UserDefaults.
    • AppDelegate: Keep SDK setup/push/deep link handling; fix loop variable when reading key_value_pairs from notifications.
    • Configs: Update Info.plist (URL schemes, background remote-notification & location, location usage strings, bundle IDs). CocoaPods adds KlaviyoLocation; SPM switches to local package, adds KlaviyoLocation, updates project/scheme.
  • SDK (KlaviyoLocation):
    • Add KlaviyoLocationManager.getCurrentGeofences() and surface via KlaviyoSDK.getCurrentGeofences() (SPI, deprecated notice); keep registerGeofencing/unregisterGeofencing APIs.
  • Models & Assets:
    • MenuItem becomes Codable and drops image; adjust Cart and remove unused image assets.

Written by Cursor Bugbot for commit ee8c74d. This will update automatically on new commits. Configure here.

@belleklaviyo belleklaviyo force-pushed the bl/sample-app-geofencing branch from e0cc089 to 40eed06 Compare September 25, 2025 12:21
@wiz-inc-faae60d47d
Copy link

wiz-inc-faae60d47d bot commented Sep 25, 2025

Wiz Scan Summary

Scanner Findings
Vulnerability Finding Vulnerabilities -
Data Finding Sensitive Data -
Total -

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

@belleklaviyo belleklaviyo force-pushed the bl/sample-app-geofencing branch from dc5502f to 2c58995 Compare September 25, 2025 12:42
@belleklaviyo belleklaviyo marked this pull request as ready for review September 25, 2025 12:43
@belleklaviyo belleklaviyo requested a review from a team as a code owner September 25, 2025 12:43
@ab1470
Copy link
Contributor

ab1470 commented Sep 26, 2025

Was all of the SwiftUI code AI-generated? Because smh its badddd

Copy link
Contributor

@ab1470 ab1470 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to take a little more time on this. It's great that AI was able to convert crufty old storyboard code into SwiftUI... but AI didn't do a great job of writing SwiftUI. While a sample app isn't exactly mission-critical and admittedly probably won't be seen by too many people, I do think it's important to put our best foot forward and write clean UI code, even if it's minimal.

It's not a priority right now because universal links is more pressing, but I think next week I can take a stab at proposing some ways to clean it up

@belleklaviyo
Copy link
Contributor Author

^^ lol agree -- also as we discussed, we probably want the overall sample app update to SwiftUI/removing the storyboard to not be behind this feature branch, so I'll plan to restructure those code changes to be targeting master and then this could be moreso focused on adding the map view/geofencing related updates

@github-actions
Copy link

This PR has not seen any updates in the last 16 days. Without further action this PR will be closed in 14 days. To disable further staleness checks add the evergreen label.

@github-actions github-actions bot added the stale label Oct 13, 2025
Copy link
Contributor

@ab1470 ab1470 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I've reviewed everything except for MapView.swift. I need a little break to work on other stuff but I'll get back to this and review MapView a bit later

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@belleklaviyo belleklaviyo mentioned this pull request Nov 3, 2025
9 tasks
Copy link
Contributor

@ab1470 ab1470 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing all of my comments. This is much better already. I left some other suggestions for improving this a bit more, particularly where it pertains to iOS 26 compatibility.

Also see my suggestion PR for some changes to the MapView.

Copy link
Contributor

@ab1470 ab1470 Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the entire screen before/after all of the changes I suggested in this file:

BeforeAfter
Image Image

ab1470 and others added 2 commits November 10, 2025 10:31
* changed `NavigationView` to `NavigationStack`

`NavigationView` is deprecated

* added .ignoresSafeArea and background for toolbars.

this makes the map fill is parent container. The toolbar background ensures that the buttons are visible above the map on pre-iOS-26 devices

* changed Close button to show an image instead of text

* Changed "Stop" button to show an icon instead of text

this resolves an issue on iOS 26 where the "Stop" button text was getting truncated

* consolidated styles into `locationStatusLabel` tuple

* minor formatting fix

* added `geofenceMonitoringLabel` and revised geofence monitoring indicator style

this fixes an issue on iOS 26+ where the monitoring label was getting truncated

* removed geofencing monitoring label from bottom toolbar and placed it in navigation bar

* moved all controls into a menu button at the top of the screen

* moved monitoring buttons to bottom bar

---------

Co-authored-by: Belle Lim <[email protected]>
@belleklaviyo belleklaviyo requested a review from ab1470 November 10, 2025 16:44
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Menu Item Identity Crisis Breaks Cart

The id for "Beef Bolognese" is set to 5 but should be 4 based on the menu initialization in MenuPageViewController and MenuView. This mismatch causes cart restoration to create items with incorrect IDs.

Examples/KlaviyoSwiftExamples/Shared/Cart.swift#L62-L63

case "Beef Bolognese":
cartItems.append(

Fix in Cursor Fix in Web


Copy link
Contributor

@ab1470 ab1470 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋👋 storyboards! Can't say we'll miss you

Nice job, feels good to have a modern sample app!

@belleklaviyo belleklaviyo merged commit 014c286 into feat/geofencing Nov 10, 2025
6 checks passed
@belleklaviyo belleklaviyo deleted the bl/sample-app-geofencing branch November 10, 2025 17:10
@ab1470
Copy link
Contributor

ab1470 commented Nov 10, 2025

Bug: Menu Item Identity Crisis Breaks Cart

The id for "Beef Bolognese" is set to 5 but should be 4 based on the menu initialization in MenuPageViewController and MenuView. This mismatch causes cart restoration to create items with incorrect IDs.

Cursor's got beef with your beef 🐄

@ajaysubra
Copy link
Contributor

👋👋 storyboards! Can't say we'll miss you

Nice job, feels good to have a modern sample app!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants