Skip to content

Conversation

@AnEnigmaticBug
Copy link
Contributor

Fixes #1073

Changes: Earlier, there were two ViewModels being used in WelcomeFragment.

In MVVM, a View is associated with only one ViewModel. To avoid duplication of logic, all distinct logic is split into smaller classes which can then be reused in different ViewModels.

So now, WelcomeFragment has only one ViewModel(with two flavors: fdroid and playstore). Also, made a separate package for the welcome screen since its also a screen like others.

Screenshots for the change:
This is purely a refactoring. So, UI hasn't been changed in any way.

Since the welcome screen will be having at least a Fragment and a
ViewModel, it makes sense to place them in their own package.
@AnEnigmaticBug AnEnigmaticBug changed the title Iss1073 feat: refactor the two ViewModels in the welcome screen into one ViewModel Feb 12, 2019
@iamareebjamal
Copy link
Member

Good work but now we have split two components. 1- Location component and 2- Welcome ViewModel

I'm going to merge but this is considered boilerplate

@iamareebjamal iamareebjamal changed the title feat: refactor the two ViewModels in the welcome screen into one ViewModel refactor: two ViewModels in the welcome screen into one ViewModel Feb 12, 2019
@iamareebjamal iamareebjamal merged commit 94307a1 into fossasia:development Feb 12, 2019
@AnEnigmaticBug
Copy link
Contributor Author

@iamareebjamal I'm trying to rectify this exact issue. Can you please read my new comment on #1082?

angmas1 pushed a commit to angmas1/open-event-android that referenced this pull request Feb 13, 2019
…ssasia#1083)

Changes: Earlier, there were two `ViewModel`s being used in `WelcomeFragment`.

In MVVM, a `View` is associated with only one `ViewModel`. To avoid duplication of logic, all distinct logic is split into smaller classes which can then be reused in different `ViewModel`s.

So now, `WelcomeFragment` has only one `ViewModel`(with two flavors: fdroid and playstore). Also, made a separate package for the welcome screen since its also a screen like others.

Fixes fossasia#1073
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.

2 participants