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.

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

refactor: Restructure ViewMediaActivity and related fragments #489

Merged
merged 1 commit into from
Mar 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 29 additions & 26 deletions app/src/main/java/app/pachli/ViewMediaActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ import app.pachli.core.navigation.AttachmentViewData
import app.pachli.core.navigation.ViewMediaActivityIntent
import app.pachli.core.navigation.ViewThreadActivityIntent
import app.pachli.databinding.ActivityViewMediaBinding
import app.pachli.fragment.ViewImageFragment
import app.pachli.fragment.ViewVideoFragment
import app.pachli.fragment.MediaActionsListener
import app.pachli.pager.ImagePagerAdapter
import app.pachli.pager.SingleImagePagerAdapter
import app.pachli.util.getTemporaryMediaFilename
Expand All @@ -76,7 +75,7 @@ typealias ToolbarVisibilityListener = (isVisible: Boolean) -> Unit
* Show one or more media items (pictures, video, audio, etc).
*/
@AndroidEntryPoint
class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener, ViewVideoFragment.VideoActionsListener {
class ViewMediaActivity : BaseActivity(), MediaActionsListener {
@Inject
lateinit var okHttpClient: OkHttpClient

Expand All @@ -88,15 +87,21 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
var isToolbarVisible = true
private set

private var attachments: List<AttachmentViewData>? = null
private var attachmentViewData: List<AttachmentViewData>? = null
private val toolbarVisibilityListeners = mutableListOf<ToolbarVisibilityListener>()
private var imageUrl: String? = null

/** True if a download to share media is in progress */
private var isDownloading: Boolean = false

/**
* Adds [listener] to the list of toolbar listeners and immediately calls
* it with the current toolbar visibility.
*
* @return A function that must be called to remove the listener.
*/
fun addToolbarVisibilityListener(listener: ToolbarVisibilityListener): Function0<Boolean> {
this.toolbarVisibilityListeners.add(listener)
toolbarVisibilityListeners.add(listener)
listener(isToolbarVisible)
return { toolbarVisibilityListeners.remove(listener) }
}
Expand All @@ -108,16 +113,16 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
supportPostponeEnterTransition()

// Gather the parameters.
attachments = ViewMediaActivityIntent.getAttachments(intent)
attachmentViewData = ViewMediaActivityIntent.getAttachments(intent)
val initialPosition = ViewMediaActivityIntent.getAttachmentIndex(intent)

// Adapter is actually of existential type PageAdapter & SharedElementsTransitionListener
// but it cannot be expressed and if I don't specify type explicitly compilation fails
// (probably a bug in compiler)
val adapter: ViewMediaAdapter = if (attachments != null) {
val realAttachs = attachments!!.map(AttachmentViewData::attachment)
val adapter: ViewMediaAdapter = if (attachmentViewData != null) {
val attachments = attachmentViewData!!.map(AttachmentViewData::attachment)
// Setup the view pager.
ImagePagerAdapter(this, realAttachs, initialPosition)
ImagePagerAdapter(this, attachments, initialPosition)
} else {
imageUrl = ViewMediaActivityIntent.getImageUrl(intent)
?: throw IllegalArgumentException("attachment list or image url has to be set")
Expand All @@ -137,12 +142,12 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener

// Setup the toolbar.
setSupportActionBar(binding.toolbar)
val actionBar = supportActionBar
if (actionBar != null) {
actionBar.setDisplayHomeAsUpEnabled(true)
actionBar.setDisplayShowHomeEnabled(true)
actionBar.title = getPageTitle(initialPosition)
supportActionBar?.apply {
setDisplayHomeAsUpEnabled(true)
setDisplayShowHomeEnabled(true)
title = getPageTitle(initialPosition)
}

binding.toolbar.setNavigationOnClickListener { supportFinishAfterTransition() }
binding.toolbar.setOnMenuItemClickListener { item: MenuItem ->
when (item.itemId) {
Expand Down Expand Up @@ -170,7 +175,7 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
override fun onCreateOptionsMenu(menu: Menu): Boolean {
menuInflater.inflate(R.menu.view_media_toolbar, menu)
// We don't support 'open status' from single image views
menu.findItem(R.id.action_open_status)?.isVisible = (attachments != null)
menu.findItem(R.id.action_open_status)?.isVisible = (attachmentViewData != null)
return true
}

Expand All @@ -179,15 +184,15 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
return true
}

override fun onBringUp() {
override fun onMediaReady() {
supportStartPostponedEnterTransition()
}

override fun onDismiss() {
override fun onMediaDismiss() {
supportFinishAfterTransition()
}

override fun onPhotoTap() {
override fun onMediaTap() {
isToolbarVisible = !isToolbarVisible
for (listener in toolbarVisibilityListeners) {
listener(isToolbarVisible)
Expand All @@ -214,14 +219,12 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
}

private fun getPageTitle(position: Int): CharSequence {
if (attachments == null) {
return ""
}
return String.format(Locale.getDefault(), "%d/%d", position + 1, attachments?.size)
attachmentViewData ?: return ""
return String.format(Locale.getDefault(), "%d/%d", position + 1, attachmentViewData?.size)
}

private fun downloadMedia() {
val url = imageUrl ?: attachments!![binding.viewPager.currentItem].attachment.url
val url = imageUrl ?: attachmentViewData!![binding.viewPager.currentItem].attachment.url
val filename = Uri.parse(url).lastPathSegment
Toast.makeText(applicationContext, resources.getString(R.string.download_image, filename), Toast.LENGTH_SHORT).show()

Expand All @@ -248,12 +251,12 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
}

private fun onOpenStatus() {
val attach = attachments!![binding.viewPager.currentItem]
val attach = attachmentViewData!![binding.viewPager.currentItem]
startActivityWithSlideInAnimation(ViewThreadActivityIntent(this, attach.statusId, attach.statusUrl))
}

private fun copyLink() {
val url = imageUrl ?: attachments!![binding.viewPager.currentItem].attachment.url
val url = imageUrl ?: attachmentViewData!![binding.viewPager.currentItem].attachment.url
val clipboard = getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager
clipboard.setPrimaryClip(ClipData.newPlainText(null, url))
}
Expand All @@ -268,7 +271,7 @@ class ViewMediaActivity : BaseActivity(), ViewImageFragment.PhotoActionsListener
if (imageUrl != null) {
shareMediaFile(directory, imageUrl!!)
} else {
val attachment = attachments!![binding.viewPager.currentItem].attachment
val attachment = attachmentViewData!![binding.viewPager.currentItem].attachment
shareMediaFile(directory, attachment.url)
}
}
Expand Down
69 changes: 20 additions & 49 deletions app/src/main/java/app/pachli/fragment/ViewImageFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package app.pachli.fragment
import android.animation.Animator
import android.animation.AnimatorListenerAdapter
import android.annotation.SuppressLint
import android.content.Context
import android.graphics.PointF
import android.graphics.drawable.Drawable
import android.os.Bundle
Expand All @@ -30,6 +29,7 @@ import android.view.View
import android.view.ViewGroup
import android.widget.ImageView
import androidx.core.view.GestureDetectorCompat
import androidx.lifecycle.lifecycleScope
import app.pachli.R
import app.pachli.ViewMediaActivity
import app.pachli.core.common.extensions.hide
Expand All @@ -43,35 +43,21 @@ import com.bumptech.glide.request.RequestListener
import com.bumptech.glide.request.target.Target
import com.ortiz.touchview.OnTouchCoordinatesListener
import com.ortiz.touchview.TouchImageView
import io.reactivex.rxjava3.subjects.BehaviorSubject
import kotlin.math.abs
import kotlinx.coroutines.launch

class ViewImageFragment : ViewMediaFragment() {
interface PhotoActionsListener {
fun onBringUp()
fun onDismiss()
fun onPhotoTap()
}

private val binding by viewBinding(FragmentViewImageBinding::bind)

private lateinit var photoActionsListener: PhotoActionsListener
private lateinit var toolbar: View
private var transition = BehaviorSubject.create<Unit>()

// Volatile: Image requests happen on background thread and we want to see updates to it
// immediately on another thread. Atomic is an overkill for such thing.
@Volatile
private var startedTransition = false

override fun onAttach(context: Context) {
super.onAttach(context)
photoActionsListener = context as PhotoActionsListener
}

override fun setupMediaView(
showingDescription: Boolean,
) {
override fun setupMediaView(showingDescription: Boolean) {
binding.photoView.transitionName = attachment.url
binding.mediaDescription.text = attachment.description
binding.captionSheet.visible(showingDescription)
Expand All @@ -82,7 +68,6 @@ class ViewImageFragment : ViewMediaFragment() {

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
toolbar = (requireActivity() as ViewMediaActivity).toolbar
this.transition = BehaviorSubject.create()
return inflater.inflate(R.layout.fragment_view_image, container, false)
}

Expand All @@ -95,7 +80,7 @@ class ViewImageFragment : ViewMediaFragment() {
object : GestureDetector.SimpleOnGestureListener() {
override fun onDown(e: MotionEvent) = true
override fun onSingleTapConfirmed(e: MotionEvent): Boolean {
photoActionsListener.onPhotoTap()
mediaActionsListener.onMediaTap()
return false
}
},
Expand Down Expand Up @@ -191,7 +176,7 @@ class ViewImageFragment : ViewMediaFragment() {
*/
private fun onGestureEnd(view: View) {
if (abs(view.translationY) > 180) {
photoActionsListener.onDismiss()
mediaActionsListener.onMediaDismiss()
} else {
view.animate().translationY(0f).scaleX(1f).scaleY(1f).start()
}
Expand All @@ -200,11 +185,6 @@ class ViewImageFragment : ViewMediaFragment() {
)
}

override fun onResume() {
super.onResume()
finalizeViewSetup()
}

override fun onToolbarVisibilityChange(visible: Boolean) {
if (!userVisibleHint) return

Expand All @@ -228,30 +208,24 @@ class ViewImageFragment : ViewMediaFragment() {
Glide.with(this).clear(binding.photoView)
}

override fun onDestroyView() {
transition.onComplete()
super.onDestroyView()
}

@SuppressLint("CheckResult")
private fun loadImageFromNetwork(url: String, previewUrl: String?, photoView: ImageView) {
val glide = Glide.with(this)
// Request image from the any cache
glide
.load(url)
.dontAnimate()
.onlyRetrieveFromCache(true)
.let {
if (previewUrl != null) {
it.thumbnail(
.apply {
previewUrl?.let {
thumbnail(
glide
.load(previewUrl)
.load(it)
.dontAnimate()
.onlyRetrieveFromCache(true)
.centerInside()
.addListener(ImageRequestListener(true, isThumbnailRequest = true)),
)
} else {
it
}
}
// Request image from the network on fail load image from cache
Expand Down Expand Up @@ -297,10 +271,10 @@ class ViewImageFragment : ViewMediaFragment() {
isFirstResource: Boolean,
): Boolean {
// If cache for full image failed complete transition
if (isCacheRequest && !isThumbnailRequest && shouldStartTransition &&
if (isCacheRequest && !isThumbnailRequest && shouldCallMediaReady &&
!startedTransition
) {
photoActionsListener.onBringUp()
mediaActionsListener.onMediaReady()
}
// Hide progress bar only on fail request from internet
if (!isCacheRequest) binding.progressBar.hide()
Expand All @@ -318,29 +292,26 @@ class ViewImageFragment : ViewMediaFragment() {
): Boolean {
binding.progressBar.hide() // Always hide the progress bar on success

if (!startedTransition || !shouldStartTransition) {
if (!startedTransition || !shouldCallMediaReady) {
// Set this right away so that we don't have to concurrent post() requests
startedTransition = true
// post() because load() replaces image with null. Sometimes after we set
// the thumbnail.
binding.photoView.post {
target.onResourceReady(resource, null)
if (shouldStartTransition) photoActionsListener.onBringUp()
if (shouldCallMediaReady) mediaActionsListener.onMediaReady()
}
} else {
// This wait for transition. If there's no transition then we should hit
// another branch. take() will unsubscribe after we have it to not leak memory
transition
.take(1)
.subscribe {
// Wait for the fragment transition to complete before signalling to
// Glide that the resource is ready.
transitionComplete?.let {
viewLifecycleOwner.lifecycleScope.launch {
it.await()
target.onResourceReady(resource, null)
}
}
}
return true
}
}

override fun onTransitionEnd() {
this.transition.onNext(Unit)
}
}
Loading
Loading