Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce unbind methods based binding identifiers #73

Merged
merged 2 commits into from Feb 1, 2017

Conversation

mac-cain13
Copy link
Contributor

This is an implementation of #70 (and thus also of #53). Feedback is very welcome, I had some spare time to code a solution. You probably have feedback on what is where, comments, tests and coding conventions. Feel free to update the PR or add comments so I can make some changes.

(Or even take this as inspiration/a starting point to start your own implementation.)

Most important changes:

  • bind returns an opaque TweakBindingIdentifier as a reference to that binding
  • unbind can be used to unregister a bound closure
  • Introduced MultiTweakBinding to keep logic for bindMultiple simple and consistent with bind
  • bindMultiple returns MultiTweakBindingIdentifier that can be used with unbindMultiple

Other small changes:

  • tweakBindings are now based on hashable AnyTweak instead of the "arbitrary" tweak.persistenceIdentifier
  • Removed unused tweak property from TweakBinding
  • Made binding private in TweakBinding

Usage:

let colorTintBinding = ExampleTweaks.bind(ExampleTweaks.colorTint) { button.tintColor = $0 }
// Some time later
ExampleTweaks.unbind(colorTintBinding)

- `tweakBindings` are now based on hashable `AnyTweak` instead of the "arbitrary" `tweak.persistenceIdentifier`
- `bind` returns an opaque `TweakBindingIdentifier` as a reference to that binding
- `unbind` can be used to unregister a bound closure
-  Introduced `MultiTweakBinding` to keep logic for `bindMultiple` simple and consistent with `bind`
- `bindMultiple` returns `MultiTweakBindingIdentifier` that can be used with `unbindMultiple`
- Removed unused `tweak` property from `TweakBinding`
- Made `binding` private in `TweakBinding`
@bryanjclark
Copy link
Owner

Thanks so much!! I'll review this in a bit. Your notes sound great - excited to peek under the hood here.

@bryanjclark
Copy link
Owner

Just did a review - this looks wonderful! Sorry for this taking foooooorever, you've probably moved on by now. My bad!

Two things I noticed:

  • I wish there were a single struct, like an AnyTweakBindingIdentifier, that could represent a TweakBindingIdentifier or a MultiTweakBindingIdentifier -- though I'm still puzzling through how we could do that. It'd make it easier for adopters to hold on to a single Set<AnyTweakBindingIdentifier> to use in deinit later on -- but that can wait as an enhancement!
  • Need to update the ViewController.swift in the example project (though I've done that, so that'll be incoming here!)

This looks great! I'll merge it in. Thank you!!

@bryanjclark bryanjclark merged commit 2dcbcd8 into bryanjclark:master Feb 1, 2017
@mac-cain13 mac-cain13 deleted the feature/unbind branch February 2, 2017 06:57
@mac-cain13
Copy link
Contributor Author

Don't worry, I know how hard it is to make time to review PRs on Open Source projects. Thanks for merging it in. :)

Agree on your BindingIdentifier comment, that would be a nice improvement!

@DanielAsher
Copy link

DanielAsher commented Feb 14, 2017

Thanks for this!

In case anyone uses RxSwift, here is an extension to Tweak that allows integration with Observables

import RxSwift

extension Tweak {
    public var rx : Observable<T> {
        return Observable<T>.create { observer in
            
            let binding = YourTweaks.bind(self, binding: { (value: T) -> Void in
                observer.onNext(value)                
            })
            return Disposables.create {
                YourTweaks.unbind(identifier: binding)
            }
        }
    }
}

This can be used so:

       Observable.combineLatest(YourTweaks.textViewFontscale.rx, YourTweaks.textViewLineHeightMultiple.rx) { ($0, $1) }
            .subscribe(onNext: { [weak self] fontScale, lineHeightMultiple in
                guard let this = self else { return }
                let fontSize = this.originalFontSize * fontScale
                let font = UIFont.getCustomFont(size: fontSize)
                this.textView.font = font
                // Uses BonMot attributed string API
                this.baseStyle = StringStyle(.lineHeightMultiple(lineHeightMultiple), .adapt(.control), .font(font))
                let fullStyle = this.baseStyle.byAdding(stringStyle: this.focusStyle)
                this.textView.attributedText = presenter.text.styled(with: fullStyle) 
            })
            .addDisposableTo(disposeBag)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants