Merge pull request #20861 from wordpress-mobile/issue/20621-tags-feed…

…-onboarding-improvements [Reader] Implement announcement card improvements
wordpress-mobile · May 22, 2024 · 5644c2b · 5644c2b
2 parents 5dde4db + 9d73d4a
commit 5644c2b
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 57 deletions.
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/discover/ReaderDiscoverViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/discover/ReaderDiscoverViewModel.kt
@@ -154,11 +154,13 @@ class ReaderDiscoverViewModel @Inject constructor(
                 // since new users have the dailyprompt tag followed by default, we need to ignore them when
                 // checking if the user has any tags followed, so we show the onboarding state (ShowNoFollowedTags)
                 if (userTags.filterNot { it.tagSlug == BLOGGING_PROMPT_TAG }.isEmpty()) {
+                    parentViewModel.onFeedEmptyStateLoaded()
                     _uiState.value = DiscoverUiState.EmptyUiState.ShowNoFollowedTagsUiState {
                         parentViewModel.onShowReaderInterests()
                     }
                 } else {
                     if (posts != null && posts.cards.isNotEmpty()) {
+                        parentViewModel.onFeedContentLoaded()
                         _uiState.value = DiscoverUiState.ContentUiState(
                             convertCardsToUiStates(posts),
                             reloadProgressVisibility = false,
@@ -169,6 +171,7 @@ class ReaderDiscoverViewModel @Inject constructor(
                             swipeToRefreshTriggered = false
                         }
                     } else {
+                        parentViewModel.onFeedEmptyStateLoaded()
                         _uiState.value = DiscoverUiState.EmptyUiState.ShowNoPostsUiState {
                             _navigationEvents.value = Event(ShowReaderSubs)
                         }

diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderViewModel.kt
@@ -132,7 +132,6 @@ class ReaderViewModel @Inject constructor(
         if (initialized) return
         loadTabs(savedInstanceState)
         if (jetpackBrandingUtils.shouldShowJetpackPoweredBottomSheet()) showJetpackPoweredBottomSheet()
-        updateAnnouncementCard()
     }
 
     fun onSaveInstanceState(out: Bundle) {
@@ -142,6 +141,20 @@ class ReaderViewModel @Inject constructor(
         }
     }
 
+    fun onFeedEmptyStateLoaded() {
+        hideAnnouncementCard()
+    }
+
+    fun onFeedContentLoaded() {
+        updateAnnouncementCard()
+    }
+
+    private fun hideAnnouncementCard() {
+        _announcementCardState.value = _announcementCardState.value?.copy(
+            shouldShow = false,
+        )
+    }
+
     private fun showJetpackPoweredBottomSheet() {
 //        _showJetpackPoweredBottomSheet.value = Event(true)
     }
@@ -166,9 +179,9 @@ class ReaderViewModel @Inject constructor(
                 descriptionRes = R.string.reader_announcement_card_reading_preferences_description,
             )
         )
-
+        val isDiscoverSelected = selectedReaderTag()?.isDiscover == true
         _announcementCardState.value = AnnouncementCardUiState(
-            shouldShow = readerAnnouncementCardFeatureConfig.isEnabled() &&
+            shouldShow = isDiscoverSelected && readerAnnouncementCardFeatureConfig.isEnabled() &&
                     appPrefsWrapper.shouldShowReaderAnnouncementCard(),
             items = items,
         )
@@ -199,6 +212,9 @@ class ReaderViewModel @Inject constructor(
     }
 
     fun onTagChanged(selectedTag: ReaderTag?) {
+        if (selectedTag?.isDiscover == false) {
+            hideAnnouncementCard()
+        }
         selectedTag?.let {
             trackReaderTabShownIfNecessary(it)
         }

diff --git a/WordPress/src/main/res/layout/reader_fragment_layout.xml b/WordPress/src/main/res/layout/reader_fragment_layout.xml
@@ -16,47 +16,26 @@
             android:layout_height="wrap_content"
             app:layout_scrollFlags="scroll|enterAlways" />
 
-    </com.google.android.material.appbar.AppBarLayout>
-
-    <androidx.constraintlayout.widget.ConstraintLayout
-        android:layout_width="match_parent"
-        android:layout_height="match_parent"
-        android:paddingBottom="56dp"
-        app:layout_behavior="@string/appbar_scrolling_view_behavior">
-
-        <FrameLayout
-            android:id="@+id/container"
-            android:layout_width="0dp"
-            android:layout_height="0dp"
-            app:layout_constraintEnd_toEndOf="parent"
-            app:layout_constraintStart_toStartOf="parent"
-            app:layout_constraintBottom_toBottomOf="parent"
-            app:layout_constraintTop_toBottomOf="@+id/reader_announcement_card_compose_view" />
-
         <androidx.compose.ui.platform.ComposeView
             android:id="@+id/reader_announcement_card_compose_view"
-            android:layout_width="0dp"
+            android:layout_width="match_parent"
             android:layout_height="wrap_content"
-            app:layout_constraintEnd_toEndOf="parent"
-            app:layout_constraintStart_toStartOf="parent"
-            app:layout_constraintTop_toTopOf="parent" />
+            app:layout_scrollFlags="scroll|enterAlways" />
 
-        <FrameLayout
-            android:id="@+id/interests_fragment_container"
-            android:layout_width="0dp"
-            android:layout_height="0dp"
-            app:layout_constraintBottom_toBottomOf="parent"
-            app:layout_constraintEnd_toEndOf="parent"
-            app:layout_constraintStart_toStartOf="parent"
-            app:layout_constraintTop_toTopOf="parent" />
+    </com.google.android.material.appbar.AppBarLayout>
 
-        <include
-            android:id="@+id/jetpack_banner"
-            layout="@layout/jetpack_banner"
-            app:layout_constraintBottom_toBottomOf="parent"
-            app:layout_constraintEnd_toEndOf="parent"
-            app:layout_constraintStart_toStartOf="parent" />
+    <FrameLayout
+        android:id="@+id/container"
+        android:layout_width="match_parent"
+        android:layout_height="match_parent"
+        app:layout_behavior="@string/appbar_scrolling_view_behavior"/>
 
-    </androidx.constraintlayout.widget.ConstraintLayout>
+    <include
+        android:id="@+id/jetpack_banner"
+        layout="@layout/jetpack_banner" />
 
+    <FrameLayout
+        android:id="@+id/interests_fragment_container"
+        android:layout_width="match_parent"
+        android:layout_height="match_parent" />
 </androidx.coordinatorlayout.widget.CoordinatorLayout>
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderViewModelTest.kt
@@ -510,10 +510,10 @@ class ReaderViewModelTest : BaseUnitTest() {
     }
 
     @Test
-    fun `Should load announcement card correctly with tags item`() = testWithNonEmptyTags {
+    fun `Should update announcement card UI correctly with tags item`() = testWithNonEmptyTags {
         whenever(readerTagsFeedFeatureConfig.isEnabled()).thenReturn(true)
 
-        triggerContentDisplay()
+        viewModel.onFeedContentLoaded()
         val observers = initObservers()
 
         val announcementCardUiState = observers.announcementCardStateEvents.first()
@@ -536,10 +536,10 @@ class ReaderViewModelTest : BaseUnitTest() {
     }
 
     @Test
-    fun `Should load announcement card correctly without tags item`() = testWithNonEmptyTags {
+    fun `Should update announcement card UI correctly without tags item onFeedContentLoaded`() = testWithNonEmptyTags {
         whenever(readerTagsFeedFeatureConfig.isEnabled()).thenReturn(false)
 
-        triggerContentDisplay()
+        viewModel.onFeedContentLoaded()
         val observers = initObservers()
 
         val announcementCardUiState = observers.announcementCardStateEvents.first()
@@ -557,32 +557,41 @@ class ReaderViewModelTest : BaseUnitTest() {
     }
 
     @Test
-    fun `Should show announcement card if feature flag is enabled and app preference returns true`() =
+    fun `Should show announcement card if feature flag is enabled, app preference returns true and feed is Discover`() =
         testWithNonEmptyTags {
-        whenever(readerAnnouncementCardFeatureConfig.isEnabled()).thenReturn(true)
-        whenever(appPrefsWrapper.shouldShowReaderAnnouncementCard()).thenReturn(true)
-        triggerContentDisplay()
-        val observers = initObservers()
+            val readerTag = ReaderTag("Discover", "Discover", "Discover", DISCOVER_PATH, ReaderTagType.DEFAULT)
+            whenever(readerAnnouncementCardFeatureConfig.isEnabled()).thenReturn(true)
+            whenever(appPrefsWrapper.shouldShowReaderAnnouncementCard()).thenReturn(true)
+            val observers = initObservers()
+            triggerContentDisplay()
+            viewModel.updateSelectedContent(readerTag)
+            viewModel.onFeedContentLoaded()
 
-        val announcementCardUiState = observers.announcementCardStateEvents.first()
-        assertTrue(announcementCardUiState.shouldShow)
-    }
+            val announcementCardUiState = observers.announcementCardStateEvents.first()
+            assertTrue(announcementCardUiState.shouldShow)
+        }
 
     @Test
     fun `Should NOT show announcement card if feature flag is disabled`() = testWithNonEmptyTags {
-            whenever(readerAnnouncementCardFeatureConfig.isEnabled()).thenReturn(false)
-            triggerContentDisplay()
-            val observers = initObservers()
+        val readerTag = ReaderTag("Discover", "Discover", "Discover", DISCOVER_PATH, ReaderTagType.DEFAULT)
+        whenever(readerAnnouncementCardFeatureConfig.isEnabled()).thenReturn(false)
+        val observers = initObservers()
+        triggerContentDisplay()
+        viewModel.updateSelectedContent(readerTag)
+        viewModel.onFeedContentLoaded()
 
-            val announcementCardUiState = observers.announcementCardStateEvents.first()
-            assertFalse(announcementCardUiState.shouldShow)
-        }
+        val announcementCardUiState = observers.announcementCardStateEvents.first()
+        assertFalse(announcementCardUiState.shouldShow)
+    }
 
     @Test
     fun `Should NOT show announcement card if app preference returns false`() = testWithNonEmptyTags {
+        val readerTag = ReaderTag("Discover", "Discover", "Discover", DISCOVER_PATH, ReaderTagType.DEFAULT)
         whenever(readerAnnouncementCardFeatureConfig.isEnabled()).thenReturn(true)
         whenever(appPrefsWrapper.shouldShowReaderAnnouncementCard()).thenReturn(false)
         triggerContentDisplay()
+        viewModel.updateSelectedContent(readerTag)
+        viewModel.onFeedContentLoaded()
         val observers = initObservers()
 
         val announcementCardUiState = observers.announcementCardStateEvents.first()