* Bug Fix #2585
* Added null checks on view in SearchImageFragment when updating views from external sources
* Disposed the disposables in SearchActivity and SearchImageFragment when no longer in active lifecycle

* use FragmentUtils to verify fragment active state
This commit is contained in:
Ashish Kumar 2019-03-18 15:43:36 +05:30 committed by neslihanturan
parent 27683a76db
commit 468f0b7274
3 changed files with 74 additions and 50 deletions

View file

@ -12,16 +12,10 @@ import android.text.TextUtils;
import android.view.View; import android.view.View;
import android.widget.FrameLayout; import android.widget.FrameLayout;
import android.widget.SearchView; import android.widget.SearchView;
import com.jakewharton.rxbinding2.view.RxView;
import com.jakewharton.rxbinding2.widget.RxSearchView;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
import butterknife.BindView; import butterknife.BindView;
import butterknife.ButterKnife; import butterknife.ButterKnife;
import com.jakewharton.rxbinding2.view.RxView;
import com.jakewharton.rxbinding2.widget.RxSearchView;
import fr.free.nrw.commons.Media; import fr.free.nrw.commons.Media;
import fr.free.nrw.commons.R; import fr.free.nrw.commons.R;
import fr.free.nrw.commons.explore.categories.SearchCategoryFragment; import fr.free.nrw.commons.explore.categories.SearchCategoryFragment;
@ -29,8 +23,13 @@ import fr.free.nrw.commons.explore.images.SearchImageFragment;
import fr.free.nrw.commons.explore.recentsearches.RecentSearchesFragment; import fr.free.nrw.commons.explore.recentsearches.RecentSearchesFragment;
import fr.free.nrw.commons.media.MediaDetailPagerFragment; import fr.free.nrw.commons.media.MediaDetailPagerFragment;
import fr.free.nrw.commons.theme.NavigationBaseActivity; import fr.free.nrw.commons.theme.NavigationBaseActivity;
import fr.free.nrw.commons.utils.FragmentUtils;
import fr.free.nrw.commons.utils.ViewUtil; import fr.free.nrw.commons.utils.ViewUtil;
import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.android.schedulers.AndroidSchedulers;
import io.reactivex.disposables.Disposable;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
/** /**
* Represents search screen of this app * Represents search screen of this app
@ -52,6 +51,7 @@ public class SearchActivity extends NavigationBaseActivity implements MediaDetai
private MediaDetailPagerFragment mediaDetails; private MediaDetailPagerFragment mediaDetails;
ViewPagerAdapter viewPagerAdapter; ViewPagerAdapter viewPagerAdapter;
private String query; private String query;
private Disposable searchDisposable;
@Override @Override
protected void onCreate(Bundle savedInstanceState) { protected void onCreate(Bundle savedInstanceState) {
@ -98,28 +98,33 @@ public class SearchActivity extends NavigationBaseActivity implements MediaDetai
viewPagerAdapter.setTabData(fragmentList, titleList); viewPagerAdapter.setTabData(fragmentList, titleList);
viewPagerAdapter.notifyDataSetChanged(); viewPagerAdapter.notifyDataSetChanged();
RxSearchView.queryTextChanges(searchView) searchDisposable = RxSearchView.queryTextChanges(searchView)
.takeUntil(RxView.detaches(searchView)) .takeUntil(RxView.detaches(searchView))
.debounce(500, TimeUnit.MILLISECONDS) .debounce(500, TimeUnit.MILLISECONDS)
.observeOn(AndroidSchedulers.mainThread()) .observeOn(AndroidSchedulers.mainThread())
.subscribe( query -> { .subscribe(query -> {
this.query = query.toString(); this.query = query.toString();
//update image list //update image list
if (!TextUtils.isEmpty(query)) { if (!TextUtils.isEmpty(query)) {
viewPager.setVisibility(View.VISIBLE); viewPager.setVisibility(View.VISIBLE);
tabLayout.setVisibility(View.VISIBLE); tabLayout.setVisibility(View.VISIBLE);
searchHistoryContainer.setVisibility(View.GONE); searchHistoryContainer.setVisibility(View.GONE);
searchImageFragment.updateImageList(query.toString()); if (FragmentUtils.isFragmentUIActive(searchImageFragment)) {
searchCategoryFragment.updateCategoryList(query.toString()); searchImageFragment.updateImageList(query.toString());
}else { }
viewPager.setVisibility(View.GONE);
tabLayout.setVisibility(View.GONE); if (FragmentUtils.isFragmentUIActive(searchCategoryFragment)) {
searchHistoryContainer.setVisibility(View.VISIBLE); searchCategoryFragment.updateCategoryList(query.toString());
recentSearchesFragment.updateRecentSearches(); }
// open search history fragment
} } else {
} viewPager.setVisibility(View.GONE);
); tabLayout.setVisibility(View.GONE);
searchHistoryContainer.setVisibility(View.VISIBLE);
recentSearchesFragment.updateRecentSearches();
// open search history fragment
}
});
} }
/** /**
@ -260,4 +265,12 @@ public class SearchActivity extends NavigationBaseActivity implements MediaDetai
searchImageFragment.addImagesToList(query); searchImageFragment.addImagesToList(query);
} }
} }
@Override protected void onDestroy() {
super.onDestroy();
//Dispose the disposables when the activity is destroyed
if (searchDisposable != null) {
searchDisposable.dispose();
}
}
} }

View file

@ -1,6 +1,5 @@
package fr.free.nrw.commons.explore.images; package fr.free.nrw.commons.explore.images;
import android.annotation.SuppressLint; import android.annotation.SuppressLint;
import android.content.res.Configuration; import android.content.res.Configuration;
import android.os.Bundle; import android.os.Bundle;
@ -13,19 +12,9 @@ import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
import android.widget.ProgressBar; import android.widget.ProgressBar;
import android.widget.TextView; import android.widget.TextView;
import com.pedrogomez.renderers.RVRendererAdapter;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.concurrent.TimeUnit;
import javax.inject.Inject;
import javax.inject.Named;
import butterknife.BindView; import butterknife.BindView;
import butterknife.ButterKnife; import butterknife.ButterKnife;
import com.pedrogomez.renderers.RVRendererAdapter;
import fr.free.nrw.commons.Media; import fr.free.nrw.commons.Media;
import fr.free.nrw.commons.R; import fr.free.nrw.commons.R;
import fr.free.nrw.commons.di.CommonsDaggerSupportFragment; import fr.free.nrw.commons.di.CommonsDaggerSupportFragment;
@ -37,7 +26,14 @@ import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient;
import fr.free.nrw.commons.utils.NetworkUtils; import fr.free.nrw.commons.utils.NetworkUtils;
import fr.free.nrw.commons.utils.ViewUtil; import fr.free.nrw.commons.utils.ViewUtil;
import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.android.schedulers.AndroidSchedulers;
import io.reactivex.disposables.Disposable;
import io.reactivex.schedulers.Schedulers; import io.reactivex.schedulers.Schedulers;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.concurrent.TimeUnit;
import javax.inject.Inject;
import javax.inject.Named;
import timber.log.Timber; import timber.log.Timber;
import static android.view.View.GONE; import static android.view.View.GONE;
@ -75,6 +71,7 @@ public class SearchImageFragment extends CommonsDaggerSupportFragment {
((SearchActivity)getContext()).onSearchImageClicked(index); ((SearchActivity)getContext()).onSearchImageClicked(index);
saveQuery(query); saveQuery(query);
}); });
private Disposable searchImagesDisposable;
/** /**
* This method saves Search Query in the Recent Searches Database. * This method saves Search Query in the Recent Searches Database.
@ -140,11 +137,11 @@ public class SearchImageFragment extends CommonsDaggerSupportFragment {
bottomProgressBar.setVisibility(GONE); bottomProgressBar.setVisibility(GONE);
queryList.clear(); queryList.clear();
imagesAdapter.clear(); imagesAdapter.clear();
okHttpJsonApiClient.searchImages(query, queryList.size()) searchImagesDisposable = okHttpJsonApiClient.searchImages(query, queryList.size())
.subscribeOn(Schedulers.io()) .subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread()) .observeOn(AndroidSchedulers.mainThread())
.timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) .timeout(TIMEOUT_SECONDS, TimeUnit.SECONDS)
.subscribe(this::handleSuccess, this::handleError); .subscribe(this::handleSuccess, this::handleError);
} }
@ -230,8 +227,15 @@ public class SearchImageFragment extends CommonsDaggerSupportFragment {
* Handles the UI updates for no internet scenario * Handles the UI updates for no internet scenario
*/ */
private void handleNoInternet() { private void handleNoInternet() {
progressBar.setVisibility(GONE); if (null
ViewUtil.showShortSnackbar(imagesRecyclerView, R.string.no_internet); != getView()) {//We have exposed public methods to update our ui, we will have to add null checks until we make this lifecycle aware
if (null != progressBar) {
progressBar.setVisibility(GONE);
}
ViewUtil.showShortSnackbar(imagesRecyclerView, R.string.no_internet);
} else {
Timber.d("Attempt to update fragment ui after its view was destroyed");
}
} }
/** /**
@ -259,4 +263,11 @@ public class SearchImageFragment extends CommonsDaggerSupportFragment {
return imagesAdapter.getItem(i); return imagesAdapter.getItem(i);
} }
} }
@Override public void onDestroyView() {
super.onDestroyView();
if(searchImagesDisposable!=null) {
searchImagesDisposable.dispose();//Always dispose the disposables when view is no longer there
}
}
} }

View file

@ -15,6 +15,6 @@ public class FragmentUtils {
* @return * @return
*/ */
public static boolean isFragmentUIActive(Fragment fragment) { public static boolean isFragmentUIActive(Fragment fragment) {
return fragment.getActivity() != null && fragment.isAdded() && !fragment.isDetached() && !fragment.isRemoving(); return fragment!=null && fragment.getActivity() != null && fragment.isAdded() && !fragment.isDetached() && !fragment.isRemoving();
} }
} }