Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Swipe to delete history #12610

Closed
person808 opened this issue Jul 15, 2020 · 3 comments
Closed

Swipe to delete history #12610

person808 opened this issue Jul 15, 2020 · 3 comments
Assignees
Labels

Comments

@person808
Copy link
Contributor

person808 commented Jul 15, 2020

Broken out from #176

What is the user problem or growth opportunity you want to see solved?

Users should be able to swipe to delete history.

How do you know that this problem exists today? Why is this important?

This is not possible currently. Deletion is done by tapping the three dot menu and selecting delete.

Who will benefit from it?

Everyone who deletes items from history.

┆Issue is synchronized with this Jira Task

@github-actions github-actions bot added the needs:triage Issue needs triage label Jul 15, 2020
@lime124 lime124 mentioned this issue Jul 15, 2020
18 tasks
@person808
Copy link
Contributor Author

The problem with implementing this for the history page is that we are using version 2 of the paging library. We only have one view type because we cannot insert headers into the list of history items. This means we either 1) have to implement an ItemTouchHelper that only animates the history item view when swiped instead of the whole viewholder or 2) implement the gesture within the viewholder itself. Ideally we would upgrade to paging version 3 which supports inserting headers but it is still in alpha.

If either of the solutions above don't work we may have to wait for a stable version of paging v3.

@eliserichards eliserichards added sourpatch and removed needs:triage Issue needs triage labels Jul 16, 2020
@person808 person808 self-assigned this Jul 21, 2020
@person808
Copy link
Contributor Author

After multiple attempts, I think this may not be worth the effort to do right now (especially since my semester starts soon). The main issue I have run into is that if you undo deleting a history item, the item returns in a "swiped" state (the swipe background shows instead of the original item view). Enabling change animations fixes this, but because we currently rebind all visible viewholders, this introduces noticeable flickering.

A couple things I have tried:

  1. Copying the default ItemAnimator and removing the alpha animation that causes the flickering. This introduces some other weirdness with the animations.
  2. Being smarter about rebinding only the items that change. Fixes the swipe and undo issue, but the flickering is still present for those items and the code was rather messy.
  3. Using PagedListAdapter.submitList to update the list since it uses DiffUtil. Turns out this method only runs when you pass it a new list (I think, my memory is a little fuzzy here) and honestly the flickering might still appear.

Things I have not tried:

  1. Calling notifyItemChanged from the touch helpers onSwiped method, directly or indirectly. From implementing swipe to delete bookmarks it looks like this would trigger a "swipe back" animation which might fix the issue with swipe and undo. I'm unsure how this would interact with disabled change animations.
  2. Using payloads so we can be smarter about rebinding history items. This might be able to fix the flickering.

If I had the time I would try both of these.
Regarding my previous comment: I discovered ItemTouchCallback has a getDefaultUiUtil method which makes it easy to only animate the actual history item view.

@person808 person808 removed their assignment Jul 31, 2020
@data-sync-user data-sync-user changed the title Swipe to delete history FNX2-15353 ⁃ Swipe to delete history Aug 2, 2020
@data-sync-user data-sync-user changed the title FNX2-15353 ⁃ Swipe to delete history FNX3-21783 ⁃ Swipe to delete history Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX3-21783 ⁃ Swipe to delete history FNX-13705 ⁃ Swipe to delete history Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX-13705 ⁃ Swipe to delete history FNX2-15353 ⁃ Swipe to delete history Aug 11, 2020
@kbrosnan kbrosnan changed the title FNX2-15353 ⁃ Swipe to delete history Swipe to delete history Aug 29, 2020
@liuche liuche removed the sourpatch2 label Oct 6, 2020
@csadilek
Copy link
Contributor

Moved to bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1807054

Change performed by the Move to Bugzilla add-on.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants