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

Fix nav drawer #145

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix nav drawer #145

wants to merge 5 commits into from

Conversation

SangeethaTestPress
Copy link
Contributor

Changes done

  • Added navigation header
    -Added a viewmodel to get the profileDetails

private void setupEasterEgg() {
Menu navigationMenu = navigationView.getMenu();
initializeNavigationHeaderView();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not right place to initializeNavigationHeaderView()


class ProfileDetailsViewModel : ViewModel() {
private var profileDetails = MutableLiveData<ProfileDetails?>()
fun get(serviceProvider: TestpressServiceProvider, activity: Activity): LiveData<ProfileDetails?> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use NetworkBoundResource

Copy link
Contributor

Choose a reason for hiding this comment

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

This is fetching from the network every time.

Comment on lines -36 to -37
android:layout_weight="1"
>
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this change


private void getProfileDetailsFromViewModel() {
if (isUserAuthenticated) {
profileDetailsViewModel.get(serviceProvider, this).observe(this, new Observer<ProfileDetails>() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need to pass serviceProvider. Create repository and inject it in constructor

}

private void setProfileDetailsOnDrawer() {
if (profileDetails == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Resource class to check if get is a success or error

} else {
navigationHeader.setVisibility(View.VISIBLE);
username.setText(profileDetails.getDisplayName());
getBitmapFromString();
Copy link
Contributor

Choose a reason for hiding this comment

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

Change this name

ImageLoader.getInstance().loadImage(profileDetails.getPhoto(), new SimpleImageLoadingListener() {
@Override
public void onLoadingFailed(String imageUri, View view, FailReason failReason) {
hideNavigationHeader();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not hide the navigation header if image fetch is failed. Show placeholder instead.

navigationHeader.setVisibility(View.VISIBLE);
username.setText(profileDetails.getDisplayName());
getBitmapFromString();
setOnClickListener();
Copy link
Contributor

Choose a reason for hiding this comment

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

Call this method in onCreateView

app:strokeWidth="1dp">

<ImageView
android:id="@+id/img_profile"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
android:id="@+id/img_profile"
android:id="@+id/profile_image"

Comment on lines +48 to +52
<View
android:layout_width="match_parent"
android:layout_height="1dp"
android:layout_marginTop="25dp"
android:background="@color/grey_icon" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Use navigation menu's divider

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.

None yet

2 participants