From ce4bf5a76653443a9d8bfe926ee7509c9ab840e2 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Wed, 18 May 2022 18:57:46 +0300 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/25281 - Allow the Pocket section to settle before reporting impressions Currently just a hack that will ensure more reliable data. This will have to be re-evaluated after the homescreen is migrated to compose. --- .../fenix/home/pocket/PocketStoriesComposables.kt | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesComposables.kt b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesComposables.kt index 386a3cf22..9dde2c579 100644 --- a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesComposables.kt +++ b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesComposables.kt @@ -76,6 +76,14 @@ private const val URI_PARAM_UTM_KEY = "utm_source" private const val POCKET_STORIES_UTM_VALUE = "pocket-newtab-android" private const val POCKET_FEATURE_UTM_KEY_VALUE = "utm_source=ff_android" +/** + * The Pocket section may appear first on the homescreen and be fully constructed + * to then be pushed downwards when other elements appear. + * This can lead to overcounting impressions with multiple such events being possible + * without the user actually having time to see the stories or scrolling to see the Pocket section. + */ +private const val MINIMUM_TIME_TO_SETTLE_MS = 1000 + /** * Placeholder [PocketStory] allowing to combine other items in the same list that shows stories. * It uses empty values for it's properties ensuring that no conflict is possible since real stories have @@ -264,12 +272,16 @@ private fun Modifier.onShown( @FloatRange(from = 0.0, to = 1.0) threshold: Float, onVisible: () -> Unit, ): Modifier { + val initialTime = System.currentTimeMillis() + return composed { val context = LocalContext.current var wasEventReported by remember { mutableStateOf(false) } onGloballyPositioned { coordinates -> - if (!wasEventReported && coordinates.isVisible(context, threshold)) { + if (!wasEventReported && coordinates.isVisible(context, threshold) && + System.currentTimeMillis() - initialTime > MINIMUM_TIME_TO_SETTLE_MS + ) { wasEventReported = true onVisible() }