-
-
Notifications
You must be signed in to change notification settings - Fork 388
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
Add UI for image-attachment "focus" #2620
Changes from 23 commits
f3bde24
613358e
7d86015
c283a4c
7b3fd74
02b5e68
84926d9
a439ec5
5e920da
09ab141
8f08b1b
d345e62
c89cdcf
3437fa8
480b07b
0e6c942
f4e4f8b
d929b98
dab6381
af5c995
829c151
cde5d5e
a93369b
400683d
a81ad1c
829a0ed
fc771f3
33d5745
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,7 +107,7 @@ class ComposeViewModel @Inject constructor( | |
} | ||
} | ||
|
||
suspend fun pickMedia(mediaUri: Uri, description: String? = null): Result<QueuedMedia> = withContext(Dispatchers.IO) { | ||
suspend fun pickMedia(mediaUri: Uri, description: String? = null, focus: Attachment.Focus? = null): Result<QueuedMedia> = withContext(Dispatchers.IO) { | ||
try { | ||
val (type, uri, size) = mediaUploader.prepareMedia(mediaUri) | ||
val mediaItems = media.value | ||
|
@@ -117,7 +117,7 @@ class ComposeViewModel @Inject constructor( | |
) { | ||
Result.failure(VideoOrImageException()) | ||
} else { | ||
val queuedMedia = addMediaToQueue(type, uri, size, description) | ||
val queuedMedia = addMediaToQueue(type, uri, size, description, focus) | ||
Result.success(queuedMedia) | ||
} | ||
} catch (e: Exception) { | ||
|
@@ -130,6 +130,7 @@ class ComposeViewModel @Inject constructor( | |
uri: Uri, | ||
mediaSize: Long, | ||
description: String? = null, | ||
focus: Attachment.Focus? = null, | ||
replaceItem: QueuedMedia? = null | ||
): QueuedMedia { | ||
var stashMediaItem: QueuedMedia? = null | ||
|
@@ -140,7 +141,8 @@ class ComposeViewModel @Inject constructor( | |
uri = uri, | ||
type = type, | ||
mediaSize = mediaSize, | ||
description = description | ||
description = description, | ||
focus = focus | ||
) | ||
stashMediaItem = mediaItem | ||
|
||
|
@@ -185,7 +187,7 @@ class ComposeViewModel @Inject constructor( | |
return mediaItem | ||
} | ||
|
||
private fun addUploadedMedia(id: String, type: QueuedMedia.Type, uri: Uri, description: String?) { | ||
private fun addUploadedMedia(id: String, type: QueuedMedia.Type, uri: Uri, description: String?, focus: Attachment.Focus?) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Long term observation: Adding focus to "travel along with" description when attachments hibernate/resurrect required adding plumbing in a LOT of places. It might make more sense to make a "Metadata" struct like the one already in Attachment and pass that around instead of description (although not Attachment.Meta itself, as that includes "duration", a metadata returned from the server but not tracked by us when composing attachments), in case later there is some third thing that we track per attachment (a low res version, I don't know) |
||
media.update { mediaValue -> | ||
val mediaItem = QueuedMedia( | ||
localId = (mediaValue.maxOfOrNull { it.localId } ?: 0) + 1, | ||
|
@@ -194,7 +196,8 @@ class ComposeViewModel @Inject constructor( | |
mediaSize = 0, | ||
uploadPercent = -1, | ||
id = id, | ||
description = description | ||
description = description, | ||
focus = focus | ||
) | ||
mediaValue + mediaItem | ||
} | ||
|
@@ -248,9 +251,11 @@ class ComposeViewModel @Inject constructor( | |
suspend fun saveDraft(content: String, contentWarning: String) { | ||
val mediaUris: MutableList<String> = mutableListOf() | ||
val mediaDescriptions: MutableList<String?> = mutableListOf() | ||
val mediaFocus: MutableList<Attachment.Focus?> = mutableListOf() | ||
media.value.forEach { item -> | ||
mediaUris.add(item.uri.toString()) | ||
mediaDescriptions.add(item.description) | ||
mediaFocus.add(item.focus) | ||
} | ||
|
||
draftHelper.saveDraft( | ||
|
@@ -263,6 +268,7 @@ class ComposeViewModel @Inject constructor( | |
visibility = statusVisibility.value!!, | ||
mediaUris = mediaUris, | ||
mediaDescriptions = mediaDescriptions, | ||
mediaFocus = mediaFocus, | ||
poll = poll.value, | ||
failedToSend = false | ||
) | ||
|
@@ -290,11 +296,13 @@ class ComposeViewModel @Inject constructor( | |
val mediaIds: MutableList<String> = mutableListOf() | ||
val mediaUris: MutableList<Uri> = mutableListOf() | ||
val mediaDescriptions: MutableList<String> = mutableListOf() | ||
val mediaFocus: MutableList<Attachment.Focus?> = mutableListOf() | ||
val mediaProcessed: MutableList<Boolean> = mutableListOf() | ||
for (item in media.value) { | ||
mediaIds.add(item.id!!) | ||
mediaUris.add(item.uri) | ||
mediaDescriptions.add(item.description ?: "") | ||
mediaFocus.add(item.focus) | ||
mediaProcessed.add(false) | ||
} | ||
|
||
|
@@ -306,6 +314,7 @@ class ComposeViewModel @Inject constructor( | |
mediaIds = mediaIds, | ||
mediaUris = mediaUris.map { it.toString() }, | ||
mediaDescriptions = mediaDescriptions, | ||
mediaFocus = mediaFocus, | ||
scheduledAt = scheduledAt.value, | ||
inReplyToId = inReplyToId, | ||
poll = poll.value, | ||
|
@@ -324,11 +333,12 @@ class ComposeViewModel @Inject constructor( | |
return combineLiveData(deletionObservable, sendFlow.asLiveData()) { _, _ -> } | ||
} | ||
|
||
suspend fun updateDescription(localId: Int, description: String): Boolean { | ||
// Updates a QueuedMedia item arbitrarily, then sends description and focus to server | ||
private suspend fun updateMediaItem(localId: Int, mutator: (QueuedMedia) -> QueuedMedia): Boolean { | ||
val newMediaList = media.updateAndGet { mediaValue -> | ||
mediaValue.map { mediaItem -> | ||
if (mediaItem.localId == localId) { | ||
mediaItem.copy(description = description) | ||
mutator(mediaItem) | ||
} else { | ||
mediaItem | ||
} | ||
|
@@ -337,7 +347,9 @@ class ComposeViewModel @Inject constructor( | |
|
||
val updatedItem = newMediaList.find { it.localId == localId } | ||
if (updatedItem?.id != null) { | ||
return api.updateMedia(updatedItem.id, description) | ||
val focus = updatedItem.focus | ||
val focusString = if (focus != null) "${focus.x},${focus.y}" else null | ||
return api.updateMedia(updatedItem.id, updatedItem.description, focusString) | ||
.fold({ | ||
true | ||
}, { throwable -> | ||
|
@@ -348,6 +360,18 @@ class ComposeViewModel @Inject constructor( | |
return true | ||
} | ||
|
||
suspend fun updateDescription(localId: Int, description: String): Boolean { | ||
return updateMediaItem(localId, { mediaItem -> | ||
mediaItem.copy(description = description) | ||
}) | ||
} | ||
|
||
suspend fun updateFocus(localId: Int, focus: Attachment.Focus): Boolean { | ||
return updateMediaItem(localId, { mediaItem -> | ||
mediaItem.copy(focus = focus) | ||
}) | ||
} | ||
|
||
fun searchAutocompleteSuggestions(token: String): List<AutocompleteResult> { | ||
when (token[0]) { | ||
'@' -> { | ||
|
@@ -418,7 +442,7 @@ class ComposeViewModel @Inject constructor( | |
// when coming from DraftActivity | ||
viewModelScope.launch { | ||
draftAttachments.forEach { attachment -> | ||
pickMedia(attachment.uri, attachment.description) | ||
pickMedia(attachment.uri, attachment.description, attachment.focus) | ||
} | ||
} | ||
} else composeOptions?.mediaAttachments?.forEach { a -> | ||
|
@@ -428,7 +452,7 @@ class ComposeViewModel @Inject constructor( | |
Attachment.Type.UNKNOWN, Attachment.Type.IMAGE -> QueuedMedia.Type.IMAGE | ||
Attachment.Type.AUDIO -> QueuedMedia.Type.AUDIO | ||
} | ||
addUploadedMedia(a.id, mediaType, a.url.toUri(), a.description) | ||
addUploadedMedia(a.id, mediaType, a.url.toUri(), a.description, a.meta?.focus) | ||
} | ||
|
||
draftId = composeOptions?.draftId ?: 0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
/* Copyright 2019 Tusky Contributors | ||
* | ||
* This file is a part of Tusky. | ||
* | ||
* This program is free software; you can redistribute it and/or modify it under the terms of the | ||
* GNU General Public License as published by the Free Software Foundation; either version 3 of the | ||
* License, or (at your option) any later version. | ||
* | ||
* Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even | ||
* the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General | ||
* Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License along with Tusky; if not, | ||
* see <http://www.gnu.org/licenses>. */ | ||
|
||
package com.keylesspalace.tusky.components.compose.dialog | ||
|
||
import android.app.Activity | ||
import android.content.DialogInterface | ||
import android.graphics.drawable.Drawable | ||
import android.net.Uri | ||
import android.view.WindowManager | ||
import android.widget.FrameLayout | ||
import android.widget.Toast | ||
import androidx.appcompat.app.AlertDialog | ||
import androidx.lifecycle.LifecycleOwner | ||
import androidx.lifecycle.lifecycleScope | ||
import com.bumptech.glide.Glide | ||
import com.bumptech.glide.load.DataSource | ||
import com.bumptech.glide.load.engine.GlideException | ||
import com.bumptech.glide.load.resource.bitmap.DownsampleStrategy | ||
import com.bumptech.glide.request.RequestListener | ||
import com.bumptech.glide.request.target.Target | ||
import com.keylesspalace.tusky.R | ||
import com.keylesspalace.tusky.databinding.DialogFocusBinding | ||
import com.keylesspalace.tusky.entity.Attachment.Focus | ||
import kotlinx.coroutines.launch | ||
|
||
fun <T> T.makeFocusDialog( | ||
existingFocus: Focus?, | ||
previewUri: Uri, | ||
onUpdateFocus: suspend (Focus) -> Boolean | ||
) where T : Activity, T : LifecycleOwner { | ||
val focus = existingFocus ?: Focus(0.0f, 0.0f) // Default to center | ||
|
||
val dialogBinding = DialogFocusBinding.inflate(layoutInflater) | ||
|
||
dialogBinding.focusIndicator.setFocus(focus) | ||
|
||
Glide.with(this) | ||
.load(previewUri) | ||
.downsample(DownsampleStrategy.CENTER_INSIDE) | ||
.listener(object : RequestListener<Drawable> { | ||
override fun onLoadFailed(p0: GlideException?, p1: Any?, p2: Target<Drawable?>?, p3: Boolean): Boolean { | ||
return false | ||
} | ||
|
||
override fun onResourceReady(resource: Drawable?, model: Any?, target: Target<Drawable?>?, dataSource: DataSource?, isFirstResource: Boolean): Boolean { | ||
val width = resource!!.getIntrinsicWidth() | ||
val height = resource.getIntrinsicHeight() | ||
|
||
dialogBinding.focusIndicator.setImageSize(width, height) | ||
|
||
// We want the dialog to be a little taller than the image, so you can slide your thumb past the image border, | ||
// but if it's *too* much taller that looks weird. See if a threshold has been crossed: | ||
if (width > height) { | ||
val maxHeight = dialogBinding.focusIndicator.maxAttractiveHeight() | ||
|
||
if (dialogBinding.imageView.getHeight() > maxHeight) { | ||
val verticalShrinkLayout = FrameLayout.LayoutParams(width, maxHeight) | ||
dialogBinding.imageView.setLayoutParams(verticalShrinkLayout) | ||
dialogBinding.focusIndicator.setLayoutParams(verticalShrinkLayout) | ||
} | ||
} | ||
return false // Pass through | ||
} | ||
}) | ||
.into(dialogBinding.imageView) | ||
|
||
val okListener = { dialog: DialogInterface, _: Int -> | ||
lifecycleScope.launch { | ||
if (!onUpdateFocus(dialogBinding.focusIndicator.getFocus())) { | ||
showFailedFocusMessage() | ||
} | ||
} | ||
dialog.dismiss() | ||
} | ||
|
||
val dialog = AlertDialog.Builder(this) | ||
.setView(dialogBinding.root) | ||
.setPositiveButton(android.R.string.ok, okListener) | ||
.setNegativeButton(android.R.string.cancel, null) | ||
.create() | ||
|
||
val window = dialog.window | ||
window?.setSoftInputMode( | ||
WindowManager.LayoutParams.SOFT_INPUT_ADJUST_RESIZE | ||
) | ||
|
||
dialog.show() | ||
} | ||
|
||
private fun Activity.showFailedFocusMessage() { | ||
Toast.makeText(this, R.string.error_failed_set_focus, Toast.LENGTH_SHORT).show() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the best approach? We could do something weird where we try to calculate where/whether the focal point is within the cropped area, but this feels like second guessing the user so I just reset.