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

4063: Make dialog size more stable #4066

Merged
merged 2 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

package com.keylesspalace.tusky.components.compose.dialog

import android.app.Dialog
import android.content.Context
import android.graphics.drawable.Drawable
import android.net.Uri
Expand All @@ -26,7 +25,7 @@ import android.view.View
import android.view.ViewGroup
import android.view.WindowManager
import android.widget.EditText
import androidx.appcompat.app.AlertDialog
import android.widget.LinearLayout
import androidx.core.os.BundleCompat
import androidx.core.os.bundleOf
import androidx.fragment.app.DialogFragment
Expand All @@ -36,6 +35,8 @@ import com.bumptech.glide.request.target.CustomTarget
import com.bumptech.glide.request.transition.Transition
import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.databinding.DialogImageDescriptionBinding
import com.keylesspalace.tusky.util.hide
import com.keylesspalace.tusky.util.viewBinding

// https://github.com/tootsuite/mastodon/blob/c6904c0d3766a2ea8a81ab025c127169ecb51373/app/models/media_attachment.rb#L32
private const val MEDIA_DESCRIPTION_CHARACTER_LIMIT = 1500
Expand All @@ -44,11 +45,26 @@ class CaptionDialog : DialogFragment() {
private lateinit var listener: Listener
private lateinit var input: EditText

override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
val context = requireContext()
private val binding by viewBinding(DialogImageDescriptionBinding::bind)

val binding = DialogImageDescriptionBinding.inflate(layoutInflater)
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setStyle(STYLE_NORMAL, R.style.TuskyDialogFragmentStyle)
}

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?
): View? {
savedInstanceState?.getString(DESCRIPTION_KEY)?.let {
input.setText(it)
}

return inflater.inflate(R.layout.dialog_image_description, container, false)
}

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
input = binding.imageDescriptionText
val imageView = binding.imageDescriptionView
imageView.maxZoom = 6f
Expand All @@ -61,20 +77,19 @@ class CaptionDialog : DialogFragment() {
input.filters = arrayOf(InputFilter.LengthFilter(MEDIA_DESCRIPTION_CHARACTER_LIMIT))
input.setText(arguments?.getString(EXISTING_DESCRIPTION_ARG))

binding.cancelButton.setOnClickListener {
dismiss()
}
val localId = arguments?.getInt(LOCAL_ID_ARG) ?: error("Missing localId")
val dialog = AlertDialog.Builder(context)
.setView(binding.root)
.setPositiveButton(android.R.string.ok) { _, _ ->
listener.onUpdateDescription(localId, input.text.toString())
}
.setNegativeButton(android.R.string.cancel, null)
.create()
binding.okButton.setOnClickListener {
listener.onUpdateDescription(localId, input.text.toString())
dismiss()
}

isCancelable = true
val window = dialog.window
window?.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_ADJUST_RESIZE)

val previewUri = BundleCompat.getParcelable(requireArguments(), PREVIEW_URI_ARG, Uri::class.java) ?: error("Preview Uri is null")

// Load the image and manually set it into the ImageView because it doesn't have a fixed size.
Glide.with(this)
.load(previewUri)
Expand All @@ -90,27 +105,30 @@ class CaptionDialog : DialogFragment() {
) {
imageView.setImageDrawable(resource)
}

override fun onLoadFailed(errorDrawable: Drawable?) {
super.onLoadFailed(errorDrawable)
imageView.hide()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think instead of just assuming it is an audio file when loading fails I'd rather pass an explicit flag. The flag could also be used to display a different hint that makes more sense for audio files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was wondering (and didn't quickly find) how to find the type of the media thing.

Because always putting it in Glide says that the code expects an image there - which it probably shouldn't.

}
})
}

return dialog
override fun onStart() {
super.onStart()
dialog?.apply {
window?.setLayout(
LinearLayout.LayoutParams.MATCH_PARENT,
LinearLayout.LayoutParams.MATCH_PARENT
)
window?.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_ADJUST_RESIZE)
}
}

override fun onSaveInstanceState(outState: Bundle) {
outState.putString(DESCRIPTION_KEY, input.text.toString())
super.onSaveInstanceState(outState)
}

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?
): View? {
savedInstanceState?.getString(DESCRIPTION_KEY)?.let {
input.setText(it)
}
return super.onCreateView(inflater, container, savedInstanceState)
}

override fun onAttach(context: Context) {
super.onAttach(context)
listener = context as? Listener ?: error("Activity is not ComposeCaptionDialog.Listener")
Expand Down
40 changes: 33 additions & 7 deletions app/src/main/res/layout/dialog_image_description.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:layout_width="match_parent"
Expand All @@ -11,28 +12,53 @@
android:layout_width="match_parent"
android:layout_height="0dp"
Copy link

Choose a reason for hiding this comment

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

Can layout height be set to match_parent here so you don't have to set it in whatever you are doing in the onStart block?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the image view (with layout_weight=1).

The things in the onStart influence the window/dialog. Normally an (alert) dialog implicitly has something like "wrap_content" that is overwritten there.

android:layout_weight="1"
android:contentDescription="@string/post_media_image"/>
android:contentDescription="@string/post_media_image" />

<com.google.android.material.textfield.TextInputLayout
style="@style/TuskyTextInput"

android:layout_width="match_parent"
android:layout_height="0dp"
android:layout_weight="1"
android:layout_marginStart="?dialogPreferredPadding"
android:layout_marginTop="?dialogPreferredPadding"
android:layout_marginEnd="?dialogPreferredPadding"
app:hintEnabled="false"
android:layout_marginTop="?dialogPreferredPadding"
app:counterEnabled="false"
app:counterTextColor="?android:textColorTertiary">
app:counterTextColor="?android:textColorTertiary"
app:hintEnabled="false">

<com.google.android.material.textfield.TextInputEditText
android:id="@+id/imageDescriptionText"
android:inputType="textCapSentences|textMultiLine|textAutoCorrect"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:hint="@string/hint_description"
android:gravity="start"
android:importantForAutofill="no" />
android:hint="@string/hint_description"
connyduck marked this conversation as resolved.
Show resolved Hide resolved
android:importantForAutofill="no"
android:inputType="textCapSentences|textMultiLine|textAutoCorrect" />

</com.google.android.material.textfield.TextInputLayout>

<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_weight="0"
android:orientation="horizontal"
android:gravity="end"
android:layout_marginStart="?dialogPreferredPadding"
android:layout_marginEnd="?dialogPreferredPadding">

<Button
android:id="@+id/cancelButton"
style="@style/TuskyButton.TextButton"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="@android:string/cancel" />

<Button
android:id="@+id/okButton"
style="@style/TuskyButton.TextButton"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="@android:string/ok" />
</LinearLayout>
</LinearLayout>