Repository navigation
Conversation
turnipdabeets
left a comment
turnipdabeets
left a comment
There was a problem hiding this comment.
Thanks @o-mid , a couple comments below.
9f9b7f1 to
a8ba5a3
Compare
Wire the appearance field through native maps and Dart models, wait in SurveyService with cancel/replace semantics, and add widget tests. Fixes PostHog#322
Wait out the delay on the same pending-show completer the context wait uses, so a survey with no context after its delay waits for the next navigation instead of being closed, and hideSurvey or a newer survey cancels either wait. A navigation during the delay no longer cuts it short. Cap the delay at one day so huge values can't overflow, drop the redundant generation counter, and make the changeset a minor. Co-authored-by: Omid Mirzaei <omidomirzaei@gmail.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
a8ba5a3 to
4c8283d
Compare
Don't call onClosed for a survey hidden or replaced during its popup delay. The user never saw it, so it must not be marked seen or send `survey dismissed`, matching the wait-for-context path and the Android surveys host. Native has already dropped that presentation by the time Flutter cancels the delay, so nothing is left waiting for a close. Make the replacement test fail if the replaced survey's timer keeps running and shows the newer survey early. Co-authored-by: Omid Mirzaei <omidomirzaei@gmail.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Thanks @o-mid! I rebased this onto main and pushed a couple of follow-up commits to address the review. @PostHog/team-client-libraries mind giving this a quick look before merging? |
dustinbyrne
left a comment
dustinbyrne
left a comment
There was a problem hiding this comment.
AI-assisted review.
One non-blocking timing-policy note inline: repeated matching Android events restart the pending delay, while the SDK spec does not define first-event versus last-event timing. This is a clarification, not a spec violation.
|
Thanks for the reviews, @turnipdabeets and @dustinbyrne — the follow-up commits on the branch should cover the earlier feedback (minor changeset, delay cap, gate check, test timing, no I merged Ready whenever someone with merge access wants to stamp it. |
This branch has not been deployed
3 participants
Summary
surveyPopupDelaySecondsfrom Android/iOS survey payloads intoPostHogDisplaySurveyAppearance.SurveyService.showSurveywaits before opening the bottom sheet; zero, negative, or missing values still show immediately (same rule as native).hideSurvey) or triggering another one during the wait drops the pending survey without callingonClosed, so a survey the user never saw isn't marked seen or dismissed.Fixes #322
Test plan
flutter test test/survey_popup_delay_test.dartflutter test test/surveys_test.dart test/survey_resume_test.dart