2

Using the following code, when I click a cell to create a checkmark accessory, it repeats the checkmark every 12 rows. Any ideas as to why?

      func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell { 

          let cell = tableView.dequeueReusableCellWithIdentifier("Cell", forIndexPath: indexPath) as? UITableViewCell

          cell?.textLabel = "\(indexPath.row)"

          return cell!

      }


      func tableView(tableView: UITableView, willSelectRowAtIndexPath indexPath: NSIndexPath) -> NSIndexPath? {


         if let cell = tableView.cellForRowAtIndexPath(indexPath) as? UITableViewCell {

              if cell.accessoryType == UITableViewCellAccessoryType.Checkmark
              {
                 cell.accessoryType = UITableViewCellAccessoryType.None
              }
              else
              {
                  cell.accessoryType = UITableViewCellAccessoryType.Checkmark
               }
          }

         return indexPath
      }
rmaddy
  • 314,917
  • 42
  • 532
  • 579
Andres Marin
  • 251
  • 3
  • 12

1 Answers1

8

As Cell objects are reused, you can't rely on them for storing data or state. They are simply views on data you have in your data model. You need to reset the checked/non-checked state in cellForRowAtIndexPath

One technique for recording cell selection state is to use a Set to store the indexPaths that are selected. Here is a simple example that shows this technique -

class ViewController: UIViewController,UITableViewDataSource,UITableViewDelegate {

    var checkedRows=Set<NSIndexPath>()

    func tableView(tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
        return 100     // Simple example - 100 fixed rows
    }

    func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath) -> UITableViewCell {
        var cell=tableView.dequeueReusableCellWithIdentifier("cell", forIndexPath: indexPath) as!UITableViewCell

        cell.textLabel!.text="Row \(indexPath.row)"    

        cell.accessoryType=self.accessoryForIndexPath(indexPath)


        return cell
    }

    func accessoryForIndexPath(indexPath: NSIndexPath) -> UITableViewCellAccessoryType {

        var accessory = UITableViewCellAccessoryType.None

        if self.checkedRows.contains(indexPath) {
            accessory=UITableViewCellAccessoryType.Checkmark
        }

        return accessory
    }

    func tableView(tableView: UITableView, didSelectRowAtIndexPath indexPath: NSIndexPath) {
        tableView.deselectRowAtIndexPath(indexPath, animated: true)

        if checkedRows.contains(indexPath) {
            self.checkedRows.remove(indexPath)
        } else {
            self.checkedRows.insert(indexPath)
        }

        if let cell=tableView.cellForRowAtIndexPath(indexPath) {
            cell.accessoryType=self.accessoryForIndexPath(indexPath)

        }
    }

}
Paulw11
  • 108,386
  • 14
  • 159
  • 186
  • How about using the indexPathsForSelectedRows property on the UITableView instead of reinventing it? – Kaan Dedeoglu Aug 05 '15 at 00:45
  • If you use that property then you have to keep the checked cells selected which, in my opinion, isn't very attractive visually. Also, you have to keep iterating the selected rows property, so it isn't particularly efficient – Paulw11 Aug 05 '15 at 00:52
  • cell.selectionStyle = UITableViewCellSelectionStyleNone solves the visual side. And O(1) access vs O(n) access isn't really an issue unless you have many thousands of cells anyway. All I'm saying is the UITableView API is enough to solve the original problem without introducing any extra logic. – Kaan Dedeoglu Aug 05 '15 at 01:02
  • Feel free to add your own answer using that technique. Personally, I like the visual feedback of the momentary selection highlight. This technique also lends itself to simply persist/de-persist of the checked state - you can use the data object in the set rather than then NSIndexPath – Paulw11 Aug 05 '15 at 01:06