antoninbiret / RxEureka

This library is a small RxSwift wrapper around Eureka
MIT License
37 stars 9 forks source link

Subscribe to `editingDidEnd` #10

Closed marbetschar closed 6 years ago

marbetschar commented 6 years ago

It would be awesome to filter out value changes only after the user has finished editing. On the Eureka side this can be done by adding a onCellHighlightChanged callback like follows:

row.onCellHighlightChanged { _, row in
    guard false == row.isHighlighted, row.wasChanged else { return }
    //the user has finished editing and the row was changed
}

However, this approach has one major flaw: This event may miss some changes as it is not fired for every row type. E.g. SwitchRow does not change it's cell highlight state.

That said, we may simply add a new extension for isHighlighted the developer can subscribe to. This extension looks very similar to the var value: ControlProperty<String?>:

public var isHighlighted: ControlProperty<Bool> {
    var base: Base? = self.base

    let source = Observable<Bool>.create { observer in
        observer.onNext(base?.isHighlighted ?? false)

        base?.onCellHighlightChanged{ _, row in
            observer.onNext(row.isHighlighted)
        }
        return Disposables.create {
            base = nil
            observer.onCompleted()
        }
    }
    let bindingObserver = BindableObserver(container: self.base) { (row, value) in
        row.isHighlighted = value
    }
    return ControlProperty(values: source, valueSink: bindingObserver)
}

With this in place, the developer can choose to use the following code in order to only receive the latest value after the user has finished editing (untested):

Observable.combineLatest(
    row.rx.isHighlighted,
    row.rx.value
){ (isHighlighted: $0, value: $1) }
.subscribe(onNext: { isHighlighted, value in
   //`isHighlighted == false` indicates that the user did end editing
   //`value` contains the latest value
}).dispose(by: disposeBag)

What's your thoughts?

antoninbiret commented 6 years ago

Hi @marbetschar. Adding new observable properties to the Rows is really welcome. I'll merge #13 when the review is done. Thanks

antoninbiret commented 6 years ago

I'm closing the issue as #13 is now merged. Thanks @marbetschar