devxoul / RxViewController

RxSwift wrapper for UIViewController and NSViewController
MIT License
346 stars 64 forks source link

Add new observables for displayed and dismissed states #4

Closed twittemb closed 7 years ago

twittemb commented 7 years ago

This commit add these new features:

codecov-io commented 7 years ago

Codecov Report

Merging #4 into swift-4.0 will decrease coverage by 0.44%. The diff coverage is 88.88%.

Impacted file tree graph

@@             Coverage Diff              @@
##           swift-4.0      #4      +/-   ##
============================================
- Coverage      92.75%   92.3%   -0.45%     
============================================
  Files              2       2              
  Lines             69      78       +9     
  Branches           6       7       +1     
============================================
+ Hits              64      72       +8     
  Misses             1       1              
- Partials           4       5       +1
Impacted Files Coverage Δ
Sources/RxViewController/UIViewController+Rx.swift 88% <88.88%> (+0.19%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b32c9ae...b240cfb. Read the comment docs.

devxoul commented 7 years ago

Please don't change the indentation :(

twittemb commented 7 years ago

Hi,

I've just pushed a new version where code indentation has not changed.

twittemb commented 7 years ago

Hi. I changed my PR according to your remarks. It is pretty hard to test the difference between the dismissing/dismissed states in a XCTestCase. How would you do that ?

devxoul commented 7 years ago

Thanks! After some thoughts, isDismissed can be confused because it looks like that it is executed after the completion closure in dismissViewController(). I think we would better keep only isVisible and isDismissing.

twittemb commented 7 years ago

Hi, I've just pushed a new version. thx.

devxoul commented 7 years ago

Thanks for your contribution! :tada: