Row with SwitchAccessory loses its state after scrolling the table

[n3d1117]

Hello!

I am using a Row with UISwitch as seen in the example project here.
However when scrolling down the table and then up again, the switch loses its on state.

Tested straight from example project:

Is this due to cell reusing? How can I preserve switch state on scroll?

Thanks

EDIT: This is the way I'm using it. Am I doing something wrong?

var value: Bool = false

Row(text: "UISwitch", accessory: .switchToggle(value: self.value) { [unowned self] newValue in
     self.value = newValue
})

[dmiluski]

Hi @n3d1117 , this is likely due to cell reuse/dequing/prepareForReuse.

Although you are initiating Row with an accessory "switchToggle(value:)", since this value is a struct rather than var, it doesn't change. So your row is still referencing an old boolean value.

To Update to support with current Static implementation:
You may replace this row/update accessory with a newly generated Row Item?
Or regenerate the full dataSource Section/Rows.

If you're interested in providing this support, perhaps you might file a PR to support updating self's accessoryView's value given it's a switch? As that seems like a nice to have functionality other folks would appreciate.

[n3d1117]

Hi @dmiluski ! Thanks for your answer.

You may replace this row/update accessory with a newly generated Row Item?

Would be the best place to update the row/accessory for this particular type of row? I tried hooking willDisplayCell or cellForRow using the context property to pass the updated bool, but of course it wouldn't work because the context received still holds the outdated value.

Or regenerate the full dataSource Section/Rows

This works, although it feels bad to reload the row just because switch value changes. I'm doing something like this in ViewController:

var test: Bool = false

var testRow: Row {
    let row = Row(text: "UISwitch", accessory: .switchToggle(value: self.test) { [unowned self] newValue in
        self.test = newValue
        self.reloadTestRow()
     })
    return row
}

func reloadTestRow() {
    // Delay by 0.2 seconds to preserve UISwitch animation
    DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 0.2) {
        self.dataSource.sections[0].rows[5] = self.testRow
    }
}

In viewDidLoad:

dataSource.sections = [
    Section(header: "Accessories", rows: [
        ...,
        testRow, // index 5
        ...,
    ]
]

However this is pretty bad, as it involves knowing the index of each row with a switch, as well as extracting every one of these rows into their own var to reassign it later.

I'm sure there's a better way, and I would be glad if you could point me to the right direction. Thanks again!

[dmiluski]

From what I understand this Repo began as a way to quickly render/show a static set of cells. It did evolve to allow for mutation, but it's not quite beneficial for that just yet in terms of something like a form without reloading the section.

It was only recently that a switch support was added, and consideration into this use case may have been overlooked. Funny enough, just looking through my use case, our action is handled with an API call, who's latency covers up the Row replacement that you mentioned would be glitchy when done and replaced locally.

[dmiluski]

Hm.. wondering if there's a way to add a row mutating closure which could update your model and play friendly with tableView refresh/reload.

[dmiluski]

What's available:
SwitchAccessory
Static.Cell Protocol

Potentially:
What you might be able to do is to carry state in the context property? And use that context property to configure. Where "SwitchModelProvider" is a reference type which can be mutated.

final class SwitchCell: UITableViewCell {

    private let toggle = UISwitch()

    // Any Setup for this Cell
}

class SwitchModelProvider {
   var isOn: Bool
}

extension SwitchCell: Static.Cell {
    func configure(row: Row) {
        textLabel?.text = row.text
        detailTextLabel?.text = row.detailText

        guard let modelProvider = row.context?["model"] as? SwitchModelProvider else { return }
        
       toggle.isOn = modelProvider.isOn
    }
}

This way, you don't need to refresh the full table. And the only item that is mutated is in context, which is only re-applied if table is reloaded, or dequeue for reconfiguration. So far just a theory. Would need to test it out.

[n3d1117]

What you might be able to do is to carry state in the context property?

It worked! Somehow I tested the same thing yesterday, but using Bool directly (which is a value type and not a reference type), so it didn't work. Using a class did the trick!
Here's what I got so far:

final class SwitchCell: UITableViewCell, Cell {
    
    var valueChange: ValueChange?
    private let toggle = UISwitch()
    
    override init(style: UITableViewCell.CellStyle, reuseIdentifier: String?) {
        super.init(style: style, reuseIdentifier: reuseIdentifier)
        
        accessoryView = toggle
        toggle.addTarget(self, action: #selector(change), for: .valueChanged)
    }
    
    @objc func change() {
        valueChange?(toggle.isOn)
    }

    func configure(row: Row) {
        textLabel?.text = row.text
        if let modelProvider = row.context?["model"] as? SwitchModelProvider {
            toggle.isOn = modelProvider.isOn
        }
        if let vc = row.context?["valueChange"] as? ValueChange {
            self.valueChange = vc
        }
    }
} 

class SwitchModelProvider {
    var isOn: Bool
    
    init(on: Bool) {
        self.isOn = on
    }
}

In ViewController:

var prov: SwitchModelProvider = SwitchModelProvider(on: false)

Row(text: "UISwitch1", cellClass: SwitchCell.self, context: ["model": prov, "valueChange": { [unowned self] newValue in
     self.prov.isOn = newValue
}]),

Works great! The only downside is that SwitchCell now needs to be aware of what property is it toggling (i.e. modelProvider.isOn), but that's inevitable I guess.
Also, if I had more than one Bool in my model provider I would maybe have to wrap them in an enum and pass that to the context dictionary as well, and configure the row accordingly.

Do you think it can be improved in any way?
And what would the best way to file a PR on this? Replace SwitchAccessory altogheter in favor of SwitchCell, or keep both? Or maybe just by adding this SwitchCell demo to the example project?

Again, thanks a lot for your help @dmiluski

[dmiluski]

Looks very cool.

Improvements:
While continuing to think about this. Wondering if we could bind the switch to the model with KVO/KeyPath? Providing the model change/update observation? That way the switch updates the model, and model changes update the cell?

As for separate Cell vs Accessory, I think they both serve their own purposes. I think in this particular case the cell may be preferable over the accessory given the requirement of mutability you mention. For other's implementations, it's very likely that these accessories/rows are being regenerated after an API call in order to keep in sync.

[n3d1117]

Hi again @dmiluski! Using KeyPaths I came up with this:

final class SwitchCell<T>: UITableViewCell, Cell {
    
    private let toggle = UISwitch()
    
    typealias BoolWritableKeyPath = WritableKeyPath<T, Bool>
    
    private var object: T?
    private var keyPath: BoolWritableKeyPath?
    
    override init(style: UITableViewCell.CellStyle, reuseIdentifier: String?) {
        super.init(style: style, reuseIdentifier: reuseIdentifier)
        
        accessoryView = toggle
        toggle.addTarget(self, action: #selector(change), for: .valueChanged)
    }
    
    @objc func change() {
        if let path = keyPath {
            object?[keyPath: path] = toggle.isOn
        }
    }

    func configure(row: Row) {
        textLabel?.text = row.text
        if let modelProvider = row.context?["model"] as? T, let keyPath = row.context?["keyPath"] as? BoolWritableKeyPath {
            self.object = modelProvider
            self.keyPath = keyPath
        }
    }
}

And then declare the row like this:

Row(text: "UISwitch", cellClass: SwitchCell<SwitchModelProvider>.self, context: ["model": prov, "keyPath": \SwitchModelProvider.isOn])

It seems to work! The cell now automatically updates the model when switch value changes. What do you think?

[eliperkins]

Nice solution, @n3d1117! I think we've been struggling with the context API for some time, but this gives some nice type safety.

[dmiluski]

Second @eliperkins 's comment, very cool. I'd be happy moving forward with this.

@eliperkins any thoughts on constant keys for context in this case as it requires a pair to exist? Given the pain of the context api.