From d841fd0225903791dceb9d06ef7f55344684630f Mon Sep 17 00:00:00 2001 From: Billy Brawner Date: Fri, 8 Nov 2019 09:30:55 -0600 Subject: [PATCH] Fix various concurrency issues and simplify some logic throughout the app --- .../simplemarkdown/MarkdownApplication.kt | 4 - .../view/activity/MainActivity.kt | 19 ++-- .../view/activity/MarkdownInfoActivity.kt | 47 ++++++-- .../view/activity/SplashActivity.kt | 40 +++---- .../view/fragment/EditFragment.kt | 24 ++-- .../view/fragment/MainMenuFragment.kt | 107 +++++++----------- .../view/fragment/PreviewFragment.kt | 33 ++---- .../view/fragment/SettingsFragment.kt | 55 +++------ .../view/overrides/SafeListView.kt | 20 ---- .../viewmodel/MarkdownViewModel.kt | 14 +-- .../res/layout/activity_markdown_info.xml | 1 - app/src/main/res/layout/content_explorer.xml | 20 ---- .../layout/preference_list_fragment_safe.xml | 14 --- 13 files changed, 143 insertions(+), 255 deletions(-) delete mode 100644 app/src/main/java/com/wbrawner/simplemarkdown/view/overrides/SafeListView.kt delete mode 100644 app/src/main/res/layout/content_explorer.xml delete mode 100644 app/src/main/res/layout/preference_list_fragment_safe.xml diff --git a/app/src/main/java/com/wbrawner/simplemarkdown/MarkdownApplication.kt b/app/src/main/java/com/wbrawner/simplemarkdown/MarkdownApplication.kt index 1d033ae..169640c 100644 --- a/app/src/main/java/com/wbrawner/simplemarkdown/MarkdownApplication.kt +++ b/app/src/main/java/com/wbrawner/simplemarkdown/MarkdownApplication.kt @@ -4,12 +4,8 @@ import android.app.Application import android.os.StrictMode import com.wbrawner.simplemarkdown.utility.CrashlyticsErrorHandler import com.wbrawner.simplemarkdown.utility.ErrorHandler -import com.wbrawner.simplemarkdown.viewmodel.MarkdownViewModelFactory class MarkdownApplication : Application() { - val viewModelFactory: MarkdownViewModelFactory by lazy { - MarkdownViewModelFactory() - } val errorHandler: ErrorHandler by lazy { CrashlyticsErrorHandler() } diff --git a/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/MainActivity.kt b/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/MainActivity.kt index f46f972..646754b 100644 --- a/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/MainActivity.kt +++ b/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/MainActivity.kt @@ -20,8 +20,8 @@ import androidx.core.content.ContextCompat import androidx.lifecycle.Observer import androidx.lifecycle.ViewModelProviders import androidx.preference.PreferenceManager -import com.wbrawner.simplemarkdown.MarkdownApplication import com.wbrawner.simplemarkdown.R +import com.wbrawner.simplemarkdown.utility.hideKeyboard import com.wbrawner.simplemarkdown.view.adapter.EditPagerAdapter import com.wbrawner.simplemarkdown.view.fragment.MainMenuFragment import com.wbrawner.simplemarkdown.viewmodel.MarkdownViewModel @@ -49,10 +49,7 @@ class MainActivity : AppCompatActivity(), ActivityCompat.OnRequestPermissionsRes or View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN ) } - viewModel = ViewModelProviders.of( - this, - (application as MarkdownApplication).viewModelFactory - ).get(MarkdownViewModel::class.java) + viewModel = ViewModelProviders.of(this).get(MarkdownViewModel::class.java) val adapter = EditPagerAdapter(supportFragmentManager, this@MainActivity) pager.adapter = adapter pager.addOnPageChangeListener(adapter) @@ -66,6 +63,11 @@ class MainActivity : AppCompatActivity(), ActivityCompat.OnRequestPermissionsRes viewModel.fileName.observe(this, Observer { title = it }) + intent?.data?.let { + launch { + viewModel.load(this@MainActivity, it) + } + } } override fun onUserLeaveHint() { @@ -114,11 +116,8 @@ class MainActivity : AppCompatActivity(), ActivityCompat.OnRequestPermissionsRes override fun onOptionsItemSelected(item: MenuItem): Boolean { when (item.itemId) { android.R.id.home -> { - MainMenuFragment() - .apply { - errorHandler = (application as MarkdownApplication).errorHandler - } - .show(supportFragmentManager, null) + MainMenuFragment().show(supportFragmentManager, null) + window.decorView.hideKeyboard() } R.id.action_save -> { launch { diff --git a/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/MarkdownInfoActivity.kt b/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/MarkdownInfoActivity.kt index 8d06107..6f7addd 100644 --- a/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/MarkdownInfoActivity.kt +++ b/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/MarkdownInfoActivity.kt @@ -3,24 +3,32 @@ package com.wbrawner.simplemarkdown.view.activity import android.content.res.Configuration import android.os.Bundle import android.view.MenuItem +import android.widget.Toast import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.app.AppCompatDelegate +import com.wbrawner.simplemarkdown.MarkdownApplication import com.wbrawner.simplemarkdown.R +import com.wbrawner.simplemarkdown.utility.readAssetToString +import com.wbrawner.simplemarkdown.utility.toHtml import kotlinx.android.synthetic.main.activity_markdown_info.* +import kotlinx.coroutines.* +import kotlin.coroutines.CoroutineContext -class MarkdownInfoActivity : AppCompatActivity() { +class MarkdownInfoActivity : AppCompatActivity(), CoroutineScope { + override val coroutineContext: CoroutineContext = Dispatchers.Main override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setContentView(R.layout.activity_markdown_info) setSupportActionBar(toolbar) supportActionBar?.setDisplayHomeAsUpEnabled(true) - val intent = intent - if (intent == null || !intent.hasExtra("title") || !intent.hasExtra("html")) { + val title = intent?.getStringExtra(EXTRA_TITLE) + val fileName = intent?.getStringExtra(EXTRA_FILE) + if (title.isNullOrBlank() || fileName.isNullOrBlank()) { finish() return } - title = intent.getStringExtra("title") + val isNightMode = AppCompatDelegate.getDefaultNightMode() == AppCompatDelegate.MODE_NIGHT_YES || resources.configuration.uiMode and Configuration.UI_MODE_NIGHT_MASK == Configuration.UI_MODE_NIGHT_YES @@ -30,12 +38,29 @@ class MarkdownInfoActivity : AppCompatActivity() { R.string.pref_custom_css_default } val css: String? = getString(defaultCssId) + launch { + try { + val html = assets?.readAssetToString(fileName) + ?.toHtml() + ?: throw RuntimeException("Unable to open stream to $fileName") + infoWebview.loadDataWithBaseURL(null, + String.format(FORMAT_CSS, css) + html, + "text/html", + "UTF-8", null + ) + } catch (e: Exception) { + (application as MarkdownApplication).errorHandler.reportException(e) + Toast.makeText(this@MarkdownInfoActivity, R.string.file_load_error, Toast.LENGTH_SHORT).show() + finish() + } + } + } - infoWebview.loadDataWithBaseURL(null, - String.format(FORMAT_CSS, css) + intent.getStringExtra("html"), - "text/html", - "UTF-8", null - ) + override fun onDestroy() { + coroutineContext[Job]?.let { + cancel() + } + super.onDestroy() } @@ -48,8 +73,10 @@ class MarkdownInfoActivity : AppCompatActivity() { } companion object { - var FORMAT_CSS = "" + const val EXTRA_TITLE = "title" + const val EXTRA_FILE = "file" } } diff --git a/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/SplashActivity.kt b/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/SplashActivity.kt index 85daafd..f1030ca 100644 --- a/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/SplashActivity.kt +++ b/app/src/main/java/com/wbrawner/simplemarkdown/view/activity/SplashActivity.kt @@ -4,13 +4,10 @@ import android.content.Intent import android.net.Uri import android.os.Build import android.os.Bundle -import android.preference.PreferenceManager import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.app.AppCompatDelegate -import androidx.lifecycle.ViewModelProviders -import com.wbrawner.simplemarkdown.MarkdownApplication +import androidx.preference.PreferenceManager import com.wbrawner.simplemarkdown.R -import com.wbrawner.simplemarkdown.viewmodel.MarkdownViewModel import kotlinx.coroutines.* import kotlin.coroutines.CoroutineContext @@ -18,15 +15,8 @@ class SplashActivity : AppCompatActivity(), CoroutineScope { override val coroutineContext: CoroutineContext = Dispatchers.Main - lateinit var viewModel: MarkdownViewModel - override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - viewModel = ViewModelProviders.of( - this, - (application as MarkdownApplication).viewModelFactory - ).get(MarkdownViewModel::class.java) - launch { val darkMode = withContext(Dispatchers.IO) { val darkModeValue = PreferenceManager.getDefaultSharedPreferences(this@SplashActivity) @@ -40,7 +30,7 @@ class SplashActivity : AppCompatActivity(), CoroutineScope { darkModeValue.equals(getString(R.string.pref_value_dark), ignoreCase = true) -> AppCompatDelegate.MODE_NIGHT_YES else -> { if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { - AppCompatDelegate.MODE_NIGHT_AUTO + AppCompatDelegate.MODE_NIGHT_AUTO_BATTERY } else { AppCompatDelegate.MODE_NIGHT_FOLLOW_SYSTEM } @@ -50,21 +40,21 @@ class SplashActivity : AppCompatActivity(), CoroutineScope { } AppCompatDelegate.setDefaultNightMode(darkMode) - withContext(Dispatchers.IO) { - var uri = intent?.data - if (uri == null) { - uri = PreferenceManager.getDefaultSharedPreferences(this@SplashActivity) - .getString( - getString(R.string.pref_key_autosave_uri), - null - )?.let { - Uri.parse(it) - } - } - - viewModel.load(this@SplashActivity, uri) + val uri = withContext(Dispatchers.IO) { + intent?.data + ?: PreferenceManager.getDefaultSharedPreferences(this@SplashActivity) + .getString( + getString(R.string.pref_key_autosave_uri), + null + )?.let { + Uri.parse(it) + } } + val startIntent = Intent(this@SplashActivity, MainActivity::class.java) + .apply { + data = uri + } startActivity(startIntent) finish() } diff --git a/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/EditFragment.kt b/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/EditFragment.kt index 22a4ce3..2c4fd9c 100644 --- a/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/EditFragment.kt +++ b/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/EditFragment.kt @@ -3,7 +3,6 @@ package com.wbrawner.simplemarkdown.view.fragment import android.annotation.SuppressLint import android.graphics.Color import android.os.Bundle -import android.preference.PreferenceManager import android.text.Editable import android.text.SpannableString import android.text.TextWatcher @@ -19,7 +18,7 @@ import android.widget.TextView import androidx.fragment.app.Fragment import androidx.lifecycle.Observer import androidx.lifecycle.ViewModelProviders -import com.wbrawner.simplemarkdown.MarkdownApplication +import androidx.preference.PreferenceManager import com.wbrawner.simplemarkdown.R import com.wbrawner.simplemarkdown.model.Readability import com.wbrawner.simplemarkdown.utility.hideKeyboard @@ -33,7 +32,7 @@ import kotlin.math.abs class EditFragment : Fragment(), ViewPagerPage, CoroutineScope { private var markdownEditor: EditText? = null private var markdownEditorScroller: ScrollView? = null - private lateinit var viewModel: MarkdownViewModel + lateinit var viewModel: MarkdownViewModel override val coroutineContext: CoroutineContext = Dispatchers.Main private var readabilityWatcher: TextWatcher? = null @@ -45,6 +44,9 @@ class EditFragment : Fragment(), ViewPagerPage, CoroutineScope { @SuppressLint("ClickableViewAccessibility") override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + activity?.let { + viewModel = ViewModelProviders.of(it).get(MarkdownViewModel::class.java) + } ?: return markdownEditor = view.findViewById(R.id.markdown_edit) markdownEditorScroller = view.findViewById(R.id.markdown_edit_container) markdownEditor?.addTextChangedListener(object : TextWatcher { @@ -95,21 +97,9 @@ class EditFragment : Fragment(), ViewPagerPage, CoroutineScope { } false } - } - - override fun onActivityCreated(savedInstanceState: Bundle?) { - super.onActivityCreated(savedInstanceState) - viewModel = ViewModelProviders.of( - this, - (requireActivity().application as MarkdownApplication).viewModelFactory - ).get(MarkdownViewModel::class.java) viewModel.originalMarkdown.observe(this, Observer { markdownEditor?.setText(it) }) - } - - override fun onStart() { - super.onStart() launch { val enableReadability = withContext(Dispatchers.IO) { context?.let { @@ -132,11 +122,11 @@ class EditFragment : Fragment(), ViewPagerPage, CoroutineScope { } } - override fun onDestroy() { + override fun onDestroyView() { coroutineContext[Job]?.let { cancel() } - super.onDestroy() + super.onDestroyView() } override fun onSelected() { diff --git a/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/MainMenuFragment.kt b/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/MainMenuFragment.kt index 77030e6..70f66dd 100644 --- a/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/MainMenuFragment.kt +++ b/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/MainMenuFragment.kt @@ -1,31 +1,21 @@ package com.wbrawner.simplemarkdown.view.fragment -import android.content.Context import android.content.Intent import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import android.widget.Toast import com.google.android.material.bottomsheet.BottomSheetDialogFragment import com.wbrawner.simplemarkdown.R -import com.wbrawner.simplemarkdown.utility.ErrorHandler -import com.wbrawner.simplemarkdown.utility.readAssetToString -import com.wbrawner.simplemarkdown.utility.toHtml import com.wbrawner.simplemarkdown.view.activity.MainActivity import com.wbrawner.simplemarkdown.view.activity.MarkdownInfoActivity +import com.wbrawner.simplemarkdown.view.activity.MarkdownInfoActivity.Companion.EXTRA_FILE +import com.wbrawner.simplemarkdown.view.activity.MarkdownInfoActivity.Companion.EXTRA_TITLE import com.wbrawner.simplemarkdown.view.activity.SettingsActivity import com.wbrawner.simplemarkdown.view.activity.SupportActivity import kotlinx.android.synthetic.main.fragment_menu_main.* -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch -import kotlin.coroutines.CoroutineContext - -class MainMenuFragment : BottomSheetDialogFragment(), CoroutineScope { - override val coroutineContext: CoroutineContext = Dispatchers.Main - lateinit var errorHandler: ErrorHandler +class MainMenuFragment : BottomSheetDialogFragment() { override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -34,20 +24,47 @@ class MainMenuFragment : BottomSheetDialogFragment(), CoroutineScope { override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - mainMenuNavigationView.setNavigationItemSelectedListener { - when (it.itemId) { - R.id.action_help -> showInfoActivity(context, R.id.action_help) - R.id.action_settings -> { - val settingsIntent = Intent(context, SettingsActivity::class.java) - startActivityForResult(settingsIntent, MainActivity.REQUEST_DARK_MODE) - } - R.id.action_libraries -> showInfoActivity(context, R.id.action_libraries) - R.id.action_privacy -> showInfoActivity(context, R.id.action_privacy) - R.id.action_support -> Intent(context, SupportActivity::class.java) - .apply { - startActivity(this) - dialog?.dismiss() - } + mainMenuNavigationView.setNavigationItemSelectedListener { menuItem -> + val (intentClass, fileName, title) = when (menuItem.itemId) { + R.id.action_help -> Triple( + MarkdownInfoActivity::class.java, + "Cheatsheet.md", + R.string.action_help + ) + R.id.action_settings -> Triple( + SettingsActivity::class.java, + null, + null + ) + R.id.action_libraries -> Triple( + MarkdownInfoActivity::class.java, + "Libraries.md", + R.id.action_libraries + ) + R.id.action_privacy -> Triple( + MarkdownInfoActivity::class.java, + "Privacy Policy.md", + R.id.action_libraries + ) + R.id.action_support -> Triple( + SupportActivity::class.java, + null, + null + ) + else -> throw IllegalStateException("This shouldn't happen") + } + val intent = Intent(context, intentClass) + fileName?.let { + intent.putExtra(EXTRA_FILE, it) + } + title?.let { + intent.putExtra(EXTRA_TITLE, getString(it)) + } + if (intentClass == SettingsActivity::class.java) { + startActivityForResult(intent, MainActivity.REQUEST_DARK_MODE) + } else { + startActivity(intent) + dialog?.dismiss() } true } @@ -61,40 +78,4 @@ class MainMenuFragment : BottomSheetDialogFragment(), CoroutineScope { } super.onActivityResult(requestCode, resultCode, data) } - - private fun showInfoActivity(context: Context?, action: Int) { - val infoIntent = Intent(context, MarkdownInfoActivity::class.java) - var fileName = "" - var title = "" - when (action) { - R.id.action_help -> { - fileName = "Cheatsheet.md" - title = getString(R.string.action_help) - } - R.id.action_libraries -> { - fileName = "Libraries.md" - title = getString(R.string.action_libraries) - } - R.id.action_privacy -> { - fileName = "Privacy Policy.md" - title = getString(R.string.action_privacy) - } - } - infoIntent.putExtra("title", title) - launch { - // TODO: Refactor this to have the info activity load the markdown instead of doing - // it here - try { - val html = context?.assets?.readAssetToString(fileName) - ?.toHtml() - ?: throw RuntimeException("Unable to open stream to $fileName") - infoIntent.putExtra("html", html) - startActivity(infoIntent, null) - } catch (e: Exception) { - errorHandler.reportException(e) - Toast.makeText(context, R.string.file_load_error, Toast.LENGTH_SHORT).show() - } - dialog?.dismiss() - } - } } \ No newline at end of file diff --git a/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/PreviewFragment.kt b/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/PreviewFragment.kt index 3d6726f..e5f963b 100644 --- a/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/PreviewFragment.kt +++ b/app/src/main/java/com/wbrawner/simplemarkdown/view/fragment/PreviewFragment.kt @@ -3,7 +3,6 @@ package com.wbrawner.simplemarkdown.view.fragment import android.content.res.Configuration.UI_MODE_NIGHT_MASK import android.content.res.Configuration.UI_MODE_NIGHT_YES import android.os.Bundle -import android.preference.PreferenceManager import android.view.LayoutInflater import android.view.View import android.view.ViewGroup @@ -12,8 +11,8 @@ import androidx.appcompat.app.AppCompatDelegate import androidx.fragment.app.Fragment import androidx.lifecycle.Observer import androidx.lifecycle.ViewModelProviders +import androidx.preference.PreferenceManager import com.wbrawner.simplemarkdown.BuildConfig -import com.wbrawner.simplemarkdown.MarkdownApplication import com.wbrawner.simplemarkdown.R import com.wbrawner.simplemarkdown.utility.toHtml import com.wbrawner.simplemarkdown.viewmodel.MarkdownViewModel @@ -35,14 +34,9 @@ class PreviewFragment : Fragment(), CoroutineScope { override fun onViewCreated(view: View, savedInstanceState: Bundle?) { markdownPreview = view.findViewById(R.id.markdown_view) WebView.setWebContentsDebuggingEnabled(BuildConfig.DEBUG) - } - - override fun onActivityCreated(savedInstanceState: Bundle?) { - super.onActivityCreated(savedInstanceState) - viewModel = ViewModelProviders.of( - this, - (requireActivity().application as MarkdownApplication).viewModelFactory - ).get(MarkdownViewModel::class.java) + activity?.let { + viewModel = ViewModelProviders.of(it).get(MarkdownViewModel::class.java) + } ?: return launch { val isNightMode = AppCompatDelegate.getDefaultNightMode() == AppCompatDelegate.MODE_NIGHT_YES @@ -53,18 +47,19 @@ class PreviewFragment : Fragment(), CoroutineScope { R.string.pref_custom_css_default } val css = withContext(Dispatchers.IO) { + val context = context ?: return@withContext null @Suppress("ConstantConditionIf") if (!BuildConfig.ENABLE_CUSTOM_CSS) { - requireActivity().getString(defaultCssId) + context.getString(defaultCssId) } else { - PreferenceManager.getDefaultSharedPreferences(requireActivity()) + PreferenceManager.getDefaultSharedPreferences(context) .getString( getString(R.string.pref_custom_css), getString(defaultCssId) - ) ?: "" + ) } } - style = String.format(FORMAT_CSS, css) + style = String.format(FORMAT_CSS, css ?: "") updateWebContent(viewModel.markdownUpdates.value ?: "") viewModel.markdownUpdates.observe(this@PreviewFragment, Observer { updateWebContent(it) @@ -85,6 +80,9 @@ class PreviewFragment : Fragment(), CoroutineScope { } override fun onDestroyView() { + coroutineContext[Job]?.let { + cancel() + } markdownPreview?.let { (it.parent as ViewGroup).removeView(it) it.destroy() @@ -93,13 +91,6 @@ class PreviewFragment : Fragment(), CoroutineScope { super.onDestroyView() } - override fun onDestroy() { - coroutineContext[Job]?.let { - cancel() - } - super.onDestroy() - } - companion object { var FORMAT_CSS = "