use existing delegate method to close open cells on table swipe

[DimaVartanian]

When a pan begins on the tableview, all open cells (except for the one being touched/panned) will call the swipeableTableViewCellShouldHideUtilityButtonsOnSwipe: delegate method and close if YES.

This is pretty much exactly how the Mail.app tables behaves and seems like something that should be included in the core.

[DimaVartanian]

I am seeing an occasional EXC_BAD_ACCESS KERN_INVALID_ADDRESS occur in observeValueForKeyPath:ofObject:change:context after making this change and I'm not sure why. From what I can tell, the add/remove observer calls are completely balanced out. I don't see how a cell could possibly be deallocated without removing itself as an observer. Perhaps someone else has an idea.

[DimaVartanian]

Okay, I believe this is ready to go now. I found that the observers were getting leaked because by the time didMoveToSuperview (and willMoveToSuperView:) is called for a cell getting removed, the containing tableview has already been deallocated, which prevents the cell from removing itself as an observer. It turned out to be an easy fix by just retaining the panGestureRecognizer and observing it directly. The observers no longer get leaked and both the tableview and cells get deallocated when they should (which would not be possible if the tableView was retained instead).

[CEWendel]

This is looking good only weird behavior I'm seeing is when you I have one side of utility buttons open and scroll down quickly the utility buttons of other cells are sometimes visible. May be something screwing up with just the state of the cell and how they are getting laid out as the table view scrolls quickly.

In the Mail app when you have the utility buttons open and scroll the table view the table view does not scroll it only closes the utility buttons. Do you think that is something we could implement instead of letting the user scroll and have the utility buttons close at the same time?

[DimaVartanian]

1. Hmm so for point Only Swipe Gesture will work #1 I see what you are talking about. I didn't notice it before because I only use a drawer on one side, but the problem occurs when the same cell which is closing is being reused several times on a fast scroll, and only if the cell has drawers on both sides (the opposite side drawer to the one that was already open appears a bit). I do see that if I turn the animation off for the "hideUtilityButtonsAnimated:` call, the problem goes away, so I suspect it has something to do with the animation occurring while the cell is being reused/laid out. I will look into this and try to figure out why it's happening and get back to you about it.
2. As far as the Mail app functionality, that's something I considered as well and decided not to pursue it at the time for 2 main reasons:

• I could not figure out an easy way to replicate this which fits into the current architecture of the component. In order to correctly intercept and block a pan/swipe gesture we would need to either intercept the tableview delegate, or the gesture recognizer delegate, which is probably not a good idea. the other idea that I've seen similar implementations use is simply attaching an invisible overlay view over the tableview when a cell is in an open state in order to intercept any swipes or pans, and then remove it when the cell closes. This is something we could do but I could see there being some problems with it potentially.
• I think in Mail and Messages, this is actually may be accomplished at the UITableView level. When you open a cell it puts the table into "editing" mode, during which pans are disabled. This could of course be easily accomplished on our end by just using the cell delegate to see when a cell opens and close it instead of allowing a pan when it first occurs. This however, is not exactly modular as it is at the UITableView level.

Do you have any other ideas on how to it could be accomplished be done in a passive/modular way?

• The second reason I did it that way was actually usability. I think allowing the user to scroll instead of blocking touches while a cell being open feels much more responsive and behaves closer to what a user is actually trying to accomplish (If they try to start scrolling, we can assume they actually want to scroll and let them do so and adapt our view rather than blocking them). What do you think about this?

[DimaVartanian]

Hey! So I messed around with it a bit more and isolated the issue. Although it was hard to see exactly what was going on, the issue was basically that the scroll offset was going crazy when an animation was in progress while the cell was being reused / laid out.

It was a very easy fix, simply forcing the cell into the Center state instantly in prepareForReuse with hideUtilityButtonsAnimated:. After doing that, everything is working as expected. Can you take a look?

[ebgraham]

On issue #2, I agree that allowing the user to scroll is better behavior than the Apple Mail default. However, this library should be a 1:1 replacement for Apple's behavior. Perhaps emulate Mail as default, and allow the scrolling with a BOOL?

[DimaVartanian]

The issue is that the implementation may become significantly more complicated. The 2 ways I can think of doing this are at the UITableView level by disabling scrolling when there is an open cell, or having the cell itself overlay an invisible view over its containing table when it is open. This view could intercept touches if they are not within the frame of the open cell, and just close the cell instead of passing them onto the table view.

[ebgraham]

How about adding to a list of openCellIndexPaths whenever the delegate swipeableTableViewCell:scrollingToState: is called with _cellState != kCellStateCenter, and a generic gesture recognizer attached to self.containingTableView. On any gesture, close all in openCells.

Also, the current PR doesn't handle the case correctly where swipeableTableViewCellShouldHideUtilityButtonsOnSwipe: returns NO, which allows multiple cells to be open at once. Scrolling should close them too, since they will be closed when coming back into view.

[DimaVartanian]

So as for your first solution, this involves adding code and an additional data structure at the UITableView level, right? Could you create a fork so I/we could take a look? It's tough for me to picture it from that description.

As far the second part, I'm not sure what you mean. The functionality I wrote uses that delegate call to decide whether or not to close any open cells. This allows the user of the component to use that delegate method to specify if they want to close a cell if another cell opens or if the table scrolls. What exactly is the issue with it?

[ebgraham]

You are right about the second part, it's actually just how the library works -- when an open cell goes off screen it always comes back closed.

Hopefully this weekend I can get to maintaining an array of open cells. This could also be used to make the above case more consistent; i.e. an open cell that goes off screen will come back on screen as an open cell.

[DimaVartanian]

Looking forward to seeing what you're talking about. One thing to mention hoever: what you just mentioned (an open cell going off screen and coming back an open cell) is very specifically avoided here. The point is that by default, the cells are reset when they are dequeued. Having a cell remain open when it is dequeued in the bare component would not make sense, because there is no model tied to the cells and there is no guarantee or implication that the same cell would be representing the same object (since the cells themselves are reused at different indexpaths). It is up to your application and the code that uses these cells to keep track of any open cells based on your models. and set the state when configuring the cell in cellForRowAtIndexPath:. It is possible I misunderstood you though, in which case.. carry on :)

[DimaVartanian]

@CEWendel Have you had a chance to look at the modified pull request yet?

[CEWendel]

Just merged it, thanks.

Would eventually like to make this exactly like Mail.app when it comes to functionality, but this works for now (in fact I actually like this scrolling behavior better)