Search code examples
iosswiftcollectionview

I Want to change An Image On tap in UICollection View, Actually Image is Changing, But Not At right Place


I am trying to change selected and unselected image on a tap in collection view, but if I select but from the first index it reflecting in other indices. I want only one selection at a time, but it's reflecting on other sections too.

This is my struct for collection view.

struct teamSelected {
    var logoImage: String
    var isImageSelected: Bool 
}

I made a variable for currentIndex

var currentIndex : Int = 0

Here is how data for my collection view looks like:

var teamSelectionList: [teamSelected] = [
    teamSelected(logoImage: "ic_team_yellow_big", isImageSelected: false),
    teamSelected(logoImage: "ic_team_red_big", isImageSelected: false),
    teamSelected(logoImage: "ic_team_purple_big", isImageSelected: false),
    teamSelected(logoImage: "ic_team_blue_big", isImageSelected: false),
    teamSelected(logoImage: "ic_team_green_big", isImageSelected: false),
    teamSelected(logoImage: "ic_team_orange_big", isImageSelected: false)
]

Here is my collection view methods:

extension TeamViewController : UICollectionViewDelegate , UICollectionViewDataSource, UICollectionViewDelegateFlowLayout {
    func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
        return  teamSelectionList.count
    }
    
    func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
        let teamSelection : TeamSelectionCollectionViewCell = self.teamCollectionView.dequeueReusableCell(withReuseIdentifier: "teamCell", for: indexPath) as! TeamSelectionCollectionViewCell
        let row = teamSelectionList[indexPath.row]
        teamSelection.logoImage.image = UIImage(named: row.logoImage)
        teamSelection.logoButton.isSelected = row.isImageSelected
        //teamSelection.logoButton.layer.setValue(row, forKey: "index")
        
        
        teamSelection.logoButton.tag = indexPath.row
        teamSelection.logoButton.addTarget(self, action: #selector(logoButtonTapped), for: .touchUpInside)
        teamSelection.seperatorView.isHidden = indexPath.row == 2 || indexPath.row == self.teamSelectionList.count - 1 ? true : false
        
        
        
        return teamSelection
    }
    
    func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, sizeForItemAt indexPath: IndexPath) -> CGSize
    {
        return CGSize(width: (teamCollectionView.frame.width / 3), height: 110.0)
    }
    
    
    @objc func logoButtonTapped(sender: UIButton){
        
       // self.teamSelectionList[sender.tag].isImageSelected = true
     //   self.teamSelectionList[self.currentIndex].isImageSelected = self.currentIndex != sender.tag ? false : true

        self.currentIndex = sender.tag
        if (teamSelectionList[self.currentIndex].isImageSelected == false){
            teamSelectionList[self.currentIndex].isImageSelected = true
            sender.setImage(UIImage(named: "ic_radio_selected"), for: UIControl.State.normal)
        } else {
            teamSelectionList[self.currentIndex].isImageSelected = false
            sender.setImage(UIImage(named: "ic_radio_normal"), for: UIControl.State.normal)
        }
        self.teamCollectionView.reloadData()
    }
}

This is the output I'm getting:


Solution

  • You're updating the selection image with sender.setImage after the tap. Collection view is reusing cells and there's no guarantee that the same cell will be used on the next layout.

    I suggest you moving it from logoButtonTapped into cellForItemAt, for example like this:

    teamSelection.logoButton.isSelected = row.isImageSelected
    teamSelection.logoButton.setImage(
        UIImage(named: row.isImageSelected ? "ic_radio_normal" : "ic_radio_selected"),
        for: UIControl.State.normal
    )
    

    Also a couple of tips.

    1. According to swift code style guide, names of classes and structs start with an uppercase letter and use camel case, e.g. struct TeamSelected instead of struct teamSelected. It's much easier to read when you understand wether you're calling a function or a class/struct constructor.
    2. It's much easier to store selected indices instead of storing selection state for each item. It takes less code and decreases mistake probability. You code can be updated like this:
    class TeamViewController: UIViewController {
        let teamLogoList = [
            "ic_team_yellow_big",
            "ic_team_red_big",
            "ic_team_purple_big",
            "ic_team_blue_big",
            "ic_team_green_big",
            "ic_team_orange_big",
        ]
        var selection = Set<Int>()
    }
    
    extension TeamViewController : UICollectionViewDelegate , UICollectionViewDataSource, UICollectionViewDelegateFlowLayout {
        func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
            return teamLogoList.count
        }
    
        func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
            let teamSelection : TeamSelectionCollectionViewCell = self.teamCollectionView.dequeueReusableCell(withReuseIdentifier: "teamCell", for: indexPath) as! TeamSelectionCollectionViewCell
            let index = indexPath.row
            teamSelection.logoImage.image = UIImage(named: teamLogoList[index])
            let isImageSelected = selection.contains(index)
            teamSelection.logoButton.isSelected = isImageSelected
            teamSelection.logoButton.setImage(
                UIImage(named: isImageSelected ? "ic_radio_normal" : "ic_radio_selected"),
                for: UIControl.State.normal
            )
            //teamSelection.logoButton.layer.setValue(row, forKey: "index")
    
            teamSelection.logoButton.tag = indexPath.row
            teamSelection.logoButton.addTarget(self, action: #selector(logoButtonTapped), for: .touchUpInside)
            teamSelection.seperatorView.isHidden = indexPath.row == 2 || indexPath.row == self.teamLogoList.count - 1 ? true : false
    
    
    
            return teamSelection
        }
    
        func collectionView(_ collectionView: UICollectionView, layout collectionViewLayout: UICollectionViewLayout, sizeForItemAt indexPath: IndexPath) -> CGSize
        {
            return CGSize(width: (teamCollectionView.frame.width / 3), height: 110.0)
        }
    
    
        @objc func logoButtonTapped(sender: UIButton){
            let index = sender.tag
            if (selection.contains(index)){
                selection.remove(index)
            } else {
                selection.insert(index)
            }
            self.teamCollectionView.reloadData()
        }
    }