Skip to content
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

feat: add base survey integration #314

Open
wants to merge 8 commits into
base: feat/mobile-surveys
Choose a base branch
from

Conversation

ioannisj
Copy link
Contributor

@ioannisj ioannisj commented Mar 10, 2025

💡 Motivation and Context

  • Added base logic for survey integration, including flag-based/internal targeting and device type checks for display conditions.

  • Added some draft/test implementation on presentation logic.

    • iOS: Surveys are displayed via a separate passthrough window rather than the root view controller of the host app. This avoids conflicts with existing modal presentations in the host app. We do all our work in a separate context without affecting or introducing any side effects to the app's root view controller.
    • SwiftUI: Some limitations since Apple introduced the majority of bottom sheet functionality in SwiftUI with iOS 16+. Will probably need to dive into UIKit world for proper display here.
  • No event trigger handling yet—considering a simple surveyIntegration.onEventCaptured() call for now. Ideally, though, internal delegates or handlers could provide better decoupling and would also enable us exposing these to the public API in the near future, something which is already requested here

  • Public API is still draft

#skip-changelog

TODO:

  • Feature flags & property targeting
  • Device type targeting
  • Event triggers
  • All question types
  • Multi-question surveys
  • Confirmation message
  • Appearance
  • Appearance - Custom theme
  • Surveys play well with session replay
    - user inputs are masked
    - surveys are captured

💚 How did you test it?

Unit tests

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

ioannisj added 4 commits March 5, 2025 13:14
# Conflicts:
#	CHANGELOG.md
#	Makefile
#	PostHog.podspec
#	PostHog.xcodeproj/project.pbxproj
#	PostHog/PostHogApi.swift
#	PostHog/PostHogSDK.swift
#	PostHog/PostHogVersion.swift
#	PostHogTests/PostHogFeatureFlagsTest.swift
#	PostHogTests/PostHogSessionManagerTest.swift
#	PostHogTests/TestUtils/MockApplicationLifecyclePublisher.swift
#	PostHogTests/TestUtils/TestPostHog.swift
@@ -59,11 +59,11 @@ import Foundation
@objc public let sessionReplayConfig: PostHogSessionReplayConfig = .init()
#endif

#if os(iOS)
#if os(iOS) || TESTING
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Public api is still draft here. Just added TESTING so that tests can run on CI as well for now

#else
properties["$is_emulator"] = false
#endif
if let deviceType = PostHogContext.deviceType {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to extract deviceType logic here since it was needed for surveys as well. Cleaned up/refactored a bit PostHogContext

@@ -37,7 +37,7 @@ let maxRetryDelay = 30.0

private var queue: PostHogQueue?
private var replayQueue: PostHogQueue?
private var storage: PostHogStorage?
private(set) var storage: PostHogStorage?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to internal visibility since it will be used by surveys to check survey seen info

return postHog.isFeatureEnabled(flagKey)
}

private func canRenderSurvey(survey _: Survey) -> Bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Temp. Not called yet

Comment on lines +250 to +269
private func setActiveSurvey(survey: Survey) {
activeSurveyLock.withLock {
if let activeSurvey {
hedgeLog("Survey \(activeSurvey) already in focus. Cannot add survey \(survey).")
return
}
activeSurvey = survey
}
}

/// Removes given survey as active survey
private func removeActiveSurvey(survey: Survey) {
activeSurveyLock.withLock {
guard activeSurvey?.id != survey.id else {
hedgeLog("Survey \(survey) is not in focus. Cannot remove survey \(survey)")
return
}
activeSurvey = nil
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Temp. Not called yet

return matchType.matches(targets: deviceTypes, value: deviceType)
}

private func getNextSurveyStep(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Temp. Not called yet

@ioannisj ioannisj marked this pull request as ready for review March 10, 2025 07:41
@ioannisj ioannisj requested a review from marandaneto as a code owner March 10, 2025 07:41
@ioannisj ioannisj requested a review from a team March 10, 2025 07:41
@ioannisj ioannisj changed the title feat: add base display logic feat: add base survey integration Mar 10, 2025
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.

1 participant