From 2f26982e346d65de82d0809d7262a17f737d9eb2 Mon Sep 17 00:00:00 2001 From: Andreas Date: Wed, 31 Aug 2022 22:47:07 +0200 Subject: [PATCH] Resolve review comments for BrowseSourceScreens (#7912) --- .../presentation/browse/BrowseLatestScreen.kt | 12 +--- .../presentation/browse/BrowseSourceScreen.kt | 10 +-- .../presentation/browse/SourceSearchScreen.kt | 10 +++ .../browse/components/BrowseLatestToolbar.kt | 6 +- .../browse/components/BrowseSourceToolbar.kt | 65 +++++-------------- .../kanade/presentation/components/AppBar.kt | 6 ++ .../source/browse/BrowseSourceController.kt | 9 +++ .../source/latest/LatestUpdatesController.kt | 9 +++ 8 files changed, 56 insertions(+), 71 deletions(-) diff --git a/app/src/main/java/eu/kanade/presentation/browse/BrowseLatestScreen.kt b/app/src/main/java/eu/kanade/presentation/browse/BrowseLatestScreen.kt index 0eba56cf6d..30a259a695 100644 --- a/app/src/main/java/eu/kanade/presentation/browse/BrowseLatestScreen.kt +++ b/app/src/main/java/eu/kanade/presentation/browse/BrowseLatestScreen.kt @@ -4,17 +4,14 @@ import androidx.compose.material3.SnackbarHostState import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.remember -import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalUriHandler import androidx.paging.compose.collectAsLazyPagingItems import eu.kanade.domain.manga.model.Manga import eu.kanade.presentation.browse.components.BrowseLatestToolbar import eu.kanade.presentation.components.Scaffold import eu.kanade.tachiyomi.source.LocalSource -import eu.kanade.tachiyomi.source.online.HttpSource import eu.kanade.tachiyomi.ui.browse.source.browse.BrowseSourcePresenter import eu.kanade.tachiyomi.ui.more.MoreController -import eu.kanade.tachiyomi.ui.webview.WebViewActivity @Composable fun BrowseLatestScreen( @@ -22,21 +19,16 @@ fun BrowseLatestScreen( navigateUp: () -> Unit, onMangaClick: (Manga) -> Unit, onMangaLongClick: (Manga) -> Unit, + onWebViewClick: () -> Unit, ) { val columns by presenter.getColumnsPreferenceForCurrentOrientation() - val context = LocalContext.current + val uriHandler = LocalUriHandler.current val onHelpClick = { uriHandler.openUri(LocalSource.HELP_URL) } - val onWebViewClick = f@{ - val source = presenter.source as? HttpSource ?: return@f - val intent = WebViewActivity.newIntent(context, source.baseUrl, source.id, source.name) - context.startActivity(intent) - } - Scaffold( topBar = { scrollBehavior -> BrowseLatestToolbar( diff --git a/app/src/main/java/eu/kanade/presentation/browse/BrowseSourceScreen.kt b/app/src/main/java/eu/kanade/presentation/browse/BrowseSourceScreen.kt index 3eca115040..e287c6fcad 100644 --- a/app/src/main/java/eu/kanade/presentation/browse/BrowseSourceScreen.kt +++ b/app/src/main/java/eu/kanade/presentation/browse/BrowseSourceScreen.kt @@ -34,12 +34,10 @@ import eu.kanade.presentation.components.Scaffold import eu.kanade.tachiyomi.R import eu.kanade.tachiyomi.source.CatalogueSource import eu.kanade.tachiyomi.source.LocalSource -import eu.kanade.tachiyomi.source.online.HttpSource import eu.kanade.tachiyomi.ui.browse.source.browse.BrowseSourcePresenter import eu.kanade.tachiyomi.ui.browse.source.browse.NoResultsException import eu.kanade.tachiyomi.ui.library.setting.LibraryDisplayMode import eu.kanade.tachiyomi.ui.more.MoreController -import eu.kanade.tachiyomi.ui.webview.WebViewActivity import eu.kanade.tachiyomi.widget.EmptyView @Composable @@ -50,6 +48,7 @@ fun BrowseSourceScreen( onFabClick: () -> Unit, onMangaClick: (Manga) -> Unit, onMangaLongClick: (Manga) -> Unit, + onWebViewClick: () -> Unit, ) { val columns by presenter.getColumnsPreferenceForCurrentOrientation() @@ -57,19 +56,12 @@ fun BrowseSourceScreen( val snackbarHostState = remember { SnackbarHostState() } - val context = LocalContext.current val uriHandler = LocalUriHandler.current val onHelpClick = { uriHandler.openUri(LocalSource.HELP_URL) } - val onWebViewClick = f@{ - val source = presenter.source as? HttpSource ?: return@f - val intent = WebViewActivity.newIntent(context, source.baseUrl, source.id, source.name) - context.startActivity(intent) - } - Scaffold( topBar = { scrollBehavior -> BrowseSourceToolbar( diff --git a/app/src/main/java/eu/kanade/presentation/browse/SourceSearchScreen.kt b/app/src/main/java/eu/kanade/presentation/browse/SourceSearchScreen.kt index fa3affb8c3..cd14f6923e 100644 --- a/app/src/main/java/eu/kanade/presentation/browse/SourceSearchScreen.kt +++ b/app/src/main/java/eu/kanade/presentation/browse/SourceSearchScreen.kt @@ -1,8 +1,11 @@ package eu.kanade.presentation.browse import androidx.compose.runtime.Composable +import androidx.glance.LocalContext import eu.kanade.domain.manga.model.Manga +import eu.kanade.tachiyomi.source.online.HttpSource import eu.kanade.tachiyomi.ui.browse.source.browse.BrowseSourcePresenter +import eu.kanade.tachiyomi.ui.webview.WebViewActivity @Composable fun SourceSearchScreen( @@ -11,6 +14,8 @@ fun SourceSearchScreen( onFabClick: () -> Unit, onClickManga: (Manga) -> Unit, ) { + val context = LocalContext.current + BrowseSourceScreen( presenter = presenter, navigateUp = navigateUp, @@ -18,5 +23,10 @@ fun SourceSearchScreen( onFabClick = onFabClick, onMangaClick = onClickManga, onMangaLongClick = onClickManga, + onWebViewClick = f@{ + val source = presenter.source as? HttpSource ?: return@f + val intent = WebViewActivity.newIntent(context, source.baseUrl, source.id, source.name) + context.startActivity(intent) + }, ) } diff --git a/app/src/main/java/eu/kanade/presentation/browse/components/BrowseLatestToolbar.kt b/app/src/main/java/eu/kanade/presentation/browse/components/BrowseLatestToolbar.kt index 46b7bd4c26..f1100470e1 100644 --- a/app/src/main/java/eu/kanade/presentation/browse/components/BrowseLatestToolbar.kt +++ b/app/src/main/java/eu/kanade/presentation/browse/components/BrowseLatestToolbar.kt @@ -42,19 +42,19 @@ fun BrowseLatestToolbar( AppBarActions( actions = listOf( AppBar.Action( - title = "display_mode", + title = stringResource(id = R.string.action_display_mode), icon = Icons.Filled.ViewModule, onClick = { selectingDisplayMode = true }, ), if (source is LocalSource) { AppBar.Action( - title = "help", + title = stringResource(id = R.string.label_help), icon = Icons.Outlined.Help, onClick = onHelpClick, ) } else { AppBar.Action( - title = "webview", + title = stringResource(id = R.string.action_web_view), icon = Icons.Outlined.Public, onClick = onWebViewClick, ) diff --git a/app/src/main/java/eu/kanade/presentation/browse/components/BrowseSourceToolbar.kt b/app/src/main/java/eu/kanade/presentation/browse/components/BrowseSourceToolbar.kt index 2a95bf9bd8..6cf7b9366f 100644 --- a/app/src/main/java/eu/kanade/presentation/browse/components/BrowseSourceToolbar.kt +++ b/app/src/main/java/eu/kanade/presentation/browse/components/BrowseSourceToolbar.kt @@ -1,42 +1,33 @@ package eu.kanade.presentation.browse.components -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.text.BasicTextField import androidx.compose.foundation.text.KeyboardActions import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.ViewModule import androidx.compose.material.icons.outlined.Check -import androidx.compose.material.icons.outlined.Clear import androidx.compose.material.icons.outlined.Help import androidx.compose.material.icons.outlined.Public import androidx.compose.material.icons.outlined.Search import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.Icon -import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.material3.TopAppBarScrollBehavior import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue -import androidx.compose.ui.Modifier -import androidx.compose.ui.focus.FocusRequester -import androidx.compose.ui.focus.focusRequester -import androidx.compose.ui.graphics.SolidColor import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.input.ImeAction import eu.kanade.presentation.browse.BrowseSourceState import eu.kanade.presentation.components.AppBar import eu.kanade.presentation.components.AppBarActions import eu.kanade.presentation.components.DropdownMenu +import eu.kanade.presentation.components.SearchToolbar import eu.kanade.tachiyomi.R import eu.kanade.tachiyomi.source.CatalogueSource import eu.kanade.tachiyomi.source.LocalSource import eu.kanade.tachiyomi.ui.library.setting.LibraryDisplayMode -import kotlinx.coroutines.delay @Composable fun BrowseSourceToolbar( @@ -95,24 +86,24 @@ fun BrowseSourceRegularToolbar( AppBarActions( actions = listOf( AppBar.Action( - title = "search", + title = stringResource(id = R.string.action_search), icon = Icons.Outlined.Search, onClick = onSearchClick, ), AppBar.Action( - title = "display_mode", + title = stringResource(id = R.string.action_display_mode), icon = Icons.Filled.ViewModule, onClick = { selectingDisplayMode = true }, ), if (source is LocalSource) { AppBar.Action( - title = "help", + title = stringResource(id = R.string.label_help), icon = Icons.Outlined.Help, onClick = onHelpClick, ) } else { AppBar.Action( - title = "webview", + title = stringResource(id = R.string.action_web_view), icon = Icons.Outlined.Public, onClick = onWebViewClick, ) @@ -174,41 +165,17 @@ fun BrowseSourceSearchToolbar( onSearchClick: () -> Unit, scrollBehavior: TopAppBarScrollBehavior, ) { - val focusRequester = remember { FocusRequester() } - AppBar( - navigateUp = navigateUp, - titleContent = { - BasicTextField( - value = searchQuery, - onValueChange = onSearchQueryChanged, - modifier = Modifier - .fillMaxWidth() - .focusRequester(focusRequester), - keyboardOptions = KeyboardOptions(imeAction = ImeAction.Search), - keyboardActions = KeyboardActions( - onSearch = { - onSearchClick() - }, - ), - cursorBrush = SolidColor(MaterialTheme.colorScheme.onSurface), - ) - }, - actions = { - AppBarActions( - actions = listOf( - AppBar.Action( - title = "clear", - icon = Icons.Outlined.Clear, - onClick = onResetClick, - ), - ), - ) - }, + SearchToolbar( + searchQuery = searchQuery, + onChangeSearchQuery = onSearchQueryChanged, + keyboardOptions = KeyboardOptions(imeAction = ImeAction.Search), + keyboardActions = KeyboardActions( + onSearch = { + onSearchClick() + }, + ), + onClickCloseSearch = navigateUp, + onClickResetSearch = onResetClick, scrollBehavior = scrollBehavior, ) - LaunchedEffect(Unit) { - // TODO: https://issuetracker.google.com/issues/204502668 - delay(100) - focusRequester.requestFocus() - } } diff --git a/app/src/main/java/eu/kanade/presentation/components/AppBar.kt b/app/src/main/java/eu/kanade/presentation/components/AppBar.kt index 4bb4e62db5..e3a28118da 100644 --- a/app/src/main/java/eu/kanade/presentation/components/AppBar.kt +++ b/app/src/main/java/eu/kanade/presentation/components/AppBar.kt @@ -7,6 +7,8 @@ import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.statusBars import androidx.compose.foundation.text.BasicTextField +import androidx.compose.foundation.text.KeyboardActions +import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.ArrowBack import androidx.compose.material.icons.filled.Close @@ -220,6 +222,8 @@ fun AppBarActions( fun SearchToolbar( searchQuery: String, onChangeSearchQuery: (String) -> Unit, + keyboardOptions: KeyboardOptions = KeyboardOptions.Default, + keyboardActions: KeyboardActions = KeyboardActions.Default, onClickCloseSearch: () -> Unit, onClickResetSearch: () -> Unit, incognitoMode: Boolean = false, @@ -236,6 +240,8 @@ fun SearchToolbar( .fillMaxWidth() .focusRequester(focusRequester), textStyle = MaterialTheme.typography.bodyMedium.copy(color = MaterialTheme.colorScheme.onBackground), + keyboardOptions = keyboardOptions, + keyboardActions = keyboardActions, singleLine = true, cursorBrush = SolidColor(MaterialTheme.colorScheme.onBackground), ) diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/browse/source/browse/BrowseSourceController.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/browse/source/browse/BrowseSourceController.kt index eca86ad13d..3ce32e71ff 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/browse/source/browse/BrowseSourceController.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/browse/source/browse/BrowseSourceController.kt @@ -4,6 +4,7 @@ import android.os.Bundle import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.ui.platform.LocalContext import androidx.core.os.bundleOf import eu.kanade.domain.source.model.Source import eu.kanade.presentation.browse.BrowseSourceScreen @@ -12,11 +13,13 @@ import eu.kanade.presentation.components.ChangeCategoryDialog import eu.kanade.presentation.components.DuplicateMangaDialog import eu.kanade.tachiyomi.source.CatalogueSource import eu.kanade.tachiyomi.source.model.Filter +import eu.kanade.tachiyomi.source.online.HttpSource import eu.kanade.tachiyomi.ui.base.controller.FullComposeController import eu.kanade.tachiyomi.ui.base.controller.pushController import eu.kanade.tachiyomi.ui.browse.source.browse.BrowseSourcePresenter.Dialog import eu.kanade.tachiyomi.ui.category.CategoryController import eu.kanade.tachiyomi.ui.manga.MangaController +import eu.kanade.tachiyomi.ui.webview.WebViewActivity import eu.kanade.tachiyomi.util.lang.launchIO open class BrowseSourceController(bundle: Bundle) : @@ -41,6 +44,7 @@ open class BrowseSourceController(bundle: Bundle) : @Composable override fun ComposeContent() { val scope = rememberCoroutineScope() + val context = LocalContext.current BrowseSourceScreen( presenter = presenter, @@ -58,6 +62,11 @@ open class BrowseSourceController(bundle: Bundle) : } } }, + onWebViewClick = f@{ + val source = presenter.source as? HttpSource ?: return@f + val intent = WebViewActivity.newIntent(context, source.baseUrl, source.id, source.name) + context.startActivity(intent) + }, ) val onDismissRequest = { presenter.dialog = null } diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/browse/source/latest/LatestUpdatesController.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/browse/source/latest/LatestUpdatesController.kt index 7f062b6135..ac89ac8ec6 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/browse/source/latest/LatestUpdatesController.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/browse/source/latest/LatestUpdatesController.kt @@ -3,17 +3,20 @@ package eu.kanade.tachiyomi.ui.browse.source.latest import android.os.Bundle import androidx.compose.runtime.Composable import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.ui.platform.LocalContext import androidx.core.os.bundleOf import eu.kanade.domain.source.model.Source import eu.kanade.presentation.browse.BrowseLatestScreen import eu.kanade.presentation.browse.components.RemoveMangaDialog import eu.kanade.presentation.components.ChangeCategoryDialog import eu.kanade.presentation.components.DuplicateMangaDialog +import eu.kanade.tachiyomi.source.online.HttpSource import eu.kanade.tachiyomi.ui.base.controller.pushController import eu.kanade.tachiyomi.ui.browse.source.browse.BrowseSourceController import eu.kanade.tachiyomi.ui.browse.source.browse.BrowseSourcePresenter import eu.kanade.tachiyomi.ui.category.CategoryController import eu.kanade.tachiyomi.ui.manga.MangaController +import eu.kanade.tachiyomi.ui.webview.WebViewActivity import eu.kanade.tachiyomi.util.lang.launchIO /** @@ -32,6 +35,7 @@ class LatestUpdatesController(bundle: Bundle) : BrowseSourceController(bundle) { @Composable override fun ComposeContent() { val scope = rememberCoroutineScope() + val context = LocalContext.current BrowseLatestScreen( presenter = presenter, @@ -47,6 +51,11 @@ class LatestUpdatesController(bundle: Bundle) : BrowseSourceController(bundle) { } } }, + onWebViewClick = f@{ + val source = presenter.source as? HttpSource ?: return@f + val intent = WebViewActivity.newIntent(context, source.baseUrl, source.id, source.name) + context.startActivity(intent) + }, ) val onDismissRequest = { presenter.dialog = null }