From 58a1046348fe661e2f150c22c3cc7bb3cde4bec2 Mon Sep 17 00:00:00 2001 From: Ivan Kupalov Date: Mon, 22 Jun 2020 21:26:37 +0200 Subject: [PATCH 1/2] Improve image viewer (#1843) This commit does 3 things: 1. Replaces PhotoView (which is abandonware) with modern TouchImageView 2. Fixes an issue with panning images. Gesture was not intercepted properly and pager was taking control instead of image being moved. 3. Adds feedback to dismissing of images with vertical gesture. --- app/build.gradle | 2 +- .../compose/dialog/CaptionDialog.kt | 7 +- .../tusky/fragment/ViewImageFragment.kt | 98 +++++++++++++------ .../main/res/layout/fragment_view_image.xml | 3 +- 4 files changed, 72 insertions(+), 38 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 6f8f08a9d..38996d500 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -159,7 +159,7 @@ dependencies { implementation "com.github.connyduck:sparkbutton:4.0.0" - implementation "com.github.chrisbanes:PhotoView:2.3.0" + implementation 'com.github.MikeOrtiz:TouchImageView:3.0.1' implementation "com.mikepenz:materialdrawer:$materialdrawerVersion" implementation "com.mikepenz:materialdrawer-iconics:$materialdrawerVersion" diff --git a/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/CaptionDialog.kt b/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/CaptionDialog.kt index f10fb444a..a768df098 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/CaptionDialog.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/compose/dialog/CaptionDialog.kt @@ -33,9 +33,9 @@ import at.connyduck.sparkbutton.helpers.Utils import com.bumptech.glide.Glide import com.bumptech.glide.request.target.CustomTarget import com.bumptech.glide.request.transition.Transition -import com.github.chrisbanes.photoview.PhotoView import com.keylesspalace.tusky.R import com.keylesspalace.tusky.util.withLifecycleContext +import com.ortiz.touchview.TouchImageView // https://github.com/tootsuite/mastodon/blob/1656663/app/models/media_attachment.rb#L94 private const val MEDIA_DESCRIPTION_CHARACTER_LIMIT = 420 @@ -50,9 +50,8 @@ fun T.makeCaptionDialog(existingDescription: String?, dialogLayout.setPadding(padding, padding, padding, padding) dialogLayout.orientation = LinearLayout.VERTICAL - val imageView = PhotoView(this).apply { - // If it seems a lot, try opening an image of A4 format or similar - maximumScale = 6.0f + val imageView = TouchImageView(this).apply { + maxZoom = 6f } val displayMetrics = DisplayMetrics() diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt index 2831100db..49e313fae 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt @@ -21,9 +21,7 @@ import android.annotation.SuppressLint import android.content.Context import android.graphics.drawable.Drawable import android.os.Bundle -import android.view.LayoutInflater -import android.view.View -import android.view.ViewGroup +import android.view.* import android.widget.ImageView import android.widget.TextView import com.bumptech.glide.Glide @@ -31,7 +29,6 @@ import com.bumptech.glide.load.DataSource import com.bumptech.glide.load.engine.GlideException import com.bumptech.glide.request.RequestListener import com.bumptech.glide.request.target.Target -import com.github.chrisbanes.photoview.PhotoViewAttacher import com.keylesspalace.tusky.R import com.keylesspalace.tusky.entity.Attachment import com.keylesspalace.tusky.util.hide @@ -48,11 +45,11 @@ class ViewImageFragment : ViewMediaFragment() { fun onPhotoTap() } - private lateinit var attacher: PhotoViewAttacher private lateinit var photoActionsListener: PhotoActionsListener private lateinit var toolbar: View private var transition = BehaviorSubject.create() private var shouldStartTransition = false + // 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 @@ -67,23 +64,6 @@ class ViewImageFragment : ViewMediaFragment() { override fun setupMediaView(url: String, previewUrl: String?) { descriptionView = mediaDescription photoView.transitionName = url - attacher = PhotoViewAttacher(photoView).apply { - // Clicking outside the photo closes the viewer. - setOnOutsidePhotoTapListener { photoActionsListener.onDismiss() } - setOnClickListener { onMediaTap() } - - /* A vertical swipe motion also closes the viewer. This is especially useful when the photo - * mostly fills the screen so clicking outside is difficult. */ - setOnSingleFlingListener { _, _, velocityX, velocityY -> - var result = false - if (abs(velocityY) > abs(velocityX)) { - photoActionsListener.onDismiss() - result = true - } - result - } - } - startedTransition = false loadImageFromNetwork(url, previewUrl, photoView) } @@ -94,9 +74,65 @@ class ViewImageFragment : ViewMediaFragment() { return inflater.inflate(R.layout.fragment_view_image, container, false) } + @SuppressLint("ClickableViewAccessibility") override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + val gestureDetector = GestureDetector(requireContext(), object : GestureDetector.SimpleOnGestureListener() { + override fun onSingleTapConfirmed(e: MotionEvent?): Boolean { + onMediaTap() + return true + } + }) + + var lastY = 0f + photoView.setOnTouchListener { _, event -> + // This part is for scaling/translating on vertical move. + // We use raw coordinates to get the correct ones during scaling + var result = true + + gestureDetector.onTouchEvent(event) + + if (event.action == MotionEvent.ACTION_DOWN) { + lastY = event.rawY + } else if (!photoView.isZoomed && event.action == MotionEvent.ACTION_MOVE) { + val diff = event.rawY - lastY + // This code is to prevent transformations during page scrolling + // If we are already translating or we reached the threshold, then transform. + if (photoView.translationY != 0f || abs(diff) > 40) { + photoView.translationY += (diff) + val scale = (-abs(photoView.translationY) / 720 + 1).coerceAtLeast(0.5f) + photoView.scaleY = scale + photoView.scaleX = scale + lastY = event.rawY + } + return@setOnTouchListener true + } else if (event.action == MotionEvent.ACTION_UP || event.action == MotionEvent.ACTION_CANCEL) { + onGestureEnd() + } else if (event.pointerCount >= 2 || photoView.canScrollHorizontally(1) && photoView.canScrollHorizontally(-1)) { + // Starting from here is adapted code from TouchImageView to play nice with pager. + + // Can scroll horizontally checks if there's still a part of the image. + // That can be scrolled until you reach the edge multi-touch event. + val parent = view.parent + result = when (event.action) { + MotionEvent.ACTION_DOWN, MotionEvent.ACTION_MOVE -> { + // Disallow RecyclerView to intercept touch events. + parent.requestDisallowInterceptTouchEvent(true) + // Disable touch on view + false + } + MotionEvent.ACTION_UP -> { + // Allow RecyclerView to intercept touch events. + parent.requestDisallowInterceptTouchEvent(false) + true + } + else -> true + } + } + result + } + val arguments = this.requireArguments() val attachment = arguments.getParcelable(ARG_ATTACHMENT) this.shouldStartTransition = arguments.getBoolean(ARG_START_POSTPONED_TRANSITION) @@ -116,6 +152,14 @@ class ViewImageFragment : ViewMediaFragment() { finalizeViewSetup(url, attachment?.previewUrl, description) } + private fun onGestureEnd() { + if (abs(photoView.translationY) > 180) { + photoActionsListener.onDismiss() + } else { + photoView.animate().translationY(0f).scaleX(1f).scaleY(1f).start() + } + } + private fun onMediaTap() { photoActionsListener.onPhotoTap() } @@ -155,7 +199,6 @@ class ViewImageFragment : ViewMediaFragment() { .load(previewUrl) .dontAnimate() .onlyRetrieveFromCache(true) - .centerInside() .addListener(ImageRequestListener(true, isThumnailRequest = true))) else it } @@ -164,7 +207,6 @@ class ViewImageFragment : ViewMediaFragment() { .centerInside() .addListener(ImageRequestListener(false, isThumnailRequest = false)) ) - .centerInside() .addListener(ImageRequestListener(true, isThumnailRequest = false)) .into(photoView) } @@ -225,13 +267,7 @@ class ViewImageFragment : ViewMediaFragment() { // another branch. take() will unsubscribe after we have it to not leak menmory transition .take(1) - .subscribe { - target.onResourceReady(resource, null) - // It's needed. Don't ask why, I don't know, setImageDrawable() should - // do it by itself but somehow it doesn't work automatically. - // Just do it. If you don't, image will jump around when touched. - attacher.update() - } + .subscribe { target.onResourceReady(resource, null) } } return true } diff --git a/app/src/main/res/layout/fragment_view_image.xml b/app/src/main/res/layout/fragment_view_image.xml index faf1f806f..1e3f442ac 100644 --- a/app/src/main/res/layout/fragment_view_image.xml +++ b/app/src/main/res/layout/fragment_view_image.xml @@ -4,11 +4,10 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="match_parent" - android:layout_gravity="center" android:clickable="true" android:focusable="true"> - From dfd30ec52a3791bae64bf16f319e8745ba7a478f Mon Sep 17 00:00:00 2001 From: Konrad Pozniak Date: Tue, 23 Jun 2020 19:59:49 +0200 Subject: [PATCH 2/2] correctly update the menu when muting domains (#1848) --- .../keylesspalace/tusky/AccountActivity.kt | 26 ++++++++++++----- .../tusky/entity/Relationship.kt | 3 +- .../tusky/viewmodel/AccountViewModel.kt | 29 +++++++++++++++++-- app/src/main/res/values/strings.xml | 1 + 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/AccountActivity.kt b/app/src/main/java/com/keylesspalace/tusky/AccountActivity.kt index 9c72da3f8..7f6ee5894 100644 --- a/app/src/main/java/com/keylesspalace/tusky/AccountActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/AccountActivity.kt @@ -81,6 +81,7 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidI private var followState: FollowState = FollowState.NOT_FOLLOWING private var blocking: Boolean = false private var muting: Boolean = false + private var blockingDomain: Boolean = false private var showingReblogs: Boolean = false private var loadedAccount: Account? = null @@ -528,6 +529,7 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidI } blocking = relation.blocking muting = relation.muting + blockingDomain = relation.blockingDomain showingReblogs = relation.showingReblogs accountFollowsYouTextView.visible(relation.followedBy) @@ -626,7 +628,11 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidI // If we can't get the domain, there's no way we can mute it anyway... menu.removeItem(R.id.action_mute_domain) } else { - muteDomain.title = getString(R.string.action_mute_domain, domain) + if (blockingDomain) { + muteDomain.title = getString(R.string.action_unmute_domain, domain) + } else { + muteDomain.title = getString(R.string.action_mute_domain, domain) + } } } @@ -671,12 +677,16 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidI .show() } - private fun showMuteDomainWarningDialog(instance: String) { - AlertDialog.Builder(this) - .setMessage(getString(R.string.mute_domain_warning, instance)) - .setPositiveButton(getString(R.string.mute_domain_warning_dialog_ok)) { _, _ -> viewModel.muteDomain(instance) } - .setNegativeButton(android.R.string.cancel, null) - .show() + private fun toggleBlockDomain(instance: String) { + if(blockingDomain) { + viewModel.unblockDomain(instance) + } else { + AlertDialog.Builder(this) + .setMessage(getString(R.string.mute_domain_warning, instance)) + .setPositiveButton(getString(R.string.mute_domain_warning_dialog_ok)) { _, _ -> viewModel.blockDomain(instance) } + .setNegativeButton(android.R.string.cancel, null) + .show() + } } private fun toggleBlock() { @@ -757,7 +767,7 @@ class AccountActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidI return true } R.id.action_mute_domain -> { - showMuteDomainWarningDialog(domain) + toggleBlockDomain(domain) return true } R.id.action_show_reblogs -> { diff --git a/app/src/main/java/com/keylesspalace/tusky/entity/Relationship.kt b/app/src/main/java/com/keylesspalace/tusky/entity/Relationship.kt index 3da157038..eb1d20b6a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/entity/Relationship.kt +++ b/app/src/main/java/com/keylesspalace/tusky/entity/Relationship.kt @@ -24,5 +24,6 @@ data class Relationship ( val blocking: Boolean, val muting: Boolean, val requested: Boolean, - @SerializedName("showing_reblogs") val showingReblogs: Boolean + @SerializedName("showing_reblogs") val showingReblogs: Boolean, + @SerializedName("domain_blocking") val blockingDomain: Boolean ) diff --git a/app/src/main/java/com/keylesspalace/tusky/viewmodel/AccountViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/viewmodel/AccountViewModel.kt index ad90cacc9..2495fce2a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/viewmodel/AccountViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/viewmodel/AccountViewModel.kt @@ -156,18 +156,41 @@ class AccountViewModel @Inject constructor( } } - fun muteDomain(instance: String) { + fun blockDomain(instance: String) { mastodonApi.blockDomain(instance).enqueue(object: Callback { override fun onResponse(call: Call, response: Response) { if (response.isSuccessful) { eventHub.dispatch(DomainMuteEvent(instance)) + val relation = relationshipData.value?.data + if(relation != null) { + relationshipData.postValue(Success(relation.copy(blockingDomain = true))) + } } else { - Log.e(TAG, String.format("Error muting %s", instance)) + Log.e(TAG, "Error muting %s".format(instance)) } } override fun onFailure(call: Call, t: Throwable) { - Log.e(TAG, String.format("Error muting %s", instance), t) + Log.e(TAG, "Error muting %s".format(instance), t) + } + }) + } + + fun unblockDomain(instance: String) { + mastodonApi.unblockDomain(instance).enqueue(object: Callback { + override fun onResponse(call: Call, response: Response) { + if (response.isSuccessful) { + val relation = relationshipData.value?.data + if(relation != null) { + relationshipData.postValue(Success(relation.copy(blockingDomain = false))) + } + } else { + Log.e(TAG, "Error unmuting %s".format(instance)) + } + } + + override fun onFailure(call: Call, t: Throwable) { + Log.e(TAG, "Error unmuting %s".format(instance), t) } }) } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index fb4032f43..99bb228a3 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -109,6 +109,7 @@ Mute Unmute Mute %s + Unmute %s Mute conversation Unmute conversation Mention