From e129619ec05bb01dca9dd6c04ad17efc5ffce7c2 Mon Sep 17 00:00:00 2001 From: Wiktor Wichrowski Date: Sun, 15 Mar 2026 22:31:51 +0100 Subject: [PATCH] improve page loading and error handling in EditorView and EditorViewModel - Refactor `loadBookData` to be a suspend function that returns a success boolean. - Update `EditorView` to handle missing pages by triggering a notebook fix and navigating to the library. - Implement centralized error logging and snackbar notifications via `SnackState.logAndShowError`. - Add debug logging to `CanvasObserverRegistry` and `QuickNavViewModel` for canvas restoration and scrubbing. - Simplify `EditorView` logic by using a single entry point for book data loading based on `pageId`. --- .../ethran/notable/data/PageDataManager.kt | 16 ++--- .../com/ethran/notable/editor/EditorView.kt | 43 +++++------ .../ethran/notable/editor/EditorViewModel.kt | 71 ++++++++++++------- .../editor/canvas/CanvasObserverRegistry.kt | 2 + .../ui/viewmodels/QuickNavViewModel.kt | 3 + 5 files changed, 74 insertions(+), 61 deletions(-) diff --git a/app/src/main/java/com/ethran/notable/data/PageDataManager.kt b/app/src/main/java/com/ethran/notable/data/PageDataManager.kt index 49115a2f..05a21ba5 100644 --- a/app/src/main/java/com/ethran/notable/data/PageDataManager.kt +++ b/app/src/main/java/com/ethran/notable/data/PageDataManager.kt @@ -320,8 +320,11 @@ object PageDataManager { // 3) Reconcile: if they disagree, warn and clear if (jobSnapshot.isNotNull() && dataLoaded != jobDone) { - log.e("Inconsistent state for page($pageId): dataLoaded=$dataLoaded, jobDone=$jobDone, job=$jobSnapshot") - showHint("Fixing inconsistent page state: $pageId") + SnackState.logAndShowError( + "PageDataManager.validatePageDataLoaded", + "Inconsistent state for page($pageId): dataLoaded=$dataLoaded," + + " jobDone=$jobDone, job=$jobSnapshot, trying to fix." + ) dataLoadingScope.launch { // Cancel/remove any job for this page jobLock.withLock { @@ -707,12 +710,9 @@ object PageDataManager { fun removePage(pageId: String): Boolean { log.d("Removing page $pageId") if (pageId == currentPage) { - log.e("Removing current page!") - SnackState.globalSnackFlow.tryEmit( - SnackConf( - text = "Cannot remove current page, there is a bug in code", - duration = 3000 - ) + SnackState.logAndShowError( + "PageDataManager.removePage", + "Cannot remove current page, there is a bug in code", ) return false } diff --git a/app/src/main/java/com/ethran/notable/editor/EditorView.kt b/app/src/main/java/com/ethran/notable/editor/EditorView.kt index 50608bdd..f42f45b4 100644 --- a/app/src/main/java/com/ethran/notable/editor/EditorView.kt +++ b/app/src/main/java/com/ethran/notable/editor/EditorView.kt @@ -35,19 +35,15 @@ import com.ethran.notable.io.exportToLinkedFile import com.ethran.notable.navigation.NavigationDestination import com.ethran.notable.ui.LocalSnackContext import com.ethran.notable.ui.SnackConf -import com.ethran.notable.ui.SnackState import com.ethran.notable.ui.convertDpToPixel import com.ethran.notable.ui.theme.InkaTheme import com.ethran.notable.ui.views.BugReportDestination import com.ethran.notable.ui.views.LibraryDestination import com.ethran.notable.ui.views.PagesDestination import io.shipbook.shipbooksdk.ShipBook -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.filterNotNull -import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext private val log = ShipBook.getLogger("EditorView") @@ -86,38 +82,30 @@ fun EditorView( val scope = rememberCoroutineScope() var pageExists by remember(pageId) { mutableStateOf(null) } + + + // Single point of entry for loading book data based on the pageId from Navigation LaunchedEffect(pageId) { - viewModel.loadBookData(bookId, pageId) - val exists = withContext(Dispatchers.IO) { - appRepository.pageRepository.getById(pageId) != null - } - pageExists = exists - - if (!exists) { - // TODO: check if it is correct, and remove exeption throwing - throw Exception("Page does not exist") - if (bookId != null) { - // clean the book - log.i("Could not find page, Cleaning book") - SnackState.globalSnackFlow.tryEmit( - SnackConf( - text = "Could not find page, cleaning book", duration = 4000 - ) - ) - scope.launch(Dispatchers.IO) { - appRepository.bookRepository.removePage(bookId, pageId) - } - } + log.v("EditorView: pageId changed to $pageId, loading data") + val success = viewModel.loadBookData(bookId, pageId) + pageExists = success + if (!success) { + viewModel.fixNotebook(bookId, pageId) navController.navigate(LibraryDestination.route) - } + } else + log.v("EditorView: data loaded successfully") } + if (pageExists == null){ + log.e("Page does not exist: $pageId") + return + } + // Sync isQuickNavOpen to ViewModel LaunchedEffect(isQuickNavOpen) { viewModel.onToolbarAction(ToolbarAction.UpdateQuickNavOpen(isQuickNavOpen)) } - if (pageExists == null) return BoxWithConstraints { val height = convertDpToPixel(this.maxHeight, context).toInt() @@ -230,6 +218,7 @@ fun EditorView( .distinctUntilChanged() .drop(1) // Skip initial emission from loadBookData .collect { newPageId -> + log.v("EditorView: snapshotFlow detected pageId change to $newPageId, triggering onPageChange") onPageChange(newPageId) } } diff --git a/app/src/main/java/com/ethran/notable/editor/EditorViewModel.kt b/app/src/main/java/com/ethran/notable/editor/EditorViewModel.kt index d94bce31..f70aa1f3 100644 --- a/app/src/main/java/com/ethran/notable/editor/EditorViewModel.kt +++ b/app/src/main/java/com/ethran/notable/editor/EditorViewModel.kt @@ -23,6 +23,8 @@ import com.ethran.notable.editor.utils.PenSetting import com.ethran.notable.io.ExportEngine import com.ethran.notable.io.ExportFormat import com.ethran.notable.io.ExportTarget +import com.ethran.notable.ui.SnackConf +import com.ethran.notable.ui.SnackState import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.qualifiers.ApplicationContext import io.shipbook.shipbooksdk.Log @@ -154,7 +156,7 @@ sealed class EditorUiEvent { @HiltViewModel class EditorViewModel @Inject constructor( @param:ApplicationContext private val context: Context, - private val appRepository: AppRepository, + val appRepository: AppRepository, private val exportEngine: ExportEngine ) : ViewModel() { @@ -411,40 +413,55 @@ class EditorViewModel @Inject constructor( /** * Loads context data for the toolbar (page number, background info, etc.) + * returns false if book does not exists. */ - fun loadBookData(bookId: String?, pageId: String) { + suspend fun loadBookData(bookId: String?, pageId: String): Boolean { log.v("loadBookData: bookId=$bookId, pageId=$pageId") this.bookId = bookId - viewModelScope.launch(Dispatchers.IO) { - val page = appRepository.pageRepository.getById(pageId) - val book = bookId?.let { appRepository.bookRepository.getById(it) } + val page = appRepository.pageRepository.getById(pageId) ?: return false + val book = bookId?.let { appRepository.bookRepository.getById(it) } - val pageIndex = book?.getPageIndex(pageId) ?: 0 - val totalPages = book?.pageIds?.size ?: 1 + val pageIndex = book?.getPageIndex(pageId) ?: 0 + val totalPages = book?.pageIds?.size ?: 1 - val backgroundTypeObj = BackgroundType.fromKey(page?.backgroundType ?: "native") - val bgPageNumber = when (backgroundTypeObj) { - is BackgroundType.Pdf -> backgroundTypeObj.page - is BackgroundType.AutoPdf -> { - bookId?.let { appRepository.getPageNumber(it, pageId) } ?: 0 - } - - else -> 0 + val backgroundTypeObj = BackgroundType.fromKey(page.backgroundType) + val bgPageNumber = when (backgroundTypeObj) { + is BackgroundType.Pdf -> backgroundTypeObj.page + is BackgroundType.AutoPdf -> { + bookId?.let { appRepository.getPageNumber(it, pageId) } ?: 0 } - _toolbarState.update { - it.copy( - notebookId = bookId, - pageId = pageId, - isBookActive = bookId != null, - pageNumberInfo = if (bookId != null) "${pageIndex + 1}/$totalPages" else "1/1", - currentPageNumber = pageIndex, - backgroundType = page?.backgroundType ?: "native", - backgroundPath = page?.background ?: "blank", - backgroundPageNumber = bgPageNumber + else -> 0 + } + + _toolbarState.update { + it.copy( + notebookId = bookId, + pageId = pageId, + isBookActive = bookId != null, + pageNumberInfo = if (bookId != null) "${pageIndex + 1}/$totalPages" else "1/1", + currentPageNumber = pageIndex, + backgroundType = page.backgroundType, + backgroundPath = page.background, + backgroundPageNumber = bgPageNumber + ) + } + return true + } + + /** + * Attempts to repair potential inconsistencies in the notebook's data structure. + */ + suspend fun fixNotebook(bookId: String?, pageId: String) { + if (bookId != null) { + log.i("Could not find page, Cleaning book") + SnackState.globalSnackFlow.tryEmit( + SnackConf( + text = "Could not find page, cleaning book", duration = 4000 ) - } + ) + appRepository.bookRepository.removePage(bookId, pageId) } } @@ -489,8 +506,10 @@ class EditorViewModel @Inject constructor( appRepository.bookRepository.setOpenPageId(bookId!!, newPageId) } if (newPageId != currentPageId) { + // The View's LaunchedEffect will handle the full load once navigation syncs. Log.d("EditorView", "Page changed") loadBookData(bookId, newPageId) +// _toolbarState.update { it.copy(pageId = newPageId) } } else { Log.d("EditorView", "Tried to change to same page!") sendUiEvent(EditorUiEvent.ShowSnackbar("Tried to change to same page!")) diff --git a/app/src/main/java/com/ethran/notable/editor/canvas/CanvasObserverRegistry.kt b/app/src/main/java/com/ethran/notable/editor/canvas/CanvasObserverRegistry.kt index a19af78b..e382af6e 100644 --- a/app/src/main/java/com/ethran/notable/editor/canvas/CanvasObserverRegistry.kt +++ b/app/src/main/java/com/ethran/notable/editor/canvas/CanvasObserverRegistry.kt @@ -290,8 +290,10 @@ class CanvasObserverRegistry( private fun observeRestoreCanvas() { coroutineScope.launch { CanvasEventBus.restoreCanvas.collect { + Log.d("QuickNav", "Restoring canvas") val zoneToRedraw = Rect(0, 0, page.viewWidth, page.viewHeight) drawCanvas.restoreCanvas(zoneToRedraw) + logCanvasObserver.v("Restored canvas") } } } diff --git a/app/src/main/java/com/ethran/notable/ui/viewmodels/QuickNavViewModel.kt b/app/src/main/java/com/ethran/notable/ui/viewmodels/QuickNavViewModel.kt index 8477aa01..551e1339 100644 --- a/app/src/main/java/com/ethran/notable/ui/viewmodels/QuickNavViewModel.kt +++ b/app/src/main/java/com/ethran/notable/ui/viewmodels/QuickNavViewModel.kt @@ -11,6 +11,7 @@ import com.ethran.notable.editor.canvas.CanvasEventBus import com.ethran.notable.ui.SnackConf import com.ethran.notable.ui.SnackState import com.ethran.notable.ui.components.getFolderList +import io.shipbook.shipbooksdk.ShipBook import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -42,6 +43,7 @@ class QuickNavViewModel( private val pageRepository = appRepository.pageRepository private val bookRepository = appRepository.bookRepository private val kv = appRepository.kvProxy + private val log = ShipBook.getLogger("EditorControlTower") private val _uiState = MutableStateFlow(QuickNavUiState()) val uiState: StateFlow = _uiState.asStateFlow() @@ -142,6 +144,7 @@ class QuickNavViewModel( } fun onScrubEnd(index: Int) { + log.e("onScrubEnd: $index") CanvasEventBus.restoreCanvas.tryEmit(Unit) val pageIds = _uiState.value.bookPageIds if (index in pageIds.indices) {