From 931b9f8831ad2faa498a41ab85dca0072820eb96 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Mon, 26 Sep 2022 14:20:50 -0300 Subject: [PATCH] Update stories jump logic to match spec. --- .../securesms/stories/StoryViewerArgs.kt | 3 +- .../stories/landing/StoriesLandingFragment.kt | 6 +- .../stories/viewer/StoryViewerFragment.kt | 19 ++++-- .../stories/viewer/StoryViewerPagerAdapter.kt | 9 +-- .../viewer/page/StoryViewerPageArgs.kt | 21 +++++++ .../viewer/page/StoryViewerPageFragment.kt | 63 +++++-------------- .../viewer/page/StoryViewerPageViewModel.kt | 26 ++++---- .../page/StoryViewerPageViewModelTest.kt | 31 +++++++-- 8 files changed, 96 insertions(+), 82 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageArgs.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/StoryViewerArgs.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/StoryViewerArgs.kt index 497cf74cca..abbdcfe582 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/stories/StoryViewerArgs.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/StoryViewerArgs.kt @@ -22,7 +22,8 @@ data class StoryViewerArgs( val groupReplyStartPosition: Int = -1, val isFromInfoContextMenuAction: Boolean = false, val isFromQuote: Boolean = false, - val isFromMyStories: Boolean = false + val isFromMyStories: Boolean = false, + val isJumpToUnviewed: Boolean = false ) : Parcelable { class Builder(private val recipientId: RecipientId, private val isInHiddenStoryMode: Boolean) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/landing/StoriesLandingFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/landing/StoriesLandingFragment.kt index bfbb54b66e..45d7c21595 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/stories/landing/StoriesLandingFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/landing/StoriesLandingFragment.kt @@ -33,6 +33,7 @@ import org.thoughtcrime.securesms.conversation.mutiselect.forward.MultiselectFor import org.thoughtcrime.securesms.conversation.mutiselect.forward.MultiselectForwardFragmentArgs import org.thoughtcrime.securesms.database.model.MediaMmsMessageRecord import org.thoughtcrime.securesms.database.model.MmsMessageRecord +import org.thoughtcrime.securesms.database.model.StoryViewState import org.thoughtcrime.securesms.main.Material3OnScrollHelperBinder import org.thoughtcrime.securesms.main.SearchBinder import org.thoughtcrime.securesms.mediasend.v2.MediaSelectionActivity @@ -306,8 +307,9 @@ class StoriesLandingFragment : DSLSettingsFragment(layoutId = R.layout.stories_l storyThumbTextModel = text, storyThumbUri = image, storyThumbBlur = blur, - recipientIds = viewModel.getRecipientIds(model.data.isHidden, false), - isFromInfoContextMenuAction = isFromInfoContextMenuAction + recipientIds = viewModel.getRecipientIds(model.data.isHidden, model.data.storyViewState == StoryViewState.UNVIEWED), + isFromInfoContextMenuAction = isFromInfoContextMenuAction, + isJumpToUnviewed = model.data.storyViewState == StoryViewState.UNVIEWED ) ), options.toBundle() diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/StoryViewerFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/StoryViewerFragment.kt index 8d96b16a40..f558c50796 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/StoryViewerFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/StoryViewerFragment.kt @@ -7,8 +7,10 @@ import androidx.fragment.app.viewModels import androidx.viewpager2.widget.ViewPager2 import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers import org.thoughtcrime.securesms.R +import org.thoughtcrime.securesms.recipients.Recipient import org.thoughtcrime.securesms.recipients.RecipientId import org.thoughtcrime.securesms.stories.StoryViewerArgs +import org.thoughtcrime.securesms.stories.viewer.page.StoryViewerPageArgs import org.thoughtcrime.securesms.stories.viewer.page.StoryViewerPageFragment import org.thoughtcrime.securesms.stories.viewer.reply.StoriesSharedElementCrossFaderView import org.thoughtcrime.securesms.util.LifecycleDisposable @@ -47,11 +49,18 @@ class StoryViewerFragment : val adapter = StoryViewerPagerAdapter( this, - storyViewerArgs.storyId, - storyViewerArgs.isFromNotification, - storyViewerArgs.groupReplyStartPosition, - storyViewerArgs.isFromMyStories, - storyViewerArgs.isFromInfoContextMenuAction + StoryViewerPageArgs( + recipientId = Recipient.UNKNOWN.id, + initialStoryId = storyViewerArgs.storyId, + isJumpForwardToUnviewed = storyViewerArgs.isJumpToUnviewed, + isOutgoingOnly = storyViewerArgs.isFromMyStories, + source = when { + storyViewerArgs.isFromInfoContextMenuAction -> StoryViewerPageArgs.Source.INFO_CONTEXT + storyViewerArgs.isFromNotification -> StoryViewerPageArgs.Source.NOTIFICATION + else -> StoryViewerPageArgs.Source.UNKNOWN + }, + groupReplyStartPosition = storyViewerArgs.groupReplyStartPosition + ) ) storyPager.adapter = adapter diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/StoryViewerPagerAdapter.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/StoryViewerPagerAdapter.kt index 60be4c6148..68660f6c8a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/StoryViewerPagerAdapter.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/StoryViewerPagerAdapter.kt @@ -5,15 +5,12 @@ import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.RecyclerView import androidx.viewpager2.adapter.FragmentStateAdapter import org.thoughtcrime.securesms.recipients.RecipientId +import org.thoughtcrime.securesms.stories.viewer.page.StoryViewerPageArgs import org.thoughtcrime.securesms.stories.viewer.page.StoryViewerPageFragment class StoryViewerPagerAdapter( fragment: Fragment, - private val initialStoryId: Long, - private val isFromNotification: Boolean, - private val groupReplyStartPosition: Int, - private val isOutgoingOnly: Boolean, - private val isFromInfoContextMenuAction: Boolean + private val arguments: StoryViewerPageArgs ) : FragmentStateAdapter(fragment) { private val pages: MutableList = mutableListOf() @@ -39,7 +36,7 @@ class StoryViewerPagerAdapter( } override fun createFragment(position: Int): Fragment { - return StoryViewerPageFragment.create(pages[position], initialStoryId, isFromNotification, groupReplyStartPosition, isOutgoingOnly, isFromInfoContextMenuAction) + return StoryViewerPageFragment.create(arguments.copy(recipientId = pages[position])) } private class Callback( diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageArgs.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageArgs.kt new file mode 100644 index 0000000000..5a78a7c38d --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageArgs.kt @@ -0,0 +1,21 @@ +package org.thoughtcrime.securesms.stories.viewer.page + +import android.os.Parcelable +import kotlinx.parcelize.Parcelize +import org.thoughtcrime.securesms.recipients.RecipientId + +@Parcelize +data class StoryViewerPageArgs( + val recipientId: RecipientId, + val initialStoryId: Long, + val isOutgoingOnly: Boolean, + val isJumpForwardToUnviewed: Boolean, + val source: Source, + val groupReplyStartPosition: Int +) : Parcelable { + enum class Source { + UNKNOWN, + NOTIFICATION, + INFO_CONTEXT + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageFragment.kt index dab460d68c..cb558c8b85 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageFragment.kt @@ -124,9 +124,7 @@ class StoryViewerPageFragment : private val viewModel: StoryViewerPageViewModel by viewModels( factoryProducer = { StoryViewerPageViewModel.Factory( - storyRecipientId, - initialStoryId, - isOutgoingOnly, + storyViewerPageArgs, StoryViewerPageRepository( requireContext() ), @@ -149,23 +147,7 @@ class StoryViewerPageFragment : private var sendingProgressDrawable: IndeterminateDrawable? = null - private val storyRecipientId: RecipientId - get() = requireArguments().getParcelable(ARG_STORY_RECIPIENT_ID)!! - - private val initialStoryId: Long - get() = requireArguments().getLong(ARG_STORY_ID, -1L) - - private val isFromNotification: Boolean - get() = requireArguments().getBoolean(ARG_IS_FROM_NOTIFICATION, false) - - private val groupReplyStartPosition: Int - get() = requireArguments().getInt(ARG_GROUP_REPLY_START_POSITION, -1) - - private val isOutgoingOnly: Boolean - get() = requireArguments().getBoolean(ARG_IS_OUTGOING_ONLY, false) - - private val isFromInfoContextMenuAction: Boolean - get() = requireArguments().getBoolean(ARG_IS_FROM_INFO_CONTEXT_MENU_ACTION, false) + private val storyViewerPageArgs: StoryViewerPageArgs by lazy(LazyThreadSafetyMode.NONE) { requireArguments().getParcelable(ARGS)!! } @SuppressLint("ClickableViewAccessibility") override fun onViewCreated(view: View, savedInstanceState: Bundle?) { @@ -357,7 +339,7 @@ class StoryViewerPageFragment : if (parentState.pages.size <= parentState.page) { viewModel.setIsSelectedPage(false) - } else if (storyRecipientId == parentState.pages[parentState.page]) { + } else if (storyViewerPageArgs.recipientId == parentState.pages[parentState.page]) { if (progressBar.segmentCount != 0) { progressBar.reset() progressBar.setPosition(viewModel.getRestartIndex()) @@ -412,16 +394,16 @@ class StoryViewerPageFragment : viewModel.setAreSegmentsInitialized(true) } else if (state.selectedPostIndex >= state.posts.size) { - callback.onFinishedPosts(storyRecipientId) + callback.onFinishedPosts(storyViewerPageArgs.recipientId) } else if (state.selectedPostIndex < 0) { - callback.onGoToPreviousStory(storyRecipientId) + callback.onGoToPreviousStory(storyViewerPageArgs.recipientId) } if (state.isDisplayingInitialState && !sharedViewModel.hasConsumedInitialState) { sharedViewModel.consumeInitialState() - if (isFromNotification) { - startReply(isFromNotification = true, groupReplyStartPosition = groupReplyStartPosition) - } else if (isFromInfoContextMenuAction && state.selectedPostIndex in state.posts.indices) { + if (storyViewerPageArgs.source == StoryViewerPageArgs.Source.NOTIFICATION) { + startReply(isFromNotification = true, groupReplyStartPosition = storyViewerPageArgs.groupReplyStartPosition) + } else if (storyViewerPageArgs.source == StoryViewerPageArgs.Source.INFO_CONTEXT && state.selectedPostIndex in state.posts.indices) { showInfo(state.posts[state.selectedPostIndex]) } } @@ -1060,13 +1042,13 @@ class StoryViewerPageFragment : } }, onGoToChat = { - startActivity(ConversationIntents.createBuilder(requireContext(), storyRecipientId, -1L).build()) + startActivity(ConversationIntents.createBuilder(requireContext(), storyViewerPageArgs.recipientId, -1L).build()) }, onHide = { viewModel.setIsDisplayingHideDialog(true) - StoryDialogs.hideStory(requireContext(), Recipient.resolved(storyRecipientId).getDisplayName(requireContext()), { viewModel.setIsDisplayingHideDialog(true) }) { + StoryDialogs.hideStory(requireContext(), Recipient.resolved(storyViewerPageArgs.recipientId).getDisplayName(requireContext()), { viewModel.setIsDisplayingHideDialog(true) }) { lifecycleDisposable += viewModel.hideStory().subscribe { - callback.onStoryHidden(storyRecipientId) + callback.onStoryHidden(storyViewerPageArgs.recipientId) } } }, @@ -1099,29 +1081,12 @@ class StoryViewerPageFragment : private val CHARACTERS_PER_SECOND = 15L private val DEFAULT_DURATION = TimeUnit.SECONDS.toMillis(5) - private const val ARG_STORY_RECIPIENT_ID = "arg.story.recipient.id" - private const val ARG_STORY_ID = "arg.story.id" - private const val ARG_IS_FROM_NOTIFICATION = "is_from_notification" - private const val ARG_GROUP_REPLY_START_POSITION = "group_reply_start_position" - private const val ARG_IS_OUTGOING_ONLY = "is_outgoing_only" - private const val ARG_IS_FROM_INFO_CONTEXT_MENU_ACTION = "is_from_info_context_menu_action" + private const val ARGS = "args" - fun create( - recipientId: RecipientId, - initialStoryId: Long, - isFromNotification: Boolean, - groupReplyStartPosition: Int, - isOutgoingOnly: Boolean, - isFromInfoContextMenuAction: Boolean - ): Fragment { + fun create(args: StoryViewerPageArgs): Fragment { return StoryViewerPageFragment().apply { arguments = bundleOf( - ARG_STORY_RECIPIENT_ID to recipientId, - ARG_STORY_ID to initialStoryId, - ARG_IS_FROM_NOTIFICATION to isFromNotification, - ARG_GROUP_REPLY_START_POSITION to groupReplyStartPosition, - ARG_IS_OUTGOING_ONLY to isOutgoingOnly, - ARG_IS_FROM_INFO_CONTEXT_MENU_ACTION to isFromInfoContextMenuAction, + ARGS to args ) } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageViewModel.kt index 2aed5a59ca..6b0136dc08 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageViewModel.kt @@ -25,9 +25,7 @@ import kotlin.math.min * Encapsulates presentation logic for displaying a collection of posts from a given user's story */ class StoryViewerPageViewModel( - private val recipientId: RecipientId, - private val initialStoryId: Long, - private val isOutgoingOnly: Boolean, + private val args: StoryViewerPageArgs, private val repository: StoryViewerPageRepository, val storyCache: StoryCache ) : ViewModel() { @@ -63,11 +61,11 @@ class StoryViewerPageViewModel( fun refresh() { disposables.clear() - disposables += repository.getStoryPostsFor(recipientId, isOutgoingOnly).subscribe { posts -> + disposables += repository.getStoryPostsFor(args.recipientId, args.isOutgoingOnly).subscribe { posts -> store.update { state -> val isDisplayingInitialState = state.posts.isEmpty() && posts.isNotEmpty() - val startIndex = if (state.posts.isEmpty() && initialStoryId > 0) { - val initialIndex = posts.indexOfFirst { it.id == initialStoryId } + val startIndex = if (state.posts.isEmpty() && args.initialStoryId > 0) { + val initialIndex = posts.indexOfFirst { it.id == args.initialStoryId } initialIndex.takeIf { it > -1 } ?: state.selectedPostIndex } else if (state.posts.isEmpty()) { val initialPost = getNextUnreadPost(posts) @@ -104,7 +102,7 @@ class StoryViewerPageViewModel( } fun hideStory(): Completable { - return repository.hideStory(recipientId) + return repository.hideStory(args.recipientId) } fun markViewed(storyPost: StoryPost) { @@ -133,10 +131,10 @@ class StoryViewerPageViewModel( val postIndex = store.state.selectedPostIndex val nextUnreadPost: StoryPost? = getNextUnreadPost(store.state.posts.drop(postIndex + 1)) - if (nextUnreadPost == null) { - setSelectedPostIndex(postIndex + 1) - } else { - setSelectedPostIndex(store.state.posts.indexOf(nextUnreadPost)) + when { + nextUnreadPost == null && args.isJumpForwardToUnviewed -> setSelectedPostIndex(store.state.posts.size) + nextUnreadPost == null -> setSelectedPostIndex(postIndex + 1) + else -> setSelectedPostIndex(store.state.posts.indexOf(nextUnreadPost)) } } @@ -311,14 +309,12 @@ class StoryViewerPageViewModel( } class Factory( - private val recipientId: RecipientId, - private val initialStoryId: Long, - private val isOutgoingOnly: Boolean, + private val args: StoryViewerPageArgs, private val repository: StoryViewerPageRepository, private val storyCache: StoryCache ) : ViewModelProvider.Factory { override fun create(modelClass: Class): T { - return modelClass.cast(StoryViewerPageViewModel(recipientId, initialStoryId, isOutgoingOnly, repository, storyCache)) as T + return modelClass.cast(StoryViewerPageViewModel(args, repository, storyCache)) as T } } } diff --git a/app/src/test/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageViewModelTest.kt b/app/src/test/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageViewModelTest.kt index 7b234e0474..3282640e79 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageViewModelTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/stories/viewer/page/StoryViewerPageViewModelTest.kt @@ -145,6 +145,24 @@ class StoryViewerPageViewModelTest { testSubscriber.assertValueAt(0) { it.selectedPostIndex == 2 } } + @Test + fun `Given no unread and jump to next unread enabled, when I goToNext, then I expect storyIndex to be size`() { + // GIVEN + val storyPosts = createStoryPosts(3) { true } + whenever(repository.getStoryPostsFor(any(), any())).thenReturn(Observable.just(storyPosts)) + + // WHEN + val testSubject = createTestSubject(isJumpForwardToUnviewed = true) + testScheduler.triggerActions() + testSubject.goToNextPost() + testScheduler.triggerActions() + + // THEN + val testSubscriber = testSubject.state.test() + + testSubscriber.assertValueAt(0) { it.selectedPostIndex == 3 } + } + @Test fun `Given a single story, when I goToPrevious, then I expect storyIndex to be -1`() { // GIVEN @@ -163,11 +181,16 @@ class StoryViewerPageViewModelTest { testSubscriber.assertValueAt(0) { it.selectedPostIndex == -1 } } - private fun createTestSubject(): StoryViewerPageViewModel { + private fun createTestSubject(isJumpForwardToUnviewed: Boolean = false): StoryViewerPageViewModel { return StoryViewerPageViewModel( - RecipientId.from(1), - -1L, - false, + StoryViewerPageArgs( + recipientId = RecipientId.from(1), + initialStoryId = -1L, + isOutgoingOnly = false, + isJumpForwardToUnviewed = isJumpForwardToUnviewed, + source = StoryViewerPageArgs.Source.UNKNOWN, + groupReplyStartPosition = -1 + ), repository, mock() )