From 6387c5ceaed6b6f7813a538f680838b69ff073bd Mon Sep 17 00:00:00 2001 From: MohitMaliFtechiz Date: Tue, 21 Jan 2025 15:55:18 +0530 Subject: [PATCH] Removed the unnecessary creation of ZimFileReader when opening notes on the Notes screen. * Refactored `AddNoteDialog` to use database values for performing note-related operations (e.g., view, edit, delete notes) instead of setting and using `ZimFileReader`. --- .../kiwixmobile/core/main/AddNoteDialog.kt | 99 ++++++++++++------- .../core/page/notes/adapter/NoteListItem.kt | 16 +-- .../effects/ShowDeleteNotesDialog.kt | 2 - .../viewmodel/effects/ShowOpenNoteDialog.kt | 38 +------ .../core/page/viewmodel/PageViewModel.kt | 6 +- .../core/page/viewmodel/effects/OpenNote.kt | 13 +-- 6 files changed, 80 insertions(+), 94 deletions(-) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/AddNoteDialog.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/AddNoteDialog.kt index b18b34607..504c1c4b2 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/AddNoteDialog.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/AddNoteDialog.kt @@ -25,7 +25,6 @@ import android.content.pm.PackageManager import android.os.Build import android.os.Bundle import android.text.Editable -import org.kiwix.kiwixmobile.core.utils.files.Log import android.view.LayoutInflater import android.view.MenuItem import android.view.View @@ -49,10 +48,12 @@ import org.kiwix.kiwixmobile.core.extensions.snack import org.kiwix.kiwixmobile.core.extensions.toast import org.kiwix.kiwixmobile.core.page.notes.adapter.NoteListItem import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer +import org.kiwix.kiwixmobile.core.reader.ZimReaderSource import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import org.kiwix.kiwixmobile.core.utils.SimpleTextWatcher import org.kiwix.kiwixmobile.core.utils.dialog.AlertDialogShower import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog +import org.kiwix.kiwixmobile.core.utils.files.Log import java.io.File import java.io.IOException import javax.inject.Inject @@ -110,6 +111,10 @@ class AddNoteDialog : DialogFragment() { private val deleteItem by lazy { toolbar?.menu?.findItem(R.id.delete_note) } + private var noteListItem: NoteListItem? = null + private var zimReaderSource: ZimReaderSource? = null + private var favicon: String? = null + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) coreComponent @@ -118,35 +123,45 @@ class AddNoteDialog : DialogFragment() { .build() .inject(this) - // Returns name of the form ".../Kiwix/granbluefantasy_en_all_all_nopic_2018-10.zim" - zimFileName = zimReaderContainer.zimReaderSource?.toDatabase() ?: zimReaderContainer.name - if (zimFileName != null) { // No zim file currently opened + if (arguments != null) { + // For opening the note dialog from note screen. + noteListItem = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + arguments?.getSerializable(NOTE_LIST_ITEM_TAG, NoteListItem::class.java) + } else { + @Suppress("DEPRECATION") + arguments?.getSerializable(NOTE_LIST_ITEM_TAG) as NoteListItem + } + + zimFileName = noteListItem?.zimReaderSource?.toDatabase() + zimFileTitle = noteListItem?.title + zimId = noteListItem?.zimId.orEmpty() + zimReaderSource = noteListItem?.zimReaderSource + favicon = noteListItem?.favicon + articleNoteFileName = getArticleNoteFileName() + zimNotesDirectory = noteListItem?.noteFilePath + ?.substringBefore(articleNoteFileName) + getArticleTitleAndZimFileUrlFromArguments() + } else { + // Note is opened from the reader screen. + // Returns name of the form ".../Kiwix/granbluefantasy_en_all_all_nopic_2018-10.zim" + zimFileName = zimReaderContainer.zimReaderSource?.toDatabase() ?: zimReaderContainer.name zimFileTitle = zimReaderContainer.zimFileTitle zimId = zimReaderContainer.id.orEmpty() - - if (arguments != null) { - getArticleTitleAndZimFileUrlFromArguments() - } else { - val webView = (activity as WebViewProvider?)?.getCurrentWebView() - articleTitle = webView?.title - zimFileUrl = webView?.url.orEmpty() - } + zimReaderSource = zimReaderContainer.zimReaderSource + favicon = zimReaderContainer.favicon + val webView = (activity as WebViewProvider?)?.getCurrentWebView() + articleTitle = webView?.title + zimFileUrl = webView?.url.orEmpty() // Corresponds to "ZimFileName" of "{External Storage}/Kiwix/Notes/ZimFileName/ArticleUrl.txt" articleNoteFileName = getArticleNoteFileName() zimNotesDirectory = "$NOTES_DIRECTORY$zimNoteDirectoryName/" - } else { - articleNoteFileName = getArticleNoteFileName() - zimNotesDirectory = arguments?.getString(NOTE_FILE_PATH) - ?.substringBefore(articleNoteFileName) - getArticleTitleAndZimFileUrlFromArguments() - context.toast(R.string.error_file_not_found, Toast.LENGTH_LONG) } } private fun getArticleTitleAndZimFileUrlFromArguments() { - articleTitle = arguments?.getString(NOTES_TITLE)?.substringAfter(": ") - zimFileUrl = arguments?.getString(ARTICLE_URL).orEmpty() + articleTitle = noteListItem?.title?.substringAfter(": ") + zimFileUrl = noteListItem?.zimUrl.orEmpty() } private fun isZimFileExist() = zimFileName != null @@ -174,7 +189,7 @@ class AddNoteDialog : DialogFragment() { private fun getArticleNoteFileName(): String { // Returns url of the form: "content://org.kiwix.kiwixmobile.zim.base/A/Main_Page.html" - arguments?.getString(NOTE_FILE_PATH)?.let { + noteListItem?.noteFilePath?.let { return@getArticleNoteFileName getTextAfterLastSlashWithoutExtension(it) } @@ -365,7 +380,7 @@ class AddNoteDialog : DialogFragment() { noteEdited = false // As no unsaved changes remain enableDeleteNoteMenuItem() // adding only if saving file is success - addNoteToDao(noteFile.canonicalPath, "${zimFileTitle.orEmpty()}: $articleTitle") + addNoteToDao(noteFile.canonicalPath, getNoteTitle()) disableSaveNoteMenuItem() } catch (e: IOException) { e.printStackTrace() @@ -380,20 +395,34 @@ class AddNoteDialog : DialogFragment() { } } + /** + * This method determines the note title to be saved in the database. + * - If the note is opened from the Reader screen, it combines the `zimFileTitle` + * and `articleTitle`, as it previously did. + * - If `noteListItem` is not null, it means the note is opened from the Notes screen, + * as this item is passed in the bundle from the Notes screen. In this case, it + * returns `zimFileTitle`, which represents the current note's title. + */ + private fun getNoteTitle(): String = + noteListItem?.let { + zimFileTitle + } ?: run { + "${zimFileTitle.orEmpty()}: $articleTitle" + } + private fun addNoteToDao(noteFilePath: String?, title: String) { noteFilePath?.let { filePath -> if (filePath.isNotEmpty() && zimFileUrl.isNotEmpty()) { - val zimReader = zimReaderContainer.zimFileReader - if (zimReader != null) { - val noteToSave = NoteListItem( - title = title, - url = zimFileUrl, - noteFilePath = noteFilePath, - zimFileReader = zimReader - ) - mainRepositoryActions.saveNote(noteToSave) - } else { - Log.d(TAG, "zim reader found null") + val noteToSave = NoteListItem( + zimId = zimId, + title = title, + url = zimFileUrl, + noteFilePath = noteFilePath, + zimReaderSource = zimReaderSource, + favicon = favicon, + ) + mainRepositoryActions.saveNote(noteToSave).also { + Log.e(TAG, "addNoteToDao: $noteToSave") } } else { Log.d(TAG, "Cannot process with empty zim url or noteFilePath") @@ -514,8 +543,6 @@ class AddNoteDialog : DialogFragment() { @JvmField val NOTES_DIRECTORY = instance.getExternalFilesDir("").toString() + "/Kiwix/Notes/" const val TAG = "AddNoteDialog" - const val NOTE_FILE_PATH = "NoteFilePath" - const val ARTICLE_URL = "ArticleUrl" - const val NOTES_TITLE = "NotesTitle" + const val NOTE_LIST_ITEM_TAG = "NoteListItemTag" } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/adapter/NoteListItem.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/adapter/NoteListItem.kt index c1b6cac59..d1e18adba 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/adapter/NoteListItem.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/adapter/NoteListItem.kt @@ -3,8 +3,8 @@ package org.kiwix.kiwixmobile.core.page.notes.adapter import org.kiwix.kiwixmobile.core.dao.entities.NotesEntity import org.kiwix.kiwixmobile.core.dao.entities.NotesRoomEntity import org.kiwix.kiwixmobile.core.page.adapter.Page -import org.kiwix.kiwixmobile.core.reader.ZimFileReader import org.kiwix.kiwixmobile.core.reader.ZimReaderSource +import java.io.Serializable data class NoteListItem( val databaseId: Long = 0L, @@ -17,7 +17,7 @@ data class NoteListItem( override var isSelected: Boolean = false, override val url: String = zimUrl, override val id: Long = databaseId -) : Page { +) : Page, Serializable { constructor(notesEntity: NotesEntity) : this( notesEntity.id, @@ -30,16 +30,18 @@ data class NoteListItem( ) constructor( + zimId: String, title: String, + zimReaderSource: ZimReaderSource?, url: String, - noteFilePath: String, - zimFileReader: ZimFileReader + favicon: String?, + noteFilePath: String ) : this( - zimId = zimFileReader.id, + zimId = zimId, title = title, - zimReaderSource = zimFileReader.zimReaderSource, + zimReaderSource = zimReaderSource, zimUrl = url, - favicon = zimFileReader.favicon, + favicon = favicon, noteFilePath = noteFilePath ) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowDeleteNotesDialog.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowDeleteNotesDialog.kt index 78c2116dc..8870b214d 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowDeleteNotesDialog.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowDeleteNotesDialog.kt @@ -29,7 +29,6 @@ import org.kiwix.kiwixmobile.core.page.viewmodel.effects.DeletePageItems import org.kiwix.kiwixmobile.core.utils.dialog.DialogShower import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog.DeleteAllNotes import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog.DeleteSelectedNotes -import org.kiwix.kiwixmobile.core.utils.files.Log import javax.inject.Inject data class ShowDeleteNotesDialog( @@ -41,7 +40,6 @@ data class ShowDeleteNotesDialog( @Inject lateinit var dialogShower: DialogShower override fun invokeWith(activity: AppCompatActivity) { activity.cachedComponent.inject(this) - Log.d("invoke", "invokeWith: invoked") dialogShower.show( if (state.isInSelectionState) DeleteSelectedNotes else DeleteAllNotes, { diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt index cb8990da6..f662824b6 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/notes/viewmodel/effects/ShowOpenNoteDialog.kt @@ -20,21 +20,13 @@ package org.kiwix.kiwixmobile.core.page.notes.viewmodel.effects import androidx.appcompat.app.AppCompatActivity import io.reactivex.processors.PublishProcessor -import org.json.JSONArray import org.kiwix.kiwixmobile.core.base.SideEffect import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.cachedComponent -import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.isCustomApp import org.kiwix.kiwixmobile.core.page.adapter.Page import org.kiwix.kiwixmobile.core.page.notes.adapter.NoteListItem import org.kiwix.kiwixmobile.core.page.viewmodel.effects.OpenNote import org.kiwix.kiwixmobile.core.page.viewmodel.effects.OpenPage -import org.kiwix.kiwixmobile.core.reader.ZimFileReader.Companion.CONTENT_PREFIX import org.kiwix.kiwixmobile.core.reader.ZimReaderContainer -import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil -import org.kiwix.kiwixmobile.core.utils.TAG_CURRENT_ARTICLES -import org.kiwix.kiwixmobile.core.utils.TAG_CURRENT_FILE -import org.kiwix.kiwixmobile.core.utils.TAG_CURRENT_POSITIONS -import org.kiwix.kiwixmobile.core.utils.TAG_CURRENT_TAB import org.kiwix.kiwixmobile.core.utils.dialog.DialogShower import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog.ShowNoteDialog import javax.inject.Inject @@ -52,35 +44,7 @@ data class ShowOpenNoteDialog( { effects.offer(OpenPage(page, zimReaderContainer)) }, { val item = page as NoteListItem - // Check if toDatabase is not null, and then set it in zimReaderContainer. - // For custom apps, we are currently using fileDescriptor, and they only have a single file in them, - // which is already set in zimReaderContainer, so there's no need to set it again. - item.zimReaderSource?.toDatabase().let { - val currentZimReaderSource = zimReaderContainer.zimReaderSource - if (!activity.isCustomApp()) { - zimReaderContainer.setZimReaderSource(item.zimReaderSource) - } - if (zimReaderContainer.zimReaderSource != currentZimReaderSource) { - // if current zim file is not the same set the main page of that zim file - // so that when we go back it properly loads the article, and do nothing if the - // zim file is same because there might be multiple tabs opened. - val settings = activity.getSharedPreferences( - SharedPreferenceUtil.PREF_KIWIX_MOBILE, - 0 - ) - val editor = settings.edit() - val urls = JSONArray() - val positions = JSONArray() - urls.put(CONTENT_PREFIX + zimReaderContainer.mainPage) - positions.put(0) - editor.putString(TAG_CURRENT_FILE, zimReaderContainer.zimReaderSource?.toDatabase()) - editor.putString(TAG_CURRENT_ARTICLES, "$urls") - editor.putString(TAG_CURRENT_POSITIONS, "$positions") - editor.putInt(TAG_CURRENT_TAB, 0) - editor.apply() - } - } - effects.offer(OpenNote(item.noteFilePath, item.zimUrl, item.title)) + effects.offer(OpenNote(item)) } ) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/viewmodel/PageViewModel.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/viewmodel/PageViewModel.kt index 95379f6c3..25ce27bce 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/viewmodel/PageViewModel.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/viewmodel/PageViewModel.kt @@ -110,12 +110,12 @@ abstract class PageViewModel>( abstract fun copyWithNewItems(state: S, newItems: List): S private fun handleItemClick(state: S, action: Action.OnItemClick): S { + if (state.isInSelectionState) { + return copyWithNewItems(state, state.getItemsAfterToggleSelectionOfItem(action.page)) + } if (::pageViewModelClickListener.isInitialized) { effects.offer(pageViewModelClickListener.onItemClick(action.page)) } else { - if (state.isInSelectionState) { - return copyWithNewItems(state, state.getItemsAfterToggleSelectionOfItem(action.page)) - } effects.offer(OpenPage(action.page, zimReaderContainer)) } return state diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/page/viewmodel/effects/OpenNote.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/page/viewmodel/effects/OpenNote.kt index fe15a416a..38e11c80c 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/page/viewmodel/effects/OpenNote.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/page/viewmodel/effects/OpenNote.kt @@ -24,15 +24,12 @@ import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentTransaction import org.kiwix.kiwixmobile.core.base.SideEffect import org.kiwix.kiwixmobile.core.main.AddNoteDialog -import org.kiwix.kiwixmobile.core.main.AddNoteDialog.Companion.ARTICLE_URL -import org.kiwix.kiwixmobile.core.main.AddNoteDialog.Companion.NOTES_TITLE -import org.kiwix.kiwixmobile.core.main.AddNoteDialog.Companion.NOTE_FILE_PATH +import org.kiwix.kiwixmobile.core.main.AddNoteDialog.Companion.NOTE_LIST_ITEM_TAG import org.kiwix.kiwixmobile.core.main.CoreMainActivity +import org.kiwix.kiwixmobile.core.page.notes.adapter.NoteListItem class OpenNote( - private val noteFilePath: String, - private val zimFileUrl: String, - private val title: String, + private val noteListItem: NoteListItem ) : SideEffect { override fun invokeWith(activity: AppCompatActivity) { activity as CoreMainActivity @@ -49,9 +46,7 @@ class OpenNote( if (previousInstance == null) { val dialogFragment = AddNoteDialog() val bundle = Bundle().apply { - putString(NOTE_FILE_PATH, noteFilePath) - putString(ARTICLE_URL, zimFileUrl) - putString(NOTES_TITLE, title) + putSerializable(NOTE_LIST_ITEM_TAG, noteListItem) } dialogFragment.arguments = bundle dialogFragment.show(fragmentTransaction, AddNoteDialog.TAG)