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

Refresh live streams on project page appearing #80

Merged
merged 3 commits into from
Feb 8, 2017

Conversation

justinswart
Copy link
Contributor

This is a quick fix for refreshing a project's live streams when the project page appears.

Alternatively we could be more specific about only doing this when dismissing a live stream/replay but it would require more delegate calls etc due to the nested nature of the project page and the fact that live streams are launched on ProjectPamphletContentViewController and fetched in ProjectPamphletViewModel.

Perhaps worth doing that though?

Copy link
Contributor

@mbrandonw mbrandonw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! just one lil comment, take it or leave it!

let liveStreamEventsFetch = Signal.merge(
project.take(first: 1),
projectWhenViewAppears
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice nice! i think you could even just inline this without the intermediate signal:

let liveStreamEventsFetch = project
  .take(first: 1)
  .takeWhen(self.viewWillAppearAnimated.signal)
  .switchMap { ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, wouldn't that take(first: 1) cause the signal to complete?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of yer right: https://github.com/kickstarter/Kickstarter-ReactiveExtensions/blob/master/ReactiveExtensionsTests/operators/TakeWhenTests.swift#L130-L134

when the source completes, the whole thing completes. i always forget about that. i kinda think the correct behavior is to not complete, but don't wanna open that can of worms now.

you could still inline with this:

let liveStreamEventsFetch = Signal.combineLatest(
  project.take(first: 1)
  self.viewWillAppearAnimated.signal
  )
  .switchMap { ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right!

@mbrandonw mbrandonw merged commit 6d90057 into master Feb 8, 2017
@mbrandonw mbrandonw deleted the refresh_livestreamevent_on_dismiss branch February 8, 2017 19:48
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

2 participants