Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
posthog-ios Compliance ReportDate: 2026-04-07 17:18:14 UTC ✅ All Tests Passed!0/0 tests passed |
adboio
force-pushed
the
03-10-feat_surveys_support_survey_popup_delay
branch
2 times, most recently
from
8caa460 to
eaaf956
Compare
adboio
force-pushed
the
03-10-feat_surveys_support_event_property_filters
branch
from
a334421 to
81c5433
Compare
adboio
changed the base branch from
03-10-feat_surveys_support_event_property_filters
to
graphite-base/520
adboio
force-pushed
the
graphite-base/520
branch
from
81c5433 to
a9366ad
Compare
adboio
force-pushed
the
03-10-feat_surveys_support_survey_popup_delay
branch
from
eaaf956 to
b7c6843
Compare
adboio
force-pushed
the
03-10-feat_surveys_support_survey_popup_delay
branch
from
b7c6843 to
268e6e5
Compare
adboio
force-pushed
the
03-10-feat_surveys_support_survey_popup_delay
branch
from
268e6e5 to
9e7acb3
Compare
| ] | ||
|
|
||
| let sorted = surveys.sorted { | ||
| ($0.appearance?.surveyPopupDelaySeconds ?? 0) < ($1.appearance?.surveyPopupDelaySeconds ?? 0) |
There was a problem hiding this comment.
nit: I think this could be flaky cause .sorted is not guaranteed to be stable for equal keys I think. Maybe add tie-breaker as well or remove the scenario where there are equal keys (no-delay and delayed-0)
Also, this just duplicates production code, so if we change sorting at some point, this will still pass. I think we just need to test what getActiveMatchingSurveys returns instead
There was a problem hiding this comment.
On first point above, actually it seems that it's guaranteed, so never mind
https://developer.apple.com/documentation/swift/array/sorted()
The sorting algorithm is guaranteed to be stable. A stable sort preserves the relative order of elements that compare as equal.
| let delay = survey.appearance?.surveyPopupDelaySeconds ?? 0 | ||
| let deadline: DispatchTime = delay > 0 ? .now() + delay : .now() |
There was a problem hiding this comment.
I think this may be a bit unecessary and just adds noise. We could just do this?
| let delay = survey.appearance?.surveyPopupDelaySeconds ?? 0 | |
| let deadline: DispatchTime = delay > 0 ? .now() + delay : .now() | |
| let delay = max(survey.appearance?.surveyPopupDelaySeconds ?? 0, 0) | |
| let deadline: DispatchTime = .now() + delay |
| let sortedSurveys = activeSurveys.sorted { | ||
| ($0.appearance?.surveyPopupDelaySeconds ?? 0) < ($1.appearance?.surveyPopupDelaySeconds ?? 0) | ||
| } |
There was a problem hiding this comment.
Does web sdk behave similar to this? This feels like the wrong approach to me cause it changes the whole survey selection behavior, right? Now surveys with a smaller delay are given display priority, whereas before, selection was simply by first(where:) to a list returned by the server/config
I feel that the delay should actually be handled in the UI layer, in PostHogSurveysDefaultDelegate where the delegate will delay the presentation of a survey (and handle the dismissal scenario before delay like we do in L302) after the survey has been selected by getActiveMatchingSurveys()
There was a problem hiding this comment.
Yes, looks like js SDK is performing this delay logic in display layer in handlePopoverSurvey
3 participants
💡 Motivation and Context
ios does not support survey popup delay
closes #499
💚 How did you test it?
unit test / manual testing
📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset filereleaselabel to the PR