Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions changelog/unreleased/4570
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Feature: Maintain scroll position when user scrolls and navigates in file/folder list
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feature is not a valid prefix for Calens. Check the Calens README.md to see the valid prefixes, I'd say in this case the best one is Enhancement

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is also a limit of characters length for the title, so I would just write the way you named the PR: Maintain scroll position in file list


Saves scroll position when user scrolls and navigates in file/folder list

https://github.com/owncloud/android/issues/4528
https://github.com/owncloud/android/pull/4570
Original file line number Diff line number Diff line change
Expand Up @@ -798,10 +798,15 @@ class MainFileListFragment : Fragment(),
collectLatestLifecycleFlow(mainFileListViewModel.fileListUiState) { fileListUiState ->
if (fileListUiState !is MainFileListViewModel.FileListUiState.Success) return@collectLatestLifecycleFlow

saveScrollPosition()

fileListAdapter.updateFileList(
filesToAdd = fileListUiState.folderContent,
fileListOption = fileListUiState.fileListOption,
)

restoreScrollPosition()

Comment on lines +801 to +809
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm I don't know if I like this to be here... there are 2 main points that make me doubt here:

  1. The observer where this is located. This is going to be executed everytime fileListUiState is updated, and that is a combine with 5 different variables: the current folder displayed, the file list option, the search filter, the sort type and order and the current space. Everytime one of these is updated, the scroll position is going to be saved (and the previous one restored), so it looks a bit inefficient (and a potential source of glitches)
  2. Seems we're saving just 1 scroll position, but what if we go deep into a several-level folder hierarchy? Would we be able to restore every scroll position of each level?

showOrHideEmptyView(fileListUiState)

binding.spaceHeader.root.apply {
Expand Down Expand Up @@ -994,6 +999,38 @@ class MainFileListFragment : Fragment(),
_binding = null
}

/**
* Saves the current scroll position of the RecyclerView by capturing the indices of
* the last visible items from the StaggeredGridLayoutManager.
*
* This position is stored in the ViewModel so it can be restored later,
* typically after the adapter data is updated. This ensures continuity
* in the user’s scroll experience when navigating between fragments or refreshing data.
*/
private fun saveScrollPosition() {
val layoutManager = binding.recyclerViewMainFileList.layoutManager as? StaggeredGridLayoutManager
mainFileListViewModel.lastVisibleItemPositions = layoutManager?.findLastVisibleItemPositions(null)
}

/**
* Restores the scroll position of the RecyclerView based on the previously
* saved positions in the ViewModel. It scrolls to the maximum of the last
* visible item positions captured earlier using [saveScrollPosition].
*
* This method is posted to the queue to ensure it runs after the
* RecyclerView has completed its layout pass and the adapter update has been applied.
*/
private fun restoreScrollPosition() {
binding.recyclerViewMainFileList.post {
mainFileListViewModel.lastVisibleItemPositions?.let { positions ->
val maxPosition = positions.maxOrNull() ?: 0
binding.recyclerViewMainFileList.scrollToPosition(maxPosition)
mainFileListViewModel.lastVisibleItemPositions = null
}
}
}


fun updateFileListOption(newFileListOption: FileListOption, file: OCFile) {
mainFileListViewModel.updateFileListOption(newFileListOption)
binding.swipeRefreshMainFileList.isEnabled = newFileListOption != FileListOption.AV_OFFLINE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ class MainFileListViewModel(
fileListOptionParam: FileListOption,
) : ViewModel() {

var lastVisibleItemPositions: IntArray? = null
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's usually less secure to expose variables out of the ViewModel as in this case... but let's see how things go after resolving the previous comments 😉

private val showHiddenFiles: Boolean = sharedPreferencesProvider.getBoolean(PREF_SHOW_HIDDEN_FILES, false)

val currentFolderDisplayed: MutableStateFlow<OCFile> = MutableStateFlow(initialFolderToDisplay)
Expand Down
Loading