Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quick comment navigation #749

Merged
merged 22 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
3e02d4b
Add quick top level comment navigation buttons to the bottom of the P…
yate Jun 19, 2023
a211397
Add setting for parent navigation buttons
yate Jun 19, 2023
313177d
remove unused parameter
yate Jun 19, 2023
b41364f
Merge branch 'main' into quick-comment-navigation
yate Jun 19, 2023
fe36828
update reference to look_and_feel_show_parent_comment_navigation_buttons
yate Jun 19, 2023
f002fe8
organize PostActivity imports
yate Jun 19, 2023
70b3cc6
fix kotlin linting issues
yate Jun 19, 2023
d6d362f
simplify logic to find previous/next parent index
yate Jun 19, 2023
c12079a
fix nearestNextIndex name
yate Jun 19, 2023
db434e0
add parent comment navigation with volume buttons
yate Jun 19, 2023
8e8cedc
Merge branch 'main' into quick-comment-navigation
yate Jun 19, 2023
009bfc8
add MIGRATION_15_16
yate Jun 19, 2023
3c225e8
fix default values for navigateParentCommentsWithVolumeButtons in som…
yate Jun 19, 2023
2e726e4
fix default values for navigateParentCommentsWithVolumeButtons again
yate Jun 19, 2023
cfa3c67
Merge branch 'main' into quick-comment-navigation
yate Jun 19, 2023
3588897
Merge branch 'main' into quick-comment-navigation
yate Jun 19, 2023
573a154
Merge branch 'main' into quick-comment-navigation
dessalines Jun 20, 2023
ec89c54
PR feedback
yate Jun 20, 2023
793b2f8
Merge branch 'main' into quick-comment-navigation
yate Jun 20, 2023
e5c4c35
formatting
yate Jun 20, 2023
457f0cd
remove log
yate Jun 20, 2023
c65bebe
if not a volume down/up event, bubble it up to the parent
yate Jun 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/src/main/java/com/jerboa/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,8 @@ class MainActivity : ComponentActivity() {
showCollapsedCommentContent = appSettings?.showCollapsedCommentContent ?: false,
showActionBarByDefault = appSettings?.showCommentActionBarByDefault ?: true,
showVotingArrowsInListView = appSettings?.showVotingArrowsInListView ?: true,
showParentCommentNavigationButtons = appSettings?.showParentCommentNavigationButtons ?: true,
navigateParentCommentsWithVolumeButtons = appSettings?.navigateParentCommentsWithVolumeButtons ?: false,
onClickSortType = { commentSortType ->
postViewModel.updateSortType(commentSortType)
postViewModel.getData(account)
Expand Down Expand Up @@ -494,6 +496,8 @@ class MainActivity : ComponentActivity() {
showCollapsedCommentContent = appSettings?.showCollapsedCommentContent ?: false,
showActionBarByDefault = appSettings?.showCommentActionBarByDefault ?: true,
showVotingArrowsInListView = appSettings?.showVotingArrowsInListView ?: true,
showParentCommentNavigationButtons = appSettings?.showParentCommentNavigationButtons ?: true,
navigateParentCommentsWithVolumeButtons = appSettings?.navigateParentCommentsWithVolumeButtons ?: false,
onClickSortType = { commentSortType ->
postViewModel.updateSortType(commentSortType)
postViewModel.getData(account)
Expand Down
25 changes: 24 additions & 1 deletion app/src/main/java/com/jerboa/db/AppDB.kt
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ data class AppSettings(
defaultValue = "1",
)
val showVotingArrowsInListView: Boolean,
@ColumnInfo(
name = "show_parent_comment_navigation_buttons",
defaultValue = "1",
)
val showParentCommentNavigationButtons: Boolean,
@ColumnInfo(
name = "navigate_parent_comments_with_volume_buttons",
defaultValue = "0",
)
val navigateParentCommentsWithVolumeButtons: Boolean,
@ColumnInfo(
name = "use_custom_tabs",
defaultValue = "1",
Expand Down Expand Up @@ -414,8 +424,20 @@ val MIGRATION_14_15 = object : Migration(14, 15) {
}
}

val MIGRATION_15_16 = object : Migration(15, 16) {
override fun migrate(database: SupportSQLiteDatabase) {
database.execSQL(UPDATE_APP_CHANGELOG_UNVIEWED)
database.execSQL(
"ALTER TABLE AppSettings add column show_parent_comment_navigation_buttons INTEGER NOT NULL default 1",
)
database.execSQL(
"ALTER TABLE AppSettings add column navigate_parent_comments_with_volume_buttons INTEGER NOT NULL default 0",
)
}
}

@Database(
version = 15,
version = 16,
entities = [Account::class, AppSettings::class],
exportSchema = true,
)
Expand Down Expand Up @@ -454,6 +476,7 @@ abstract class AppDB : RoomDatabase() {
MIGRATION_12_13,
MIGRATION_13_14,
MIGRATION_14_15,
MIGRATION_15_16,
)
// Necessary because it can't insert data on creation
.addCallback(object : Callback() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ fun CommentBodyPreview() {

fun LazyListScope.commentNodeItem(
node: CommentNodeData,
increaseLazyListIndexTracker: () -> Unit,
addToParentIndexes: () -> Unit,
isFlat: Boolean,
isExpanded: (commentId: Int) -> Boolean,
toggleExpanded: (commentId: Int) -> Unit,
Expand Down Expand Up @@ -217,8 +219,14 @@ fun LazyListScope.commentNodeItem(
XXL_PADDING
}

if (node.depth == 0) {
addToParentIndexes()
}

val showMoreChildren = isExpanded(commentId) && node.children.isNullOrEmpty() && node
.commentView.counts.child_count > 0 && !isFlat

increaseLazyListIndexTracker()
item(key = commentId) {
var viewSource by remember { mutableStateOf(false) }

Expand Down Expand Up @@ -350,6 +358,7 @@ fun LazyListScope.commentNodeItem(
}
}

increaseLazyListIndexTracker()
item(key = "${commentId}_show_more_children") {
AnimatedVisibility(
visible = showMoreChildren,
Expand All @@ -363,6 +372,8 @@ fun LazyListScope.commentNodeItem(
node.children?.also { nodes ->
commentNodeItems(
nodes = nodes,
increaseLazyListIndexTracker = increaseLazyListIndexTracker,
addToParentIndexes = addToParentIndexes,
isFlat = isFlat,
toggleExpanded = toggleExpanded,
toggleActionBar = toggleActionBar,
Expand Down Expand Up @@ -619,6 +630,8 @@ fun CommentNodesPreview() {
val tree = buildCommentsTree(comments, false)
CommentNodes(
nodes = tree,
increaseLazyListIndexTracker = {},
addToParentIndexes = {},
isFlat = false,
onUpvoteClick = {},
onDownvoteClick = {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import com.jerboa.db.Account
@Composable
fun CommentNodes(
nodes: List<CommentNodeData>,
increaseLazyListIndexTracker: () -> Unit,
addToParentIndexes: () -> Unit,
isFlat: Boolean,
listState: LazyListState,
onUpvoteClick: (commentView: CommentView) -> Unit,
Expand Down Expand Up @@ -48,6 +50,8 @@ fun CommentNodes(
LazyColumn(state = listState) {
commentNodeItems(
nodes = nodes,
increaseLazyListIndexTracker = increaseLazyListIndexTracker,
addToParentIndexes = addToParentIndexes,
isFlat = isFlat,
isExpanded = { commentId -> !unExpandedComments.contains(commentId) },
toggleExpanded = { commentId ->
Expand Down Expand Up @@ -94,6 +98,8 @@ fun CommentNodes(

fun LazyListScope.commentNodeItems(
nodes: List<CommentNodeData>,
increaseLazyListIndexTracker: () -> Unit,
addToParentIndexes: () -> Unit,
isFlat: Boolean,
isExpanded: (commentId: Int) -> Boolean,
toggleExpanded: (commentId: Int) -> Unit,
Expand Down Expand Up @@ -124,6 +130,8 @@ fun LazyListScope.commentNodeItems(
nodes.forEach { node ->
commentNodeItem(
node = node,
increaseLazyListIndexTracker = increaseLazyListIndexTracker,
addToParentIndexes = addToParentIndexes,
isFlat = isFlat,
isExpanded = isExpanded,
toggleExpanded = toggleExpanded,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,8 @@ fun UserTabs(
PullRefreshIndicator(loading, pullRefreshState, Modifier.align(Alignment.TopCenter))
CommentNodes(
nodes = nodes,
increaseLazyListIndexTracker = {},
addToParentIndexes = {},
isFlat = true,
listState = listState,
onMarkAsReadClick = {},
Expand Down
118 changes: 115 additions & 3 deletions app/src/main/java/com/jerboa/ui/components/post/PostActivity.kt
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
package com.jerboa.ui.components.post

import android.util.Log
import androidx.compose.foundation.focusable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.LazyListState
import androidx.compose.foundation.lazy.rememberLazyListState
import androidx.compose.material.ExperimentalMaterialApi
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.filled.KeyboardArrowDown
import androidx.compose.material.icons.filled.KeyboardArrowUp
import androidx.compose.material.icons.outlined.ArrowBack
import androidx.compose.material.icons.outlined.Sort
import androidx.compose.material.pullrefresh.PullRefreshIndicator
import androidx.compose.material.pullrefresh.pullRefresh
import androidx.compose.material.pullrefresh.rememberPullRefreshState
import androidx.compose.material3.BottomAppBar
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.Icon
import androidx.compose.material3.IconButton
Expand All @@ -23,16 +30,27 @@ import androidx.compose.material3.TopAppBar
import androidx.compose.material3.TopAppBarDefaults
import androidx.compose.material3.rememberTopAppBarState
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateListOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import androidx.compose.runtime.snapshots.SnapshotStateList
import androidx.compose.ui.Alignment
import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.scale
import androidx.compose.ui.focus.FocusRequester
import androidx.compose.ui.focus.focusRequester
import androidx.compose.ui.input.key.Key
import androidx.compose.ui.input.key.key
import androidx.compose.ui.input.key.onKeyEvent
import androidx.compose.ui.input.nestedscroll.nestedScroll
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.dp
import androidx.navigation.NavController
import com.jerboa.PostViewMode
import com.jerboa.R
Expand Down Expand Up @@ -66,6 +84,8 @@ import com.jerboa.ui.components.common.getCurrentAccount
import com.jerboa.ui.components.common.simpleVerticalScrollbar
import com.jerboa.ui.components.home.SiteViewModel
import com.jerboa.ui.components.post.edit.PostEditViewModel
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch

@Composable
fun CommentsHeaderTitle(
Expand All @@ -85,7 +105,7 @@ fun CommentsHeaderTitle(
}
}

@OptIn(ExperimentalMaterialApi::class, ExperimentalMaterial3Api::class)
@OptIn(ExperimentalMaterialApi::class, ExperimentalMaterial3Api::class, ExperimentalComposeUiApi::class)
@Composable
fun PostActivity(
postViewModel: PostViewModel,
Expand All @@ -98,6 +118,8 @@ fun PostActivity(
showCollapsedCommentContent: Boolean,
showActionBarByDefault: Boolean,
showVotingArrowsInListView: Boolean,
showParentCommentNavigationButtons: Boolean,
navigateParentCommentsWithVolumeButtons: Boolean,
onClickSortType: (CommentSortType) -> Unit,
selectedSortType: CommentSortType,
) {
Expand All @@ -113,9 +135,13 @@ fun PostActivity(
val unExpandedComments = remember { mutableStateListOf<Int>() }
val commentsWithToggledActionBar = remember { mutableStateListOf<Int>() }
var showSortOptions by remember { mutableStateOf(false) }
val focusRequester = remember { FocusRequester() }

val listState = rememberLazyListState()
var lazyListIndexTracker: Int
dessalines marked this conversation as resolved.
Show resolved Hide resolved
val parentListStateIndexes = remember { mutableStateListOf<Int>() }
val scrollBehavior = TopAppBarDefaults.enterAlwaysScrollBehavior(rememberTopAppBarState())
val scope = rememberCoroutineScope()

val pullRefreshState = rememberPullRefreshState(
refreshing = postLoading,
Expand All @@ -135,8 +161,55 @@ fun PostActivity(
)
}

LaunchedEffect(Unit) {
focusRequester.requestFocus()
}

Scaffold(
modifier = Modifier.nestedScroll(scrollBehavior.nestedScrollConnection),
modifier = Modifier
.nestedScroll(scrollBehavior.nestedScrollConnection)
.focusRequester(focusRequester)
.focusable()
.onKeyEvent { keyEvent ->
if (navigateParentCommentsWithVolumeButtons) {
if (keyEvent.key == Key.VolumeUp) {
scrollToPreviousParentComment(scope, parentListStateIndexes, listState)
} else if (keyEvent.key == Key.VolumeDown) {
scrollToNextParentComment(scope, parentListStateIndexes, listState)
}
true
} else {
false
}
},
bottomBar = {
if (showParentCommentNavigationButtons) {
BottomAppBar(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Externalize this somewhere, probably in ui/common/appbars

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I move scrollToNextParentComment and scrollToPreviousParentComment to Utils as well?
https://github.com/dessalines/jerboa/pull/749/files/573a154d6cb52b624803dc5530a91501d168576a#diff-9958f04ae114c8444c9e781405889c7fab0b5465ca616011fcfb047a243c356bR579

They will be referenced in both the external BottomAppBar and the volume button key event

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, up to you.

containerColor = MaterialTheme.colorScheme.background.copy(alpha = .75f),
modifier = Modifier.height(50.dp),
content = {
IconButton(modifier = Modifier.weight(.5f), onClick = {
scrollToPreviousParentComment(scope, parentListStateIndexes, listState)
}) {
Icon(
modifier = Modifier.scale(1.5f),
imageVector = Icons.Filled.KeyboardArrowUp,
contentDescription = "Up",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use i18n strings, lots of examples in this file.

)
}
IconButton(modifier = Modifier.weight(.5f), onClick = {
scrollToNextParentComment(scope, parentListStateIndexes, listState)
}) {
Icon(
modifier = Modifier.scale(1.5f),
imageVector = Icons.Filled.KeyboardArrowDown,
contentDescription = "Down",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

)
}
},
)
}
},
topBar = {
Column {
TopAppBar(
Expand Down Expand Up @@ -169,6 +242,8 @@ fun PostActivity(
},
content = { padding ->
Box(modifier = Modifier.pullRefresh(pullRefreshState)) {
parentListStateIndexes.clear()
lazyListIndexTracker = 2
PullRefreshIndicator(
postLoading,
pullRefreshState,
Expand All @@ -184,7 +259,7 @@ fun PostActivity(
LazyColumn(
state = listState,
modifier = Modifier
.padding(padding)
.padding(top = padding.calculateTopPadding())
.simpleVerticalScrollbar(listState),
) {
item(key = "${postView.post.id}_listing") {
Expand Down Expand Up @@ -342,6 +417,12 @@ fun PostActivity(

commentNodeItems(
nodes = commentTree,
increaseLazyListIndexTracker = {
lazyListIndexTracker++
},
addToParentIndexes = {
parentListStateIndexes.add(lazyListIndexTracker)
},
isFlat = false,
isExpanded = { commentId ->
!unExpandedComments.contains(
Expand Down Expand Up @@ -480,6 +561,11 @@ fun PostActivity(

else -> {}
}
if (showParentCommentNavigationButtons) {
item {
Spacer(modifier = Modifier.height(padding.calculateBottomPadding()))
}
}
}
}

Expand All @@ -489,3 +575,29 @@ fun PostActivity(
},
)
}

private fun scrollToNextParentComment(
scope: CoroutineScope,
parentListStateIndexes: SnapshotStateList<Int>,
listState: LazyListState,
) {
scope.launch {
parentListStateIndexes.firstOrNull { parentIndex -> parentIndex > listState.firstVisibleItemIndex }
?.let { nearestNextIndex ->
listState.animateScrollToItem(nearestNextIndex)
}
}
}

private fun scrollToPreviousParentComment(
scope: CoroutineScope,
parentListStateIndexes: SnapshotStateList<Int>,
listState: LazyListState,
) {
scope.launch {
parentListStateIndexes.lastOrNull { parentIndex -> parentIndex < listState.firstVisibleItemIndex }
?.let { nearestPreviousIndex ->
listState.animateScrollToItem(nearestPreviousIndex)
}
}
}
Loading