From f8a8f9207028a6fb5a35483c040821f22daf755e Mon Sep 17 00:00:00 2001 From: Aditya-Srivastav <54016427+4D17Y4@users.noreply.github.com> Date: Mon, 19 Apr 2021 19:49:27 +0530 Subject: [PATCH] Fixes #4329 "Back button in edit categories triggers back of media details." (#4346) * Back button on fragment * Handled back events * minor changes * removed extra lines --- .../commons/bookmarks/BookmarkFragment.java | 7 +++- .../bookmarks/BookmarkListRootFragment.java | 8 +++- .../category/CategoryDetailsActivity.java | 7 +++- .../contributions/ContributionsFragment.java | 4 ++ .../nrw/commons/explore/ExploreFragment.java | 11 ++++- .../explore/ExploreListRootFragment.java | 12 +++++- .../nrw/commons/explore/SearchActivity.java | 6 +++ .../WikidataItemDetailsActivity.java | 7 +++- .../commons/media/MediaDetailFragment.java | 20 +++++++++ .../media/MediaDetailPagerFragment.java | 41 +++++++++++++++++++ 10 files changed, 115 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/BookmarkFragment.java b/app/src/main/java/fr/free/nrw/commons/bookmarks/BookmarkFragment.java index ab8a76d07..c3b2604ec 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/BookmarkFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/BookmarkFragment.java @@ -100,8 +100,11 @@ public class BookmarkFragment extends CommonsDaggerSupportFragment { public void onBackPressed() { - ((BookmarkListRootFragment) (adapter.getItem(tabLayout.getSelectedTabPosition()))) - .backPressed(); + if(((BookmarkListRootFragment)(adapter.getItem(tabLayout.getSelectedTabPosition()))).backPressed()) { + // The event is handled internally by the adapter , no further action required. + return; + } + // Event is not handled by the adapter ( performed back action ) change action bar. ((BaseActivity)getActivity()).getSupportActionBar().setDisplayHomeAsUpEnabled(false); } } diff --git a/app/src/main/java/fr/free/nrw/commons/bookmarks/BookmarkListRootFragment.java b/app/src/main/java/fr/free/nrw/commons/bookmarks/BookmarkListRootFragment.java index 993029c41..6fafe445e 100644 --- a/app/src/main/java/fr/free/nrw/commons/bookmarks/BookmarkListRootFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/bookmarks/BookmarkListRootFragment.java @@ -180,10 +180,14 @@ public class BookmarkListRootFragment extends CommonsDaggerSupportFragment imple } } - public void backPressed() { + public boolean backPressed() { //check mediaDetailPage fragment is not null then we check mediaDetail.is Visible or not to avoid NullPointerException if(mediaDetails!=null) { if (mediaDetails.isVisible()) { + if(mediaDetails.backButtonClicked()) { + // mediaDetails handled the back clicked , no further action required. + return true; + } // todo add get list fragment ((BookmarkFragment) getParentFragment()).setupTabLayout(); ArrayList removed=mediaDetails.getRemovedItems(); @@ -206,6 +210,8 @@ public class BookmarkListRootFragment extends CommonsDaggerSupportFragment imple } else { moveToContributionsFragment(); } + // notify mediaDetails did not handled the backPressed further actions required. + return false; } void moveToContributionsFragment(){ diff --git a/app/src/main/java/fr/free/nrw/commons/category/CategoryDetailsActivity.java b/app/src/main/java/fr/free/nrw/commons/category/CategoryDetailsActivity.java index 749221547..858951eb3 100644 --- a/app/src/main/java/fr/free/nrw/commons/category/CategoryDetailsActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/category/CategoryDetailsActivity.java @@ -203,7 +203,12 @@ public class CategoryDetailsActivity extends BaseActivity @Override public void onBackPressed() { if (supportFragmentManager.getBackStackEntryCount() == 1){ - // back to search so show search toolbar and hide navigation toolbar + + // the back press is handled by the mediaDetails , no further action required. + if(mediaDetails.backButtonClicked()){ + return; + } + tabLayout.setVisibility(View.VISIBLE); viewPager.setVisibility(View.VISIBLE); mediaContainer.setVisibility(View.GONE); diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java index f766348ec..e2446b6df 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionsFragment.java @@ -594,6 +594,10 @@ public class ContributionsFragment public void backButtonClicked() { if (mediaDetailPagerFragment.isVisible()) { + if(mediaDetailPagerFragment.backButtonClicked()) { + // MediaDetailed handled the backPressed no further action required. + return; + } if (store.getBoolean("displayNearbyCardView", true)) { if (nearbyNotificationCardView.cardViewVisibilityState == NearbyNotificationCardView.CardViewVisibilityState.READY) { nearbyNotificationCardView.setVisibility(View.VISIBLE); diff --git a/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java index 703a5082c..4fd27eb5a 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java @@ -97,10 +97,17 @@ public class ExploreFragment extends CommonsDaggerSupportFragment { public void onBackPressed() { if (tabLayout.getSelectedTabPosition() == 0) { - featuredRootFragment.backPressed(); + if(featuredRootFragment.backPressed()){ + // Event is handled by the Fragment we need not do anything. + return; + } } else { - mobileRootFragment.backPressed(); + if(mobileRootFragment.backPressed()){ + // Event is handled by the Fragment we need not do anything. + return; + } } + // Event is not handled by the fragment ( i.e performed back action ) therefore change action bar. ((BaseActivity)getActivity()).getSupportActionBar().setDisplayHomeAsUpEnabled(false); } diff --git a/app/src/main/java/fr/free/nrw/commons/explore/ExploreListRootFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/ExploreListRootFragment.java index ca74ec363..9cefe39cb 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/ExploreListRootFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/ExploreListRootFragment.java @@ -164,9 +164,18 @@ public class ExploreListRootFragment extends CommonsDaggerSupportFragment implem } } - public void backPressed() { + /** + * Performs back pressed action on the fragment. + * Return true if the event was handled by the mediaDetails otherwise returns false. + * @return + */ + public boolean backPressed() { if (null!=mediaDetails && mediaDetails.isVisible()) { // todo add get list fragment + if(mediaDetails.backButtonClicked()) { + // MediaDetails handled the event no further action required. + return true; + } ((ExploreFragment)getParentFragment()).tabLayout.setVisibility(View.VISIBLE); removeFragment(mediaDetails); ((ExploreFragment) getParentFragment()).setScroll(true); @@ -175,5 +184,6 @@ public class ExploreListRootFragment extends CommonsDaggerSupportFragment implem ((MainActivity) getActivity()).setSelectedItemId(NavTab.CONTRIBUTIONS.code()); } ((MainActivity)getActivity()).showTabs(); + return false; } } diff --git a/app/src/main/java/fr/free/nrw/commons/explore/SearchActivity.java b/app/src/main/java/fr/free/nrw/commons/explore/SearchActivity.java index bc961a521..239d9a52d 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/SearchActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/SearchActivity.java @@ -245,6 +245,12 @@ public class SearchActivity extends BaseActivity @Override public void onBackPressed() { if (getSupportFragmentManager().getBackStackEntryCount() == 1){ + + // the back press is handled by the mediaDetails , no further action required. + if(mediaDetails.backButtonClicked()){ + return; + } + // back to search so show search toolbar and hide navigation toolbar searchView.setVisibility(View.VISIBLE);//set the searchview tabLayout.setVisibility(View.VISIBLE); diff --git a/app/src/main/java/fr/free/nrw/commons/explore/depictions/WikidataItemDetailsActivity.java b/app/src/main/java/fr/free/nrw/commons/explore/depictions/WikidataItemDetailsActivity.java index f36e0f01c..bccc1d836 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/depictions/WikidataItemDetailsActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/depictions/WikidataItemDetailsActivity.java @@ -163,7 +163,12 @@ public class WikidataItemDetailsActivity extends BaseActivity implements MediaDe @Override public void onBackPressed() { if (supportFragmentManager.getBackStackEntryCount() == 1){ - // back to search so show search toolbar and hide navigation toolbar + + // back pressed is handled by the mediaDetails , no further action required. + if(mediaDetailPagerFragment.backButtonClicked()){ + return; + } + tabLayout.setVisibility(View.VISIBLE); viewPager.setVisibility(View.VISIBLE); mediaContainer.setVisibility(View.GONE); diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java index f1f89d850..a8625cd6c 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java @@ -16,8 +16,11 @@ import android.os.Bundle; import android.text.Editable; import android.text.TextUtils; import android.text.TextWatcher; +import android.util.Log; +import android.view.KeyEvent; import android.view.LayoutInflater; import android.view.View; +import android.view.View.OnKeyListener; import android.view.ViewGroup; import android.view.ViewTreeObserver; import android.view.ViewTreeObserver.OnGlobalLayoutListener; @@ -298,6 +301,7 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment implements updateAspectRatio(scrollView.getWidth()); } }); + return view; } @@ -702,6 +706,22 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment implements displayHideCategorySearch(); } + /** + * Hides the categoryEditContainer. + * returns true after closing the categoryEditContainer if open, implying that event was handled. + * else returns false + * @return + */ + public boolean hideCategoryEditContainerIfOpen(){ + if (dummyCategoryEditContainer.getVisibility() == VISIBLE) { + // editCategory is open, close it and return true as the event was handled. + dummyCategoryEditContainer.setVisibility(GONE); + return true; + } + // Event was not handled. + return false; + } + public void displayHideCategorySearch() { if (dummyCategoryEditContainer.getVisibility() != VISIBLE) { dummyCategoryEditContainer.setVisibility(VISIBLE); diff --git a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java index 21dbbafcc..68605e075 100644 --- a/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java @@ -13,6 +13,7 @@ import android.view.MenuInflater; import android.view.MenuItem; import android.view.View; import android.view.ViewGroup; +import androidx.annotation.NonNull; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentManager; import androidx.fragment.app.FragmentStatePagerAdapter; @@ -381,6 +382,16 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple } } + /** + * backButtonClicked is called on a back event in the media details pager. + * returns true after closing the categoryEditContainer if open, implying that event was handled. + * else returns false + * @return + */ + public boolean backButtonClicked(){ + return ((MediaDetailFragment)(adapter.getCurrentFragment())).hideCategoryEditContainerIfOpen(); + } + public interface MediaDetailProvider { Media getMediaAtPosition(int i); @@ -392,6 +403,11 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple //FragmentStatePagerAdapter allows user to swipe across collection of images (no. of images undetermined) private class MediaDetailAdapter extends FragmentStatePagerAdapter { + /** + * Keeps track of the current displayed fragment. + */ + private Fragment mCurrentFragment; + public MediaDetailAdapter(FragmentManager fm) { super(fm); } @@ -421,5 +437,30 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple } return provider.getTotalMediaCount(); } + + /** + * Get the currently displayed fragment. + * @return + */ + public Fragment getCurrentFragment() { + return mCurrentFragment; + } + + /** + * Called to inform the adapter of which item is currently considered to be the "primary", + * that is the one show to the user as the current page. + * @param container + * @param position + * @param object + */ + @Override + public void setPrimaryItem(@NonNull final ViewGroup container, final int position, + @NonNull final Object object) { + // Update the current fragment if changed + if(getCurrentFragment() != object) { + mCurrentFragment = ((Fragment)object); + } + super.setPrimaryItem(container, position, object); + } } }