From 300de19e05a1a15ba7a15a1bda13af3793607cb9 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Thu, 26 May 2022 14:11:17 +0300 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/25281 - Improve stories impressions recording. This fixes the following two scenarios resulting in improper recordings: - sponsored stories were recorded as shown as part of the impressions recording for all the stories (both recommended and sponsored). Separated the two. - sponsored stories were not recorded as shown if the user doesn't scroll. This could happen if just toggling categories which would result in a new list of stories to show. Will recheck after the time to settle if the composable is shown. --- .../home/pocket/PocketStoriesComposables.kt | 25 ++++++++++++++++--- .../home/pocket/PocketStoriesViewHolder.kt | 4 ++- 2 files changed, 24 insertions(+), 5 deletions(-) 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 9ea66ab097..03dc09c76a 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 @@ -25,6 +25,7 @@ import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material.Icon import androidx.compose.material.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -50,6 +51,7 @@ import androidx.compose.ui.tooling.preview.PreviewParameterProvider import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp +import kotlinx.coroutines.delay import mozilla.components.service.pocket.PocketStory import mozilla.components.service.pocket.PocketStory.PocketRecommendedStory import mozilla.components.service.pocket.PocketStory.PocketSponsoredStory @@ -274,6 +276,7 @@ private fun Modifier.onShown( onVisible: () -> Unit, ): Modifier { val initialTime = System.currentTimeMillis() + var lastVisibleCoordinates: LayoutCoordinates? = null return composed { val context = LocalContext.current @@ -291,14 +294,28 @@ private fun Modifier.onShown( } } - onGloballyPositioned { coordinates -> - if (!wasEventReported && coordinates.isVisible(screenBounds, threshold) && - System.currentTimeMillis() - initialTime > MINIMUM_TIME_TO_SETTLE_MS - ) { + // In the event this composable starts as visible but then gets pushed offscreen + // before MINIMUM_TIME_TO_SETTLE_MS we will not report is as being visible. + // In the LaunchedEffect we add support for when the composable starts as visible and then + // it's position isn't changed after MINIMUM_TIME_TO_SETTLE_MS so it must be reported as visible. + LaunchedEffect(initialTime) { + delay(MINIMUM_TIME_TO_SETTLE_MS.toLong()) + if (!wasEventReported && lastVisibleCoordinates?.isVisible(screenBounds, threshold) == true) { wasEventReported = true onVisible() } } + + onGloballyPositioned { coordinates -> + if (!wasEventReported && coordinates.isVisible(screenBounds, threshold)) { + if (System.currentTimeMillis() - initialTime > MINIMUM_TIME_TO_SETTLE_MS) { + wasEventReported = true + onVisible() + } else { + lastVisibleCoordinates = coordinates + } + } + } } } diff --git a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt index a64050578b..d7e81a713d 100644 --- a/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/pocket/PocketStoriesViewHolder.kt @@ -59,7 +59,9 @@ class PocketStoriesViewHolder( // We should report back when a certain story is actually being displayed. // Cannot do it reliably so for now we'll just mass report everything as being displayed. stories?.let { - interactor.onStoriesShown(it) + // Only report here the impressions for recommended stories. + // Sponsored stories use a different API for more accurate tracking. + interactor.onStoriesShown(it.filterIsInstance()) } }