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/EditorControlTower.kt b/app/src/main/java/com/ethran/notable/editor/EditorControlTower.kt index f458f694..50fab573 100644 --- a/app/src/main/java/com/ethran/notable/editor/EditorControlTower.kt +++ b/app/src/main/java/com/ethran/notable/editor/EditorControlTower.kt @@ -39,20 +39,27 @@ class EditorControlTower( private var scrollInProgress = Mutex() private var scrollJob: Job? = null private val logEditorControlTower = ShipBook.getLogger("EditorControlTower") - - + private var changePageObserverJob: Job? = null fun registerObservers() { - scope.launch { + if (changePageObserverJob?.isActive == true) return + + changePageObserverJob = scope.launch { CanvasEventBus.changePage.collect { pageId -> logEditorControlTower.d("Change to page $pageId") switchPage(pageId) - page.changePage(pageId) +// page.changePage(pageId) refreshScreen() } } } + // TODO: remove it, change to proper solution + fun unregisterObservers() { + changePageObserverJob?.cancel() + changePageObserverJob = null + } + // returns delta if could not scroll, to be added to next request, // this ensures that smooth scroll works reliably even if rendering takes to long fun processScroll(delta: Offset): Offset { @@ -96,10 +103,10 @@ class EditorControlTower( history.cleanHistory() } - // Switch to (or ensure we are on) IO thread for Database operations - withContext(Dispatchers.IO) { - page.changePage(id) - } +// // Switch to (or ensure we are on) IO thread for Database operations +// withContext(Dispatchers.IO) { +// page.changePage(id) +// } } fun setIsDrawing(value: Boolean) { 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..6f68f2f8 100644 --- a/app/src/main/java/com/ethran/notable/editor/EditorView.kt +++ b/app/src/main/java/com/ethran/notable/editor/EditorView.kt @@ -9,16 +9,13 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope -import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle -import androidx.navigation.NavController import com.ethran.notable.data.AppRepository import com.ethran.notable.data.datastore.EditorSettingCacheManager import com.ethran.notable.editor.canvas.CanvasEventBus @@ -35,19 +32,12 @@ 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") @@ -69,47 +59,35 @@ object EditorDestination : NavigationDestination { } + @Composable fun EditorView( - editorSettingCacheManager: EditorSettingCacheManager, - exportEngine: ExportEngine, - navController: NavController, - appRepository: AppRepository, + initialPageId: String, bookId: String?, - pageId: String, isQuickNavOpen: Boolean, + + // navigation callbacks onPageChange: (String) -> Unit, + goToLibrary: (folderId: String?) -> Unit, + goToPages: (bookId: String) -> Unit, + goToBugReport: () -> Unit, + + // TODO: remove those arguments + editorSettingCacheManager: EditorSettingCacheManager, + exportEngine: ExportEngine, + appRepository: AppRepository, + viewModel: EditorViewModel = hiltViewModel() ) { val context = LocalContext.current val snackManager = LocalSnackContext.current val scope = rememberCoroutineScope() - var pageExists by remember(pageId) { mutableStateOf(null) } - 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) - } - } - navController.navigate(LibraryDestination.route) - } + // Single point of entry for loading book data based on the pageId from Navigation + // Should not be used for regular page switching + LaunchedEffect(initialPageId) { + log.v("EditorView: pageId changed to $initialPageId, loading data") + viewModel.loadToolbarState(bookId, initialPageId) } // Sync isQuickNavOpen to ViewModel @@ -117,18 +95,18 @@ fun EditorView( viewModel.onToolbarAction(ToolbarAction.UpdateQuickNavOpen(isQuickNavOpen)) } - if (pageExists == null) return BoxWithConstraints { val height = convertDpToPixel(this.maxHeight, context).toInt() val width = convertDpToPixel(this.maxWidth, context).toInt() + // Here we load initial page into the memory val page = remember { PageView( context = context, coroutineScope = scope, appRepository = appRepository, - currentPageId = pageId, + currentPageId = initialPageId, viewWidth = width, viewHeight = height, snackManager = snackManager, @@ -144,34 +122,38 @@ fun EditorView( EditorState(viewModel) } + val editorControlTower = remember { + EditorControlTower(scope, page, history, editorState) + } + + // Initialize ViewModel with persisted settings on first composition LaunchedEffect(Unit) { viewModel.initFromPersistedSettings(editorSettingCacheManager.getEditorSettings()) viewModel.updateDrawingState() } - val editorControlTower = remember { - EditorControlTower(scope, page, history, editorState).apply { registerObservers() } + DisposableEffect(editorControlTower) { + editorControlTower.registerObservers() + onDispose { + editorControlTower.unregisterObservers() + } } - // Collect UI Events from ViewModel (navigation and snackbars) + // Collect UI Events from ViewModel (navigation ) LaunchedEffect(Unit) { viewModel.uiEvents.collect { event -> when (event) { is EditorUiEvent.NavigateToLibrary -> { - navController.navigate(LibraryDestination.createRoute(event.folderId)) + goToLibrary(event.folderId) } is EditorUiEvent.NavigateToPages -> { - navController.navigate(PagesDestination.createRoute(event.bookId)) + goToPages(event.bookId) } EditorUiEvent.NavigateToBugReport -> { - navController.navigate(BugReportDestination.route) - } - - is EditorUiEvent.ShowSnackbar -> { - snackManager.displaySnack(SnackConf(text = event.message, duration = 2000)) + goToBugReport() } } } @@ -230,6 +212,11 @@ fun EditorView( .distinctUntilChanged() .drop(1) // Skip initial emission from loadBookData .collect { newPageId -> + log.v("EditorView: snapshotFlow detected pageId change to $newPageId, triggering onPageChange") + // update the PageView + page.changePage(newPageId) + + // update the navigation state 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..b78200d6 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,9 @@ 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 com.ethran.notable.ui.SnackState.Companion.logAndShowError import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.qualifiers.ApplicationContext import io.shipbook.shipbooksdk.Log @@ -141,7 +144,6 @@ sealed class CanvasCommand { // -------------------------------------------------------- sealed class EditorUiEvent { - data class ShowSnackbar(val message: String) : EditorUiEvent() data class NavigateToLibrary(val folderId: String?) : EditorUiEvent() data class NavigateToPages(val bookId: String) : EditorUiEvent() object NavigateToBugReport : EditorUiEvent() @@ -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() { @@ -321,7 +323,7 @@ class EditorViewModel @Inject constructor( val copiedFile = copyImageToDatabase(context, uri) sendCanvasCommand(CanvasCommand.CopyImageToCanvas(copiedFile.toUri())) } catch (e: Exception) { - sendUiEvent(EditorUiEvent.ShowSnackbar("Image import failed: ${e.message}")) + logAndShowError("EditorViewModel", "Image import failed: ${e.message}") } } } @@ -330,9 +332,10 @@ class EditorViewModel @Inject constructor( viewModelScope.launch(Dispatchers.IO) { try { val result = exportEngine.export(target, format) - sendUiEvent(EditorUiEvent.ShowSnackbar(result)) + val snack = SnackConf(text = result, duration = 4000) + SnackState.globalSnackFlow.emit(snack) } catch (e: Exception) { - sendUiEvent(EditorUiEvent.ShowSnackbar("Export failed: ${e.message}")) + logAndShowError("EditorViewModel", "Export failed: ${e.message}") } } } @@ -412,39 +415,60 @@ class EditorViewModel @Inject constructor( /** * Loads context data for the toolbar (page number, background info, etc.) */ - fun loadBookData(bookId: String?, pageId: String) { + suspend fun loadToolbarState(bookId: String?, pageId: String) { 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 pageIndex = book?.getPageIndex(pageId) ?: 0 - val totalPages = book?.pageIds?.size ?: 1 + val page = appRepository.pageRepository.getById(pageId) + if (page == null) { + logAndShowError( + reason = "EditorViewModel", + message = "Could not find page", + ) + fixNotebook(bookId, pageId) + return + } + val book = bookId?.let { appRepository.bookRepository.getById(it) } - 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 - } + val pageIndex = book?.getPageIndex(pageId) ?: 0 + val totalPages = book?.pageIds?.size ?: 1 - 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 + ) + } + } + + /** + * 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) } } @@ -482,6 +506,15 @@ class EditorViewModel @Inject constructor( } } + /** + * Updates the persistence layer and UI state to reflect a change in the currently opened page. + * + * This method saves the [newPageId] as the last opened page for the current notebook in the + * repository. If the page ID has changed, it updates the toolbar state; otherwise, it + * triggers a UI event to notify the user that the target page is already active. + * + * @param newPageId The unique identifier of the page to be set as open. + */ private suspend fun updateOpenedPage(newPageId: String) { log.v("updateOpenedPage: $newPageId") Log.d("EditorView", "Update open page to $newPageId") @@ -489,11 +522,14 @@ 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) +// 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!")) + val snack = SnackConf(text = "Tried to change to same page!", duration = 4000) + SnackState.globalSnackFlow.emit(snack) } } @@ -503,10 +539,17 @@ class EditorViewModel @Inject constructor( * @param id The unique identifier of the page to switch to. */ fun changePage(id: String) { - log.v("changePage: $id") log.d("Changing page to $id, from $currentPageId") viewModelScope.launch(Dispatchers.IO) { + // 1. Notify the PageView about the change + + // 2. Update the persistent layer + + // 3. Update the UI state updateOpenedPage(id) + + + // 4. Clean the selection state selectionState.reset() } } diff --git a/app/src/main/java/com/ethran/notable/editor/PageView.kt b/app/src/main/java/com/ethran/notable/editor/PageView.kt index dfa36343..2571429f 100644 --- a/app/src/main/java/com/ethran/notable/editor/PageView.kt +++ b/app/src/main/java/com/ethran/notable/editor/PageView.kt @@ -285,7 +285,7 @@ class PageView( val bookId = pageFromDb?.notebookId snackManager.showSnackDuring(text = "Loading strokes...") { val timeToLoad = measureTimeMillis { - logCache.d("Start page, id $currentPageId") + logCache.d("Start page loading, id $currentPageId") PageDataManager.requestPageLoadJoin(appRepository, currentPageId, bookId) logCache.d("Got page data (PageView.loadPage). id $currentPageId") } 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/navigation/NotableNavHost.kt b/app/src/main/java/com/ethran/notable/navigation/NotableNavHost.kt index be732c5d..80022700 100644 --- a/app/src/main/java/com/ethran/notable/navigation/NotableNavHost.kt +++ b/app/src/main/java/com/ethran/notable/navigation/NotableNavHost.kt @@ -27,7 +27,9 @@ import com.ethran.notable.ui.views.SystemInformationDestination import com.ethran.notable.ui.views.SystemInformationView import com.ethran.notable.ui.views.WelcomeDestination import com.ethran.notable.ui.views.WelcomeView +import io.shipbook.shipbooksdk.ShipBook +private val log = ShipBook.getLogger("NotableNavHost") @Composable fun NotableNavHost( @@ -102,11 +104,14 @@ fun NotableNavHost( exportEngine = exportEngine, editorSettingCacheManager = editorSettingCacheManager, appRepository = appRepository, - navController = appNavigator.navController, + goToLibrary = {appNavigator.goToLibrary(it)}, + goToPages = { bookId -> appNavigator.goToPages(bookId) }, + goToBugReport = { appNavigator.goToBugReport() }, bookId = bookId, - pageId = currentPageId, + initialPageId = currentPageId, isQuickNavOpen = appNavigator.isQuickNavOpen, onPageChange = { newPageId -> + log.d("onPageChange: $newPageId") appNavigator.onPageChange( backStackEntry, newPageId diff --git a/app/src/main/java/com/ethran/notable/navigation/NotableNavigator.kt b/app/src/main/java/com/ethran/notable/navigation/NotableNavigator.kt index 133de57b..93634838 100644 --- a/app/src/main/java/com/ethran/notable/navigation/NotableNavigator.kt +++ b/app/src/main/java/com/ethran/notable/navigation/NotableNavigator.kt @@ -16,7 +16,9 @@ import com.ethran.notable.data.datastore.GlobalAppSettings import com.ethran.notable.editor.EditorDestination import com.ethran.notable.editor.canvas.CanvasEventBus import com.ethran.notable.editor.utils.refreshScreen +import com.ethran.notable.ui.views.BugReportDestination import com.ethran.notable.ui.views.LibraryDestination +import com.ethran.notable.ui.views.PagesDestination import com.ethran.notable.ui.views.SystemInformationDestination import com.ethran.notable.ui.views.WelcomeDestination import com.ethran.notable.utils.hasFilePermission @@ -127,6 +129,14 @@ class NotableNavigator( navController.navigate(SystemInformationDestination.route) } + fun goToBugReport(){ + navController.navigate(BugReportDestination.route) + } + + fun goToPages(bookId: String) { + navController.navigate(PagesDestination.createRoute(bookId)) + } + fun goToPage(appRepository: AppRepository, pageId: String) { coroutineScope.launch { val bookId = runCatching { 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..653bfb39 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,10 +43,11 @@ 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() - + private var lastScrubEndTargetPageId: String? = null // Initialize data when the ViewModel is created or when a new page is opened fun loadPageData(currentPageId: String?) { @@ -131,6 +133,7 @@ class QuickNavViewModel( // --- Scrubber Actions --- fun onScrubStart() { + lastScrubEndTargetPageId = null CanvasEventBus.saveCurrent.tryEmit(Unit) } @@ -142,11 +145,17 @@ class QuickNavViewModel( } fun onScrubEnd(index: Int) { + log.e("onScrubEnd: $index") CanvasEventBus.restoreCanvas.tryEmit(Unit) + val pageIds = _uiState.value.bookPageIds - if (index in pageIds.indices) { - CanvasEventBus.changePage.tryEmit(pageIds[index]) - } + val targetPageId = pageIds.getOrNull(index) ?: return + + // Gesture end callbacks can fire more than once; ignore repeated commit for same target. + if (targetPageId == lastScrubEndTargetPageId) return + lastScrubEndTargetPageId = targetPageId + + CanvasEventBus.changePage.tryEmit(targetPageId) } fun onReturnClick(quickNavSourcePageId: String?) {