misteu / VocabularyTraining

Simple vocabulary Trainer written in Swift
Apache License 2.0
21 stars 15 forks source link

[Refactor] Simple Coordinator pattern #24

Closed misteu closed 1 year ago

misteu commented 1 year ago

GOAL

UINavigationController should be used for hosting all the view controllers and managing navigation. Where it makes sense, presenting/dismissing might be replaced with pushing/popping on a navigation stack.

Besides that it makes sense to implement a simple coordinator pattern as @PranjalDev mentioned.

WHY

Currently there are back buttons implemented as UIBarButtonItem look-a-likes. This adds a lot of unnecessary code and makes it easy to miss something whenever there is a change.

ACCEPTANCE CRITERIA

PranjalDev commented 1 year ago

I would love to pick this up,

Also, As we are talking about navigation here, we could also introduce light weight coordinator pattern to handle the navigation and the dependency injection between the view controllers,

Let me know if you are open for that coordinator change, if not will move forward with the MainNavigtor with enum approach

misteu commented 1 year ago

Hey!

actually this is a great idea! Having a lightweight coordinator pattern here would be really nice.

Would be happy if you want to work on this 👍

Not sure though if we should already start with it or rather wait for all of the storyboard screens being pure code.

I will already update the issue a little bit and assign it to you.

PranjalDev commented 1 year ago

What we could do is develop this in phased manner like

misteu commented 1 year ago

Sounds good 👌 Let's start with one and have separate tickets for the other ones.

I can take over getting rid of the remaining Storyboard or do you plan on including it into this first coordinator use case?

Also @nayushi wanted to work on redesigning the app (looks really great I can say so far 🔥 She already created some Figma designs.

Because of these mentioned upcoming redesigns it definitely makes sense to focus on the coordinator part in this ticket and not so much on refactoring view controllers for now I think. Otherwise we might touch stuff again and again.

Let me know what you think and if this makes sense to you :)

PranjalDev commented 1 year ago

Hey @misteu One of my peer wanted to contribute to this project, specifically to this part, and he already have a PR ready, He would be replying to this, Issue thread, Can you assign it to him?

NukeNalin commented 1 year ago

Hey @misteu Can you assign me this issue?

misteu commented 1 year ago

sure! I assigned you @NukeNalin