infinitered / redpotion

We believe iPhone development should be clean, scalable, and fast with a language that developers not only enjoy, but actively choose. With the advent of Ruby for iPhone development the RubyMotion community has combined and tested the most active and powerful gems into a single package called RedPotion
MIT License
233 stars 40 forks source link

Extend `remote_image=` to take a closure #168

Closed willem-h closed 7 years ago

willem-h commented 8 years ago

Previously it was hard to tell when an image had finished loading from a URL. Having the ability to set remote_image with a closure that will execute when an image finishes loading makes this super easy. Being able to do this is useful when displaying spinners as they can now be removed easily at the appropriate time.

afestein commented 7 years ago

ping @willem-h

jamonholmgren commented 7 years ago

There's a Travis error on this, for some reason. @willem-h @twerth

.........................................................................................................F.........
Finished in 16.843625 seconds.
Bacon::Error: not #<UIImage:0x112e70f80>.==(#<UIImage:0x1117bc420>) failed
    spec.rb:773:in `satisfy:': RubyMotionQuery styler: UIImageView - should set a remote image with a NSURL instance
    spec.rb:787:in `method_missing:'
    ui_image_view_spec.rb:68:in `<main>'
    spec.rb:463:in `execute_block'
    spec.rb:425:in `run_postponed_block:'
127 tests, 175 assertions, 1 failures, 0 errors
stty: stdin isn't a terminal
The command "bundle exec rake spec output=test_unit" exited with 1.
cache.2
store build cache
0.02s
4.55schange detected (content changed, file is created, or file is deleted):
/Users/travis/build/infinitered/redpotion/vendor/Pods/.build/libPods.a
/Users/travis/build/infinitered/redpotion/vendor/Pods/.build/libSDWebImage.a
/Users/travis/build/infinitered/redpotion/vendor/Pods/Pods.xcodeproj/xcuserdata/travis.xcuserdatad/xcschemes/Pods.xcscheme
/Users/travis/build/infinitered/redpotion/vendor/Pods/Pods.xcodeproj/xcuserdata/travis.xcuserdatad/xcschemes/SDWebImage.xcscheme
/Users/travis/build/infinitered/redpotion/vendor/Pods/build-iPhoneSimulator/libSDWebImage.a
changes detected, packing new archive
uploading archive
Done. Your build exited with 1.
/Users/travis/build.sh: line 148: shell_session_update: command not found
willem-h commented 7 years ago

Is there a way I can run those Travis tests locally @jamonholmgren? I'm having trouble working out what's causing it to break just based off that error log.

afestein commented 7 years ago

Hey @willem-h I was able to clone the repo and run the specs locally without error. Have you tried restarting the Travis test?

afestein commented 7 years ago

Actually @jamonholmgren this test failure looks to be unrelated, since I can see the same Travis failure back in March in #161, and the tests pass locally for me. It looks like something that will need to be addressed in a separate PR. Any thoughts?

afestein commented 7 years ago

Hey @jamonholmgren @twerth do you have any suggestions on where to go with this?

andrewhavens commented 7 years ago

Also, how do you feel about changing this from remote_image({ url: 'example' }) to remote_image = { url: example }. It feels a little strange to use, what would be considered, a "getter" for setting a callback.

willem-h commented 7 years ago

@andrewhavens

Also, how do you feel about changing this from remote_image({ url: 'example' }) to remote_image = { url: example }. It feels a little strange to use, what would be considered, a "getter" for setting a callback.

I don't feel good about doing that because that will be a breaking change to the interface of the gem e.g. remote_image= url. Anyone who uses this gem will need to update their code if this is changed.

andrewhavens commented 7 years ago

I'll work on getting the Travis build passing again. Looks like we just need to make a few config changes.

willem-h commented 7 years ago

Thanks for the feedback @andrewhavens

afestein commented 7 years ago

Thanks for jumping in @andrewhavens. How do we go about getting this merged?

andrewhavens commented 7 years ago

@afestein @willem-h I've been working on getting the test suite back to green, but have been blocked a little bit by the recent Travis downtime on their Mac infrastructure. I narrowed down the issues with the suite. The first issue was that the Travis configuration had simply gotten old and unsupported. The second issue actually has more to do with SDWebImage releasing a new major version. I'm working on fixing that too. Now that I know this, I don't think there is any need to hold up this PR from getting merged. I'll follow up with a new PR which fixes the Travis build and adds support for SDWebImage 4.0.

afestein commented 7 years ago

Fantastic 🎉