From 5e9691750843d9a295b88689ebbe2614322bd886 Mon Sep 17 00:00:00 2001 From: Michael <5672750+mibac138@users.noreply.github.com> Date: Tue, 28 Dec 2021 12:16:52 +0100 Subject: [PATCH] Fix overlapping text in the error dialog (#1708) --- .../wulkanowy/services/sync/SyncWorker.kt | 42 +++--- .../github/wulkanowy/ui/base/ErrorDialog.kt | 112 ++++++++-------- .../modules/dashboard/DashboardPresenter.kt | 4 +- .../ui/modules/settings/sync/SyncFragment.kt | 2 +- .../ui/modules/settings/sync/SyncPresenter.kt | 5 +- app/src/main/res/layout/dialog_error.xml | 126 +++--------------- 6 files changed, 111 insertions(+), 180 deletions(-) diff --git a/app/src/main/java/io/github/wulkanowy/services/sync/SyncWorker.kt b/app/src/main/java/io/github/wulkanowy/services/sync/SyncWorker.kt index 52979e63..a2d1dd57 100644 --- a/app/src/main/java/io/github/wulkanowy/services/sync/SyncWorker.kt +++ b/app/src/main/java/io/github/wulkanowy/services/sync/SyncWorker.kt @@ -38,13 +38,18 @@ class SyncWorker @AssistedInject constructor( private val dispatchersProvider: DispatchersProvider ) : CoroutineWorker(appContext, workerParameters) { - override suspend fun doWork() = withContext(dispatchersProvider.io) { + override suspend fun doWork(): Result = withContext(dispatchersProvider.io) { Timber.i("SyncWorker is starting") if (!studentRepository.isCurrentStudentSet()) return@withContext Result.failure() - val student = studentRepository.getCurrentStudent() - val semester = semesterRepository.getCurrentSemester(student, true) + val (student, semester) = try { + val student = studentRepository.getCurrentStudent() + val semester = semesterRepository.getCurrentSemester(student, true) + student to semester + } catch (e: Throwable) { + return@withContext getResultFromErrors(listOf(e)) + } val exceptions = works.mapNotNull { work -> try { @@ -62,20 +67,7 @@ class SyncWorker @AssistedInject constructor( } } } - val result = when { - exceptions.isNotEmpty() && inputData.getBoolean("one_time", false) -> { - Result.failure( - Data.Builder() - .putString("error", exceptions.map { it.stackTraceToString() }.toString()) - .build() - ) - } - exceptions.isNotEmpty() -> Result.retry() - else -> { - preferencesRepository.lasSyncDate = LocalDateTime.now() - Result.success() - } - } + val result = getResultFromErrors(exceptions) if (preferencesRepository.isDebugNotificationEnable) notify(result) Timber.i("SyncWorker result: $result") @@ -83,6 +75,22 @@ class SyncWorker @AssistedInject constructor( return@withContext result } + private fun getResultFromErrors(errors: List): Result = when { + errors.isNotEmpty() && inputData.getBoolean("one_time", false) -> { + Result.failure( + Data.Builder() + .putString("error_message", errors.joinToString { it.message.toString() }) + .putString("error_stack", errors.map { it.stackTraceToString() }.toString()) + .build() + ) + } + errors.isNotEmpty() -> Result.retry() + else -> { + preferencesRepository.lasSyncDate = LocalDateTime.now() + Result.success() + } + } + private fun notify(result: Result) { notificationManager.notify( Random.nextInt(Int.MAX_VALUE), diff --git a/app/src/main/java/io/github/wulkanowy/ui/base/ErrorDialog.kt b/app/src/main/java/io/github/wulkanowy/ui/base/ErrorDialog.kt index 4c279d81..c2ffff1f 100644 --- a/app/src/main/java/io/github/wulkanowy/ui/base/ErrorDialog.kt +++ b/app/src/main/java/io/github/wulkanowy/ui/base/ErrorDialog.kt @@ -1,28 +1,25 @@ package io.github.wulkanowy.ui.base +import android.app.Dialog import android.content.ClipData import android.content.ClipboardManager import android.os.Bundle import android.view.LayoutInflater -import android.view.View -import android.view.ViewGroup -import android.widget.HorizontalScrollView import android.widget.Toast import android.widget.Toast.LENGTH_LONG import androidx.appcompat.app.AlertDialog import androidx.core.content.getSystemService +import androidx.core.os.bundleOf import androidx.core.view.isGone +import androidx.fragment.app.DialogFragment +import com.google.android.material.dialog.MaterialAlertDialogBuilder import dagger.hilt.android.AndroidEntryPoint import io.github.wulkanowy.R import io.github.wulkanowy.databinding.DialogErrorBinding import io.github.wulkanowy.sdk.exception.FeatureNotAvailableException import io.github.wulkanowy.sdk.scrapper.exception.FeatureDisabledException import io.github.wulkanowy.sdk.scrapper.exception.ServiceUnavailableException -import io.github.wulkanowy.utils.AppInfo -import io.github.wulkanowy.utils.getString -import io.github.wulkanowy.utils.openAppInMarket -import io.github.wulkanowy.utils.openEmailClient -import io.github.wulkanowy.utils.openInternetBrowser +import io.github.wulkanowy.utils.* import okhttp3.internal.http2.StreamResetException import java.io.InterruptedIOException import java.net.ConnectException @@ -31,72 +28,77 @@ import java.net.UnknownHostException import javax.inject.Inject @AndroidEntryPoint -class ErrorDialog : BaseDialogFragment() { - - private lateinit var error: Throwable +class ErrorDialog : DialogFragment() { @Inject lateinit var appInfo: AppInfo companion object { - private const val ARGUMENT_KEY = "Data" + private const val ARGUMENT_KEY = "error" fun newInstance(error: Throwable) = ErrorDialog().apply { - arguments = Bundle().apply { putSerializable(ARGUMENT_KEY, error) } + arguments = bundleOf(ARGUMENT_KEY to error) } } - override fun onCreate(savedInstanceState: Bundle?) { - super.onCreate(savedInstanceState) - setStyle(STYLE_NO_TITLE, 0) - arguments?.run { - error = getSerializable(ARGUMENT_KEY) as Throwable + override fun onCreateDialog(savedInstanceState: Bundle?): Dialog { + val error = requireArguments().getSerializable(ARGUMENT_KEY) as Throwable + + val binding = DialogErrorBinding.inflate(LayoutInflater.from(context)) + binding.bindErrorDetails(error) + + return getAlertDialog(binding, error).apply { + enableReportButtonIfErrorIsReportable(error) } } - override fun onCreateView( - inflater: LayoutInflater, - container: ViewGroup?, - savedInstanceState: Bundle? - ) = DialogErrorBinding.inflate(inflater).apply { binding = this }.root - - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - super.onViewCreated(view, savedInstanceState) - - val errorStacktrace = error.stackTraceToString() - - with(binding) { - errorDialogContent.text = errorStacktrace.replace(": ${error.localizedMessage}", "") - with(errorDialogHorizontalScroll) { - post { fullScroll(HorizontalScrollView.FOCUS_LEFT) } - } - errorDialogCopy.setOnClickListener { - val clip = ClipData.newPlainText("Error details", errorStacktrace) - activity?.getSystemService()?.setPrimaryClip(clip) - - Toast.makeText(context, R.string.all_copied, LENGTH_LONG).show() - } - errorDialogCancel.setOnClickListener { dismiss() } - errorDialogReport.setOnClickListener { + private fun getAlertDialog(binding: DialogErrorBinding, error: Throwable): AlertDialog { + return MaterialAlertDialogBuilder(requireContext()).apply { + val errorStacktrace = error.stackTraceToString() + setTitle(R.string.all_details) + setView(binding.root) + setNeutralButton(R.string.about_feedback) { _, _ -> openConfirmDialog { openEmailClient(errorStacktrace) } } + setNegativeButton(android.R.string.cancel) { _, _ -> } + setPositiveButton(android.R.string.copy) { _, _ -> copyErrorToClipboard(errorStacktrace) } + }.create() + } + + private fun DialogErrorBinding.bindErrorDetails(error: Throwable) { + return with(this) { errorDialogHumanizedMessage.text = resources.getString(error) errorDialogErrorMessage.text = error.localizedMessage errorDialogErrorMessage.isGone = error.localizedMessage.isNullOrBlank() - errorDialogReport.isEnabled = when (error) { - is UnknownHostException, - is InterruptedIOException, - is ConnectException, - is StreamResetException, - is SocketTimeoutException, - is ServiceUnavailableException, - is FeatureDisabledException, - is FeatureNotAvailableException -> false - else -> true - } + errorDialogContent.text = error.stackTraceToString() + .replace(": ${error.localizedMessage}", "") } } + private fun AlertDialog.enableReportButtonIfErrorIsReportable(error: Throwable) { + setOnShowListener { + getButton(AlertDialog.BUTTON_NEUTRAL).isEnabled = isErrorShouldBeReported(error) + } + } + + private fun isErrorShouldBeReported(error: Throwable): Boolean = when (error) { + is UnknownHostException, + is InterruptedIOException, + is ConnectException, + is StreamResetException, + is SocketTimeoutException, + is ServiceUnavailableException, + is FeatureDisabledException, + is FeatureNotAvailableException -> false + else -> true + } + + private fun copyErrorToClipboard(errorStacktrace: String) { + val clip = ClipData.newPlainText("Error details", errorStacktrace) + requireActivity().getSystemService()?.setPrimaryClip(clip) + Toast.makeText(requireContext(), R.string.all_copied, LENGTH_LONG).show() + } + private fun openConfirmDialog(callback: () -> Unit) { AlertDialog.Builder(requireContext()) .setTitle(R.string.dialog_error_check_update) @@ -127,4 +129,8 @@ class ErrorDialog : BaseDialogFragment() { } ) } + + private fun showMessage(text: String) { + Toast.makeText(requireContext(), text, LENGTH_LONG).show() + } } diff --git a/app/src/main/java/io/github/wulkanowy/ui/modules/dashboard/DashboardPresenter.kt b/app/src/main/java/io/github/wulkanowy/ui/modules/dashboard/DashboardPresenter.kt index d081e19a..7ba4c4b6 100644 --- a/app/src/main/java/io/github/wulkanowy/ui/modules/dashboard/DashboardPresenter.kt +++ b/app/src/main/java/io/github/wulkanowy/ui/modules/dashboard/DashboardPresenter.kt @@ -716,7 +716,7 @@ class DashboardPresenter @Inject constructor( itemsLoadedList.find { it.type == DashboardItem.Type.ACCOUNT }?.error != null val isGeneralError = filteredItems.none { it.error == null } && filteredItems.isNotEmpty() || isAccountItemError - val errorMessage = itemsLoadedList.map { it.error?.stackTraceToString() }.toString() + val firstError = itemsLoadedList.mapNotNull { it.error }.firstOrNull() val filteredOriginalLoadedList = dashboardItemLoadedList.filterNot { it.type == DashboardItem.Type.ACCOUNT } @@ -726,7 +726,7 @@ class DashboardPresenter @Inject constructor( filteredOriginalLoadedList.none { it.error == null } && filteredOriginalLoadedList.isNotEmpty() || wasAccountItemError if (isGeneralError && isItemsLoaded) { - lastError = Exception(errorMessage) + lastError = requireNotNull(firstError) view?.run { showProgress(false) diff --git a/app/src/main/java/io/github/wulkanowy/ui/modules/settings/sync/SyncFragment.kt b/app/src/main/java/io/github/wulkanowy/ui/modules/settings/sync/SyncFragment.kt index 160b7c37..d81c35d3 100644 --- a/app/src/main/java/io/github/wulkanowy/ui/modules/settings/sync/SyncFragment.kt +++ b/app/src/main/java/io/github/wulkanowy/ui/modules/settings/sync/SyncFragment.kt @@ -91,7 +91,7 @@ class SyncFragment : PreferenceFragmentCompat(), } override fun showErrorDetailsDialog(error: Throwable) { - ErrorDialog.newInstance(error).show(childFragmentManager, error.toString()) + ErrorDialog.newInstance(error).show(childFragmentManager, "error_details") } override fun onResume() { diff --git a/app/src/main/java/io/github/wulkanowy/ui/modules/settings/sync/SyncPresenter.kt b/app/src/main/java/io/github/wulkanowy/ui/modules/settings/sync/SyncPresenter.kt index 0d404a13..fc47e29a 100644 --- a/app/src/main/java/io/github/wulkanowy/ui/modules/settings/sync/SyncPresenter.kt +++ b/app/src/main/java/io/github/wulkanowy/ui/modules/settings/sync/SyncPresenter.kt @@ -59,7 +59,10 @@ class SyncPresenter @Inject constructor( WorkInfo.State.FAILED -> { showError( syncFailedString, - Throwable(workInfo.outputData.getString("error")) + Throwable( + message = workInfo.outputData.getString("error_message"), + cause = Throwable(workInfo.outputData.getString("error_stack")) + ) ) analytics.logEvent("sync_now", "status" to "failed") } diff --git a/app/src/main/res/layout/dialog_error.xml b/app/src/main/res/layout/dialog_error.xml index b52c99ac..98b9c8b1 100644 --- a/app/src/main/res/layout/dialog_error.xml +++ b/app/src/main/res/layout/dialog_error.xml @@ -7,15 +7,6 @@ android:orientation="vertical" tools:context=".ui.base.ErrorDialog"> - - - + android:layout_height="200dp" + android:overScrollMode="ifContentScrolls" + android:paddingHorizontal="24dp" + app:layout_constraintTop_toTopOf="parent"> - + android:layout_height="wrap_content"> - - - - - - - - - - - - - - - - - - - + + +