Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix textStoryToggle visibility when hasSelectedMedia #13948

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+12 −5
Open

Fix textStoryToggle visibility when hasSelectedMedia #13948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/MediaSelectionActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@ class MediaSelectionActivity :
textViewModel.restoreFromInstanceState(savedInstanceState)
}

viewModel.hasSelectedMedia.observe(this) { hasSelectedMedia->
if(hasSelectedMedia) {
textStoryToggle.visible = false
} else {
textStoryToggle.visible = canDisplayStorySwitch()
}
}

(supportFragmentManager.findFragmentByTag(NAV_HOST_TAG) as NavHostFragment).navController.addOnDestinationChangedListener { _, d, _ ->
when (d.id) {
R.id.mediaCaptureFragment -> {
Expand Down Expand Up @@ -233,7 +241,6 @@ class MediaSelectionActivity :
private fun canDisplayStorySwitch(): Boolean {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is called from multiple places, and now those call sites no longer take into account whether there's selected media. Are those call sites still necesessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they are still required. We have to hide on DestinationChange(navigating to preview or gallery), and when we are back to MediaCapture, even when no media was selected, we have to update it back to visible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, this might create a problem, as now there are two sources for textStoryToggle visibility state(hasSelectedMedia and DestinationChangedListener) and it might create difficulty in testing and finding bugs. A better approach can be to move the logic to ViewModel and check the combinations there. If destinations are mediaCapture or textStore then handle the logic, else false. This way we will only expose a single LiveData of isTextStoryToggleVisible as a single source of truth.

return Stories.isFeatureEnabled() &&
isCameraFirst() &&
!viewModel.hasSelectedMedia() &&
(destination == MediaSelectionDestination.ChooseAfterMediaSelection || destination is MediaSelectionDestination.SingleStory)
}

Expand Down
8 changes: 4 additions & 4 deletions app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/MediaSelectionViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import android.os.Parcel
import androidx.lifecycle.LiveData
import androidx.lifecycle.ViewModel
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.distinctUntilChanged
import androidx.lifecycle.map
import com.google.common.io.ByteStreams
import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers
import io.reactivex.rxjava3.core.Flowable
Expand Down Expand Up @@ -80,6 +82,8 @@ class MediaSelectionViewModel(

val state: LiveData<MediaSelectionState> = store.stateLiveData

val hasSelectedMedia: LiveData<Boolean> = store.stateLiveData.map { it.selectedMedia.isNotEmpty() }.distinctUntilChanged()

private val internalHudCommands = PublishSubject.create<HudCommand>()

val mediaErrors: BehaviorSubject<MediaValidator.FilterError> = BehaviorSubject.createDefault(MediaValidator.FilterError.None)
Expand Down Expand Up @@ -469,10 +473,6 @@ class MediaSelectionViewModel(
}
}

fun hasSelectedMedia(): Boolean {
return store.state.selectedMedia.isNotEmpty()
}

fun clearMediaErrors() {
mediaErrors.onNext(MediaValidator.FilterError.None)
}
Expand Down