Skip to content

Fix #574 - config changes bug with localized resources - No more AndroidViewModels #631

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

Merged
merged 3 commits into from
Feb 18, 2019

Conversation

JoseAlcerreca
Copy link
Contributor

Exposing resolved resources from ViewModels is not a good idea because they survive configuration changes, including locale changes. See #574 and https://issuetracker.google.com/issues/111961971

In this PR we leverage Data Binding to load those resources for us with the nice side-effect of removing the need for AndroidViewModels so no context is available in them. (I'm considering moving AndroidViewModels to the smell category.)

@JoseAlcerreca JoseAlcerreca changed the title Todo mvvm live 574 Feb 7, 2019
return mNoTasksLabel;
}

public LiveData<Drawable> getNoTaskIconRes() {
public MutableLiveData<Integer> getNoTaskIconRes() {
return mNoTaskIconRes;
}

Choose a reason for hiding this comment

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

@JoseAlcerreca Is it really needed to expose these objects as Mutable LiveData?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, they should be MutableLiveDatas only in the ViewModel, exposed as LiveDatas. That's a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants