-1

When I tap on button it will trigger multiple times.

CollectionViewCell file code

class PhotoCell: UICollectionViewCell {
    @IBOutlet weak var deleteButton: UIButton!
}

ViewController - cellforItemAt method implementation

func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
        guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: "PhotoCell", for: indexPath) as? PhotoCell else { return UICollectionViewCell() }

        let action = UIAction { _ in
            print("Delete button tapped!!!", indexPath.row)
        }

        cell.deleteButton.addAction(action, for: .touchUpInside)

        return cell
    }

If I configure UIButton addTarget then it work fine but I am not sure why it's not working with addAction.

HangarRash
  • 7,314
  • 5
  • 5
  • 32
Yogesh Patel
  • 1,893
  • 1
  • 20
  • 55
  • Collection view cells that have gone off screen when you scroll are reused for new index paths. So, depending on how much you scroll back and forth you are repeatedly adding more and more actions to the same delete button. Your cell should add the action once in its initialiser and use a delegate protocol or a closure to communicate back to the view controller when it is tapped. – Geoff Hackworth Mar 31 '23 at 14:02
  • I have 10 visible cells and If I click on the button it will trigger multiple times. UIButton addTarget is working fine but not sure why addAction is not working. – Yogesh Patel Mar 31 '23 at 14:04
  • Yes I know using protocol and closure we can achieve this thing but want to do using addAction button. It useful and simple approach if It works. – Yogesh Patel Mar 31 '23 at 14:08
  • You can’t keep adding actions or target/actions in `cellForItem` because it will be called multiple times for the exact same collection view cell object when they are reused. – Geoff Hackworth Mar 31 '23 at 14:09
  • As a test, change your code to return only 1 item for the section and run the app. Are you saying that tapping the delete button will print the same message multiple times for index path 0,0? – Geoff Hackworth Mar 31 '23 at 14:13
  • This approach works fine on - https://stackoverflow.com/a/71667574/8201581 is this approach not good? – Yogesh Patel Mar 31 '23 at 14:13
  • If I click on the 5th cell delete button, it will print - 5th cell indexpath.row and 1st cell indexpath.row. It should call only 5th cell. – Yogesh Patel Mar 31 '23 at 14:14
  • Adding a target/action will work if you only do it once per actual instance of a collection view cell. So should adding an action. If the code that adds the action is in the collection view data source method for getting a cell then when a cell is reused you will add a second action. Then when it is reused again you will add a third action and so on. – Geoff Hackworth Mar 31 '23 at 14:15
  • Yes, I tested it's adding action again and again. So We can not achieve this using addAction. Any solution? – Yogesh Patel Mar 31 '23 at 14:17
  • “ If I click on the 5th cell delete button, it will print - 5th cell indexpath.row and 1st cell indexpath.row. It should call only 5th cell.” That sounds exactly like the problem I am describing. You said you had 10 visible cells. Do you mean 10 cells in your section or 10 cells that can all be seen on screen at once (so none of them are reused)? I suspect you can see either 3 or 4 cells so when you scroll to see the 5th, it is actually reusing the 1st, which is why both actions are called. – Geoff Hackworth Mar 31 '23 at 14:17
  • Yes, As you told me I am able to address the issue just curious how we can solve this issue using addAction. – Yogesh Patel Mar 31 '23 at 14:19
  • addAction should work fine as long as you only add it once per button. Add some debug prints to show the collection view cell object and index path when you add the action. I am sure you will be adding more than one action to the same cell as you scroll and cells are reused. – Geoff Hackworth Mar 31 '23 at 14:23
  • Yes, I just see removeAction method but it won't work. I have added this method only in cellforItem. Let me know if you have any suggestions! Thank you – Yogesh Patel Mar 31 '23 at 14:25
  • I added the UIAction code in cell file awakeFromNib. It's calling only one time but now I am not able get indexPath and want to do the same thing in ViewController not in cell. – Yogesh Patel Mar 31 '23 at 14:29
  • And that is where a delegate protocol or a closure can be used so that the cell can inform the view controller that the button has been tapped. If you have a closure property to act as a callback then you can safely overwrite it in `cellForItem`. If you have a protocol method you would pass the cell object as a parameter so the view controller could ask the collection view to find the index path from the cell. – Geoff Hackworth Mar 31 '23 at 14:31
  • Hahaha, Yes that's true. Just want to implement the new functionality. Maybe addAction will not work in the cell because it will add multiple times. Thank you! – Yogesh Patel Mar 31 '23 at 14:33
  • addAction will work fine in the cell. It will not work if you keep calling it for the same cell instance (when cells are recycled) from the view controller. Just add your own closure to the cell if you want to avoid a delegate protocol like in this answer: https://stackoverflow.com/a/64508719/870671 – Geoff Hackworth Mar 31 '23 at 14:40
  • Yes, I did the same thing now. I use the closure approach! Thank you for your time and help! – Yogesh Patel Mar 31 '23 at 14:43

2 Answers2

1

The actions get triggered multiple times because some instances of PhotoCell are reused by the UICollectionView. This means that the deleteButton of the PhotoCell returned by dequeueReusableCell() may already have actions added to it and will eventually possess multiple actions for the same events.

One possible solution is to override prepareForReuse() in PhotoCell and remove the actions there.

class PhotoCell: UICollectionViewCell {
    @IBOutlet weak var deleteButton: UIButton!
    var actions: [(UIAction, UIControl.Event)] = []

    func addAction(_ action: UIAction, for event: UIControl.Event) {
        self.actions.append((action, event))
        self.deleteButton.addAction(action, for: event)
    }

    override func prepareForReuse() {
        super.prepareForReuse()
        for tuple in self.actions {
            self.deleteButton.removeAction(tuple.0, for: tuple.1)
        }
        self.actions = []
    }
}

When using the above code addAction needs to be called on cell instead of cell.deleteButton in the function func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath):

func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
    guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: "PhotoCell", for: indexPath) as? PhotoCell else { return UICollectionViewCell() }

    let action = UIAction { _ in
        print("Delete button tapped!!!", indexPath.row)
    }
    cell.addAction(action, for: .touchUpInside)

    return cell
}
Johann Schwarze
  • 161
  • 1
  • 4
  • Thank you so much for pointing this out! Your code works fine. The solution for this issue is the prepareForReuse(). It will work without taking actions array. – Yogesh Patel Apr 01 '23 at 16:07
  • Please check my answer we have to remove all targets in prepareForReuse() method. It will work! Thank you for your help – Yogesh Patel Apr 01 '23 at 16:16
1

One possible solution is to override prepareForReuse() in PhotoCell, as Johann mentioned.

preparaForReuse() is use to Performs any clean up necessary to prepare the view for use again.

We can remove all events from the button there!

class PhotoCell: UICollectionViewCell {
    @IBOutlet weak var deleteButton: UIButton!

     override func prepareForReuse() {
        super.prepareForReuse()
        self.deleteButton.removeTarget(nil, action: nil, for: .allEvents)
    }

}

Add action in func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) like normally we do.

func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
        guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: "PhotoCell", for: indexPath) as? PhotoCell else { return UICollectionViewCell() }
        let action = UIAction { _ in
            print("Delete button tapped!!!", indexPath.row)
        }
        cell.deleteButton.addAction(action, for: .touchUpInside)
        return cell
    }
Yogesh Patel
  • 1,893
  • 1
  • 20
  • 55