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

Investigate if we want to keep swipe to delete bookmarks #15362

Closed
kbrosnan opened this issue Sep 23, 2020 · 9 comments · Fixed by #16646
Closed

Investigate if we want to keep swipe to delete bookmarks #15362

kbrosnan opened this issue Sep 23, 2020 · 9 comments · Fixed by #16646
Labels
eng:ux-debt Feature polish and UX engineering debt Feature:Bookmarks Feature:Gesture good first issue Good for newcomers

Comments

@kbrosnan
Copy link
Contributor

kbrosnan commented Sep 23, 2020

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

Swipe to delete bookmarks is nightly only at this time. We should investigate what we want to do here. Should we build some UI around having a way to restore deleted bookmarks or should we remove the feature?

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

Need to make a decision about a feature that is in limbo

@mozilla-mobile/ux to help inform a decision here.

┆Issue is synchronized with this Jira Task

@grigoryk
Copy link
Contributor

We do have an Undo functionality right now (via the snackbar action), but it's pretty time-limited and disappears after several seconds.

Screenshot_1600904699

@kbrosnan
Copy link
Contributor Author

Yes that was mentioned in #15007. We decided that Fenix needs a more robust undo option than the snackbar. Bookmarks are a different classification of data than tabs where undoing is less of a problem and we built a recently closed tabs UI in #2486.

@kbrosnan kbrosnan removed the needs:triage Issue needs triage label Sep 24, 2020
@apbitner apbitner added the needs:UX-feedback Needs UX Feedback label Sep 24, 2020
@apbitner
Copy link

@kbrosnan On Desktop, is the only way to undo deletion of bookmark through the 'Recover' option in the bookmark manager?

mcarare added a commit to mcarare/fenix that referenced this issue Oct 2, 2020
…llection button.

Check now considers selected tab tray mode.
@kbrosnan
Copy link
Contributor Author

kbrosnan commented Oct 2, 2020

The accessibility of this feature is rather different than the desktop model. Where the user makes an explicit decision to delete a bookmark by right clicking and selecting delete or using the delete key. Looking and scrolling down the list of my bookmarks has resulted in accidental deletions for me. It does not take much horizontal movement to trigger this deletion. I was aware of the feature and my first thought was cannot miss that snackbar else I will have no idea what was deleted. For a user that does not know about it they could easily miss the snackbar information.

  • Keep the feature and increase the spring rate and swipe length a lot. Make it into an explicit decision.
  • Keep the feature and create an undo or restore from backup method
  • Remove the feature

@Matth7878
Copy link

Matth7878 commented Oct 4, 2020

Swiping to delete a bookmark is really not a good idea... In no way it should be easy to delete a bookmark. You risk to do it without wanting to or even noticing it... (For instance if you screen was on and you weren't looking at it and inadvertently swiped on it)

If really it should exist then It would be better if you had to enter in a delete mode (or edit mode) where you can swipe bookmarks you want to delete. But it would be better in delete mode to select bookmarks when taping on them then pushing a button to delete them, and having a confirmation dialog to actually do it.

@kleinph
Copy link
Contributor

kleinph commented Oct 4, 2020

I am really in favor of removing the feature completely. Deleting bookmarks is not a task that is done to often. The currently implemented long tap selection and delete feature is enough for bulk deleting.

@apbitner
Copy link

@kbrosnan Let's remove it, I think the cons outweigh the pros here. Users can always delete via the inline 3-dot menu.

@apbitner apbitner removed the needs:UX-feedback Needs UX Feedback label Oct 15, 2020
@kbrosnan kbrosnan added the good first issue Good for newcomers label Nov 18, 2020
@kbrosnan
Copy link
Contributor Author

If someone wants to pick this up as a good first issue they should remove the code added by cd25323#diff-00ed770a60ab00f41043c251896f10124b006ec0bd2a93adb584d99a2e07078a

Other gestures such as swiping on the address bar and pull to refresh should continue to work.

@kleinph
Copy link
Contributor

kleinph commented Nov 18, 2020

I would like to try that

kleinph added a commit to kleinph/fenix that referenced this issue Nov 19, 2020
kleinph added a commit to kleinph/fenix that referenced this issue Nov 19, 2020
kleinph added a commit to kleinph/fenix that referenced this issue Nov 19, 2020
kleinph added a commit to kleinph/fenix that referenced this issue Nov 19, 2020
kleinph added a commit to kleinph/fenix that referenced this issue Nov 21, 2020
kleinph added a commit to kleinph/fenix that referenced this issue Nov 21, 2020
Also remove now obsolete feature flag and tests.

Closes mozilla-mobile#15362
bors bot pushed a commit that referenced this issue Nov 26, 2020
16646: For #15362: Remove swipe to delete for bookmarks r=Mugurell a=kleinph

Closes #15362



### Pull Request checklist
<!-- Before submitting the PR, please address each item -->
- [x] **Tests**: This PR includes thorough tests or an explanation of why it does not
This PR does remove a feature and its tests.
- [x] **Screenshots**: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
![no-swipe-delete-bookmarks](https://user-images.githubusercontent.com/566413/99639340-c6b92880-2a47-11eb-9686-604f09c07842.gif)
- [x] **Accessibility**: The code in this PR follows [accessibility best practices](https://github.com/mozilla-mobile/shared-docs/blob/master/android/accessibility_guide.md) or does not include any user facing features. In addition, it includes a screenshot of a successful [accessibility scan](https://play.google.com/store/apps/details?id=com.google.android.apps.accessibility.auditor&hl=en_US) to ensure no new defects are added to the product.
A feature was removed, so accessibility should not change.

### To download an APK when reviewing a PR:
1. click on Show All Checks,
2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
3. click on the "Fenix - assemble" task, then click "Run Artifacts".
4. the APK links should be on the left side of the screen, named for each CPU architecture


Co-authored-by: Philipp Klein <philipptheklein@gmail.com>
Mugurell pushed a commit that referenced this issue Dec 4, 2020
Removed now obsolete feature flag and tests.
Removed obsolete swipe refresh state from BookmarkFragmentState
Also adapted tests and remove obsolete ones.
pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this issue Aug 4, 2021
…la-mobile#16646)

Removed now obsolete feature flag and tests.
Removed obsolete swipe refresh state from BookmarkFragmentState
Also adapted tests and remove obsolete ones.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:ux-debt Feature polish and UX engineering debt Feature:Bookmarks Feature:Gesture good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants