Fix ViewModels to not depend on View #1633
Assignees
Labels
Z-ibt
Temporary label for Ben to keep track of issues he's triaged.
Comments
miaboloix
changed the title
|
@miaboloix Can you add more details, references, labels to this issue so that we can assign it to new contributors? |
|
Assigning this to @alokbharti . Feel free to get back to us if you get stuck |
|
Sure, thanks. |
8 tasks
This was referenced Oct 11, 2020
rt4914
pushed a commit
that referenced
this issue
Oct 19, 2020
…del (#1981) * Removed textview from RevisionCardViewmodel - Removed textview reference from RevisionCardViewModel - Removed explanationLiveData value from RevisionCardViewModel as the function that is used to get the charsequence was using view, now the code for the same is shifted to RevisionCardFragmentPresenter. So now RevisionCardViewModel expose RevisionCardLiveData and presenter observe it and then uses the HTML factory to get the char sequence and then update the textview - Removed the usage of explanation livedata from revision_card_fragment.xml * renamed setSubtopicIdAndBinding function and removed extra space & simplified view.text assignment * Used named arguments 'imageCenterAlign' to have compiler verification Co-authored-by: Ben Henning <[email protected]> Co-authored-by: Ben Henning <[email protected]>
rt4914
pushed a commit
that referenced
this issue
Oct 19, 2020
…2000) * Removed view dependency from ConceptCardViewModel - ConceptCardViewModel used textview reference to update the explanation text but it also has ConceptCardLiveData that can be used to observe in ConceptCardFragmentPresenter and then explanation text can be updated. - Removed the usage of textview from ConceptCardViewModel - Removed explanationLiveData from ConceptCarViewmodel and its usage in coreesponding xml files. Now the text for this textview is updated in presenter * updated 'setSkillId' function documentation and small space fix * Removed Kdoc for setSkillId function and small renaming argument
BenHenning
pushed a commit
that referenced
this issue
Nov 24, 2020
…1925) * Removed view dependency from ProfileEditViewModel - Used MutableLiveData to store the value of ProfileEditActivity Title and then expose it using a livedata variable in profleEditViewModel class - Observe the livedata variable to set ProfileEditActivity Title in ProfileEditActivityPresenter class * Resolved swith dependency from ProfileViewModel - Added a livedata variable to expose the checked state of switch in ProfileViewModel through which ProfileEditActivityPresenter can observe and update the UI accordingly - Removed switch variable initialization from viewmodel - Added activity dependeny as it was before * Added new lines as required * Removed Switch & activity from ProfileViewModel - Removed activity variable from ProfileViewModel, as we can simply use existing profile livedata variable to update the title of the ProfileEditActivity in the ProfileEditActivityPresenter - Instead of using viewModelProvider in ProfileEditActivityPresenter to get viewmodel, now it is directly injected - Since viewmodel can survive configuration changes, there is no need to create an extra variable to store profile name to use it in onRestoreInstanceState - Added comments for public variable in ProfileEditViewModel * Removed unused import * Removed @ignore from activity title related testcases from ProfileEditActivityTest and modified description for the public variable in ProfielEditViewModel * Resolved tests that are failing in roboelectric except that uses scrollTo() method * FirebaseApp initializtion was giving an IO exception and since it is not used in activity, removed it from the test * Removed unused imports * added scrollview in port layout of profile_edit_activity to resolve roboelectric tests * Removed unused imports * Added feedback changes - Removed unneccessary use of testCoroutineDispatchers.runCurrent() - Renamed editViewModel to profileEditViewModel - Few Indentation * modified indentation * Added PR requested changes - Removed '// ktlint-disable max-line-length' by following suggested convention - Renamed _isAllowedDownloadAccess to isAllowedDownloadAccessMutableLiveData * Added feedback changes, renaming tests * Used launch instead of ActivityScenario.launch * Added named argument * Removed 0dp margin and padding from scrollview and renamed id for linearlayout
|
As all the required 3 ViewModels are done, closing this issue. |
BenHenning
added
the
Z-ibt
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ViewModels to fix:
Before building app module with Bazel, the viewModels mentioned depended on bindings (for example, ConceptCardViewModel depended on ConceptCardFragmentBinding). However, the way app module needs to be split up to build with Bazel requires that viewModels do not depend on bindings - this is because the layout files need to be built after the viewModels. At the moment, we have circumvented this issue by instead having the fragment presenters use the binding to create a view that is then passed into the viewModels. So, the viewModels above depend on views instead of bindings. [ All of the changes mentioned were done in PR #1581 ]
However, having viewModels depend on views can present memory leaks and so view models shouldn't reference views directly. In this particular instance, leaks are prevented because @FragmentScope incorrectly recreates the view model.
The text was updated successfully, but these errors were encountered: