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

UITableView shows delete button on swipe cell by default #907

Closed
ishkawa opened this issue Sep 24, 2016 · 6 comments
Closed

UITableView shows delete button on swipe cell by default #907

ishkawa opened this issue Sep 24, 2016 · 6 comments

Comments

@ishkawa
Copy link
Contributor

ishkawa commented Sep 24, 2016

Short description of the issue:

UITableView which displays cells using rx.items has unwanted editingStyle. It should have .none by default, but it seems to be set as .delete. Here's a screen shot of the bug that appeared in RxExample.

Self contained code example that reproduces the issue:

https://github.com/ReactiveX/RxSwift/blob/e4e422c4f1da21aa78d9a4c982c572533f74645d/RxExample/RxExample/Examples/SimpleTableViewExample/SimpleTableViewExampleViewController.swift

Xcode version:

Version 8.0 (8A218a)

Expected outcome:

UITableView doesn't show delete button by default.

What actually happens:

UITableView shows delete button by default.

@kzaher
Copy link
Member

kzaher commented Sep 25, 2016

Hi @ishkawa ,

That's actually part of Apple black magic :)

If your delegate implements

- (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath;

and Rx delegate "does" because it fits under observable event spectrum, then UIKit has that unfortunate behavior.

You can test it out by temporarily modifying this code in _RxDelegateProxy.m

-(BOOL)respondsToSelector:(SEL)aSelector {
    if (aSelector == @selector(tableView:commitEditingStyle:forRowAtIndexPath:)) {
        return NO;
    }
    return [super respondsToSelector:aSelector]
    || [self._forwardToDelegate respondsToSelector:aSelector]
    || [self canRespondToSelector:aSelector];
}

The solution for this is simple, but annoying.

    // viewDidLoad
    tableView.rx
            .setDelegate(self)
            .addDisposableTo(disposeBag)

// to prevent swipe to delete behavior
func tableView(_ tableView: UITableView, editingStyleForRowAt indexPath: IndexPath) -> UITableViewCellEditingStyle {
    return .none
}

@ishkawa
Copy link
Contributor Author

ishkawa commented Sep 25, 2016

I understood what is happening. I didn't notice the cause of this issues is tableView(_:commit:forRowAt:), because I didn't know the method has such side-effect.

Thank you for the concise explanation 👍

BTW, my project contains approximately 20 tableView(_:editingStyleForRowAt:) to workaround this issue, but I don't think all users of RxCocoa have to do that.

Instead of implementing the delegate method in each view controllers, I suggest overriding responds(to:) of RxTableViewDataSourceProxy to avoid forwarding invocation of tableView(_:editingStyleForRowAt:) until itemInserted or itemDeleted is subscribed.

@ishkawa
Copy link
Contributor Author

ishkawa commented Sep 25, 2016

If the policy I suggested is OK, I will work on it.
Here's rough implementation: ishkawa@a086f9c

ishkawa added a commit to ishkawa/RxSwift that referenced this issue Sep 25, 2016
@kzaher
Copy link
Member

kzaher commented Sep 25, 2016

Hi @ishkawa ,

Apple's black magic is stong :)

UITableView and UICollectionView cache a list of possible selectors once delegate or dataSource is set, so one would need to set delegate or dataSource once number of observers is changed. There are also some threading concerns, etc ...

I would like to take some time and think what is the simplest way to solve this, there is a lot of possible solutions out there, including some similar to one you've suggested.

@ishkawa
Copy link
Contributor Author

ishkawa commented Sep 26, 2016

UITableView and UICollectionView cache a list of possible selectors once delegate or dataSource is set, so one would need to set delegate or dataSource once number of observers is changed. There are also some threading concerns, etc ...

😇

I would like to take some time

I agree with you. It needs further consideration.

kzaher added a commit that referenced this issue Oct 1, 2016
…w delegate proxy responding to `#selector(UITableViewDataSource.tableView(_:commit:forRowAt:)`. #907 #884
@kzaher
Copy link
Member

kzaher commented Oct 1, 2016

I think this should be solved now.

@ishkawa please let me know if this doesn't work. I'll close issue for now.

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

No branches or pull requests

2 participants