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

Add CompositeStepper #5

Merged
merged 6 commits into from
Jan 26, 2018

Conversation

dangthaison91
Copy link
Contributor

For some situation, I have both VM and ViewController act as Stepper. Navigation will be driven by business logic from VM and presentation logic from ViewController. CompositeStepper will help to combine multiple Steppers.

This PR is WIP:

let createScheduleViewController = CreateScheduleViewController()
let createScheduleViewModel = CreateScheduleViewModel(id: nil)
root.present(navi, animated: true, completion: nil)
return [Flowable(nextPresentable: createScheduleViewController, nextStepper: CompositeStepper(steppers: [createScheduleViewModel, createScheduleViewController]))]

@dangthaison91 dangthaison91 mentioned this pull request Jan 4, 2018
@twittemb twittemb force-pushed the develop branch 2 times, most recently from 80442ef to 88e3466 Compare January 19, 2018 23:48
@twittemb
Copy link
Collaborator

Hi @dangthaison91

Now that the subscriptions are flatten in the Coordinator (branch develop), do you still want to introduce a CompositeStepper ?

If not, I will cancel this PR.

Thx.

@dangthaison91
Copy link
Contributor Author

@twittemb I will test this again by today.

@twittemb
Copy link
Collaborator

Hi @dangthaison91

I am reviewing / testing your work. I will keep you in touch.

Thanks.

@twittemb twittemb self-assigned this Jan 25, 2018
Copy link
Collaborator

@twittemb twittemb left a comment

Choose a reason for hiding this comment

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

I have 3 remarks:

  • please rebase your branch on the current develop because I just made a fix that allows your code to work
  • CompositeStepper is defined 2 times
  • Rename your PR to remove [WIP]

After that we should be good to go

@@ -30,8 +27,36 @@ public class OneStepper: Stepper {
}
}

// A Stepper that combines multiple steppers
Copy link
Collaborator

Choose a reason for hiding this comment

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

CompositeStepper is defined 2 times

@dangthaison91 dangthaison91 changed the title [WIP] Add CompositeStepper Add CompositeStepper Jan 26, 2018
@twittemb
Copy link
Collaborator

Thanks @dangthaison91

@twittemb twittemb merged commit a6a423b into RxSwiftCommunity:develop Jan 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants