Skip to content

Fix deleting from slideshow #15669

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix deleting from slideshow #15669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3b2a40c
fix: deletion from slideshow works again
PhilLab Sep 14, 2025
3143bb7
fix: deletion from slideshow for cases were user is not present
PhilLab Sep 21, 2025
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: 0 additions & 4 deletions app/src/main/java/com/owncloud/android/ui/dialog/RemoveFilesDialogFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import com.owncloud.android.datamodel.OCFile
import com.owncloud.android.ui.activity.FileActivity
import com.owncloud.android.ui.activity.FileDisplayActivity
import com.owncloud.android.ui.dialog.ConfirmationDialogFragment.ConfirmationDialogFragmentListener
import com.owncloud.android.ui.preview.PreviewImageActivity
import javax.inject.Inject

/**
Expand Down Expand Up @@ -125,14 +124,11 @@ class RemoveFilesDialogFragment :
}

finishActionMode()
finishPreviewImageActivity()
}
}

override fun onNeutral(callerTag: String?) = Unit

private fun finishPreviewImageActivity() = getTypedActivity(PreviewImageActivity::class.java)?.finish()

private fun setActionMode(actionMode: ActionMode?) {
this.actionMode = actionMode
}
Expand Down
17 changes: 12 additions & 5 deletions app/src/main/java/com/owncloud/android/ui/preview/PreviewImageActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,6 @@ class PreviewImageActivity :
super.onRemoteOperationFinish(operation, result)

if (operation is RemoveFileOperation) {
val deletePosition = viewPager?.currentItem ?: return
val nextPosition = if (deletePosition > 0) deletePosition - 1 else 0

previewImagePagerAdapter?.let {
if (it.itemCount <= 1) {
Expand All @@ -289,11 +287,20 @@ class PreviewImageActivity :
}

if (user.isPresent) {
// Re-init the view pager, which will advance to the next image
initViewPager(user.get())
} else if (result.isSuccess) {
// If deletion was successful, update adapter and display next image
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove comments and extract else if (result.isSuccess) { logic to the separate function, e.g. updateViewPagerAfterDeletion() thus it will be also clear without comments.

val deletePosition = viewPager?.currentItem ?: return
previewImagePagerAdapter?.let { adapter ->
// advance to the next image if possible, since initViewPager() also advances forwards
val nextPosition = if (deletePosition < (adapter.itemCount - 1)) deletePosition + 1 else max(deletePosition - 1, 0)
viewPager?.setCurrentItem(nextPosition, true)
adapter.delete(deletePosition)
// Page needs to be reselected after the adapter has been updated. Otherwise, wrong title is shown
selectPage(nextPosition)
}
}

viewPager?.setCurrentItem(nextPosition, true)
previewImagePagerAdapter?.delete(deletePosition)
} else if (operation is SynchronizeFileOperation) {
onSynchronizeFileOperationFinish(result)
}
Expand Down
6 changes: 6 additions & 0 deletions app/src/main/java/com/owncloud/android/ui/preview/PreviewImagePagerAdapter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,10 @@ class PreviewImagePagerAdapter : FragmentStateAdapter {
override fun createFragment(position: Int): Fragment = getItem(position)

override fun getItemCount(): Int = imageFiles.size

override fun getItemId(position: Int): Long {
// The item ID function is needed to detect whether the deletion of the current item needs a UI update
// Use the OCFile id, fallback to position if not available
return imageFiles.getOrNull(position)?.fileId?.toLong() ?: position.toLong()
Copy link
Collaborator

Choose a reason for hiding this comment

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

getItemId() is part of androidx.viewpager2.adapter.FragmentStateAdapter. No need to add an explanation here since the method is already documented in the official API.

Additionally, please remove unnecessary toLong() cast for fileId since fileId is already Long.

}
}