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

To debate : Swipe mechanism improvements #905

Closed
RobbWatershed opened this issue Jul 1, 2020 · 5 comments
Closed

To debate : Swipe mechanism improvements #905

RobbWatershed opened this issue Jul 1, 2020 · 5 comments

Comments

@RobbWatershed
Copy link
Contributor

RobbWatershed commented Jul 1, 2020

I've been using FastAdapter to implement swipe to delete on my app as your sample app does :

  • Swipe (display leave-behind delete icon)
  • When swiped completely, item card is hidden and a 2s timer starts to leave time to hit the UNDO button
  • Upon timer end, content is deleted and item disappears for good

The way it works upsets the clumsiest part of my users (and they're a bunch, apparently) who complain that they've been deleting stuff unintentionally. I've now given up on understanding how it can even be possible and am trying to find a better implementation.


After a couple discussions, we found two interesting improvements on which I would like your opinion :

A/ Make swipe less sensitive (i.e. no sling; you have to intentionally swipe on all screen width to trigger the action)

and/or

B/ Implement a partial swipe

What I would love FastAdapter to do is a "partial swipe" where the swiped item card can go as far as a given distance to the left / right, and not disappear completely, so that the user can have the choice between :

  • Using the underlying action button (here it's a DELETE button)
  • Swiping back the opposite direction if he doesn't want to do the action

It's the behaviour of the "watch for later" screen of the Youtube app that you can see in action below

youtubeSwipe

Any thoughts about both of these suggestions ? 😁

@RobbWatershed
Copy link
Contributor Author

Looks like option A is pretty straightforward to implement (see https://stackoverflow.com/questions/51089275/swipe-delete-on-recycleview-is-very-sensitive)

I'll submit a PR about that one

mikepenz added a commit that referenced this issue Jul 1, 2020
Swipe customization (in response to #905)
@RobbWatershed
Copy link
Contributor Author

So I've been thinking about what you said about creating a separate specialized class for option B... and it looks like there are two different ways to acheive the desired result :

Option B1- Let the user display what he wants (buttons, images, icons...) under the item, using the same sub-layer as he's using with the current implementation.
What has to evolve is the ability for an item to be swiped partially and stay in that state to show the sub-layer under it.

I think that option can be acheived with minor additions to the current implementation and won't necessarily need to be "isolated" in a 2nd class. It also leaves the user with more flexibility, as he has total control on what he puts in the sub-layer


Option B2 - Draw buttons on the same layer as the item, in the empty space created where the item has been swiped away.

This one will probably require a separate class to be properly readable. It will also need a couple options to customize the drawn buttons (text, background color, text orientation and color, click handler....)


I personally would like to go with Option B1, but you're the boss around, so I'd like to have your opinion as well :)

@mikepenz
Copy link
Owner

mikepenz commented Jul 1, 2020

reading through it and thinking more also makes me tend towards "Option B1"

@RobbWatershed
Copy link
Contributor Author

Alright, so I've been trying to implement B1 with encouraging results. It looks like you were right all along : the code and the API will become a big mess if we don't separate the new behaviour (let's call it "drawer swipe") from the current behaviour (let's call it "delayed swipe").

Notable differences are :

  • itemSwiped callback is useless in drawer swipe because the item never gets swiped entirely; it just makes room for button(s)
  • leave-behind stuff are not relevant in drawer swipe because you want to see the buttons immediately rather than a placeholder
  • drawer swipe needs to know how much it can be swiped before stopping (the "drawer depth")

This makes it quite obvious both behaviours can't be implemented with the same components unless you aim at creating an incomprehensible API.


With that in mind, I'd like to validate a few architecture choices with you :

Library side

  • I intend to create siblings of the following classes to implement the new behaviour :

    • SimpleSwipeCallback -duplicate and adapt-> SimpleSwipeDrawerCallback
    • SimpleSwipeDragCallback -duplicate and adapt-> SimpleSwipeDrawerDragCallback
  • Question : Do we keep current class names to ensure retrocompatiblity (SimpleSwipeCallback, SimpleSwipeDragCallback...) or do we rename them (SimpleSwipeDelayedCallback, SimpleSwipeDelayedDragCallback...) to make their goal clearer ?

Sample app side

  • I intend to create siblings of the following classes to implement the new behaviour :
    • SwipeableItem -duplicate and adapt-> SwipeableDrawerItem
    • SwipeListActivity -duplicate and adapt-> SwipeDrawerListActivity
  • Question : On the sample app's main menu, do we keep a single menu item for "SwipeList Sample" and allow the user to swap behaviours (delayed vs. drawer) by adding a button on the activity's top bar, or do we create two different menu items ("SwipeList Sample (delayed)" and "SwipeList Sample (drawer)") ?

Once again, thanks in advance for your feeback 😁

@mikepenz
Copy link
Owner

mikepenz commented Jul 3, 2020

Thanks for spending so much time into that and coming back with such a detailed report :)

So for the library side. I would suggest to keep the original name to ensure retrocompatiblity. That will allow us to have this new feature in a minor update and won't require a major one due to breaking features. we could add a type alias though to have it also in the new proposed (more clear name)

In regards to the sample app. I am actually fine with both behaviors, what works best for you / means the least amount of work for you. I suppose having it as a new entry in the drawer will give it more visibility which it should get :)

Once again, thank you so much for the work! it's really amazing to see new improvements making their way into the lib :)

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

No branches or pull requests

2 participants