Add background color option for media detail page (#5394)

* feat: add backgroundColor property on media

* feat: add optional menu items for media backgroundColor

* fix: test pass when running in batch

* refactor: remove backgroundColor from media

* refactor: add string for background color menu

* chore: remove useless change

* feat: change media image background color

* feat: pass backgroundColor to ZoomableActivity

* chore: remove extra space

app/src/main/java/fr/free/nrw/commons/media/MediaDetailFragment.java

@@ -17,6 +17,7 @@ import android.annotation.SuppressLint;
 import android.app.AlertDialog;
 import android.content.Context;
 import android.content.Intent;
+import android.content.SharedPreferences;
 import android.content.res.Configuration;
 import android.graphics.drawable.Animatable;
 import android.net.Uri;
@@ -78,6 +79,7 @@ import fr.free.nrw.commons.di.CommonsDaggerSupportFragment;
 import fr.free.nrw.commons.explore.depictions.WikidataItemDetailsActivity;
 import fr.free.nrw.commons.kvstore.JsonKvStore;
 import fr.free.nrw.commons.location.LocationServiceManager;
+import fr.free.nrw.commons.media.ZoomableActivity.ZoomableActivityConstants;
 import fr.free.nrw.commons.profile.ProfileActivity;
 import fr.free.nrw.commons.settings.Prefs;
 import fr.free.nrw.commons.ui.widget.HtmlTextView;
@@ -111,8 +113,11 @@ import timber.log.Timber;
 public class MediaDetailFragment extends CommonsDaggerSupportFragment implements
     CategoryEditHelper.Callback {
 
-    private static final int REQUEST_CODE = 1001 ;
+    private static final int REQUEST_CODE = 1001;
-    private static final int REQUEST_CODE_EDIT_DESCRIPTION = 1002 ;
+    private static final int REQUEST_CODE_EDIT_DESCRIPTION = 1002;
+    private static final String IMAGE_BACKGROUND_COLOR = "image_background_color";
+    static final int DEFAULT_IMAGE_BACKGROUND_COLOR = 0;
+    
     private boolean editable;
     private boolean isCategoryImage;
     private MediaDetailPagerFragment.MediaDetailProvider detailProvider;
@@ -388,6 +393,15 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment implements
             zoomableIntent.setData(Uri.parse(media.getImageUrl()));
             zoomableIntent.putExtra(
                 ZoomableActivity.ZoomableActivityConstants.ORIGIN, "MediaDetails");
+            
+            int backgroundColor = getImageBackgroundColor();
+            if (backgroundColor != DEFAULT_IMAGE_BACKGROUND_COLOR) {
+                zoomableIntent.putExtra(
+                    ZoomableActivity.ZoomableActivityConstants.PHOTO_BACKGROUND_COLOR,
+                    backgroundColor
+                );
+            }
+            
             ctx.startActivity(
                 zoomableIntent
             );
@@ -599,6 +613,10 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment implements
      * - when the high resolution image is available, it replaces the low resolution image
      */
     private void setupImageView() {
+        int imageBackgroundColor = getImageBackgroundColor();
+        if (imageBackgroundColor != DEFAULT_IMAGE_BACKGROUND_COLOR) {
+            image.setBackgroundColor(imageBackgroundColor);
+        }
 
         image.getHierarchy().setPlaceholderImage(R.drawable.image_placeholder);
         image.getHierarchy().setFailureImage(R.drawable.image_placeholder);
@@ -1472,4 +1490,28 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment implements
     public interface Callback {
         void nominatingForDeletion(int index);
     }
+    
+    /**
+     * Called when the image background color is changed.
+     * You should pass a useable color, not a resource id.
+     * @param color
+     */
+    public void onImageBackgroundChanged(int color) {
+        int currentColor = getImageBackgroundColor();
+        if (currentColor == color) {
+            return;
+        }
+
+        image.setBackgroundColor(color);
+        getImageBackgroundColorPref().edit().putInt(IMAGE_BACKGROUND_COLOR, color).apply();
+    }
+
+    private SharedPreferences getImageBackgroundColorPref() {
+        return getContext().getSharedPreferences(IMAGE_BACKGROUND_COLOR + media.getPageId(), Context.MODE_PRIVATE);
+    }
+
+    private int getImageBackgroundColor() {
+        SharedPreferences imageBackgroundColorPref = this.getImageBackgroundColorPref();
+        return imageBackgroundColorPref.getInt(IMAGE_BACKGROUND_COLOR, DEFAULT_IMAGE_BACKGROUND_COLOR);
+    }
 }

app/src/main/java/fr/free/nrw/commons/media/MediaDetailPagerFragment.java

@@ -5,6 +5,8 @@ import static fr.free.nrw.commons.Utils.handleWebUrl;
 import android.annotation.SuppressLint;
 import android.content.ActivityNotFoundException;
 import android.content.Intent;
+import android.graphics.Bitmap;
+import android.graphics.BitmapFactory;
 import android.net.Uri;
 import android.os.Bundle;
 import android.view.LayoutInflater;
@@ -18,6 +20,7 @@ import androidx.appcompat.app.ActionBar;
 import androidx.appcompat.app.AlertDialog;
 import androidx.appcompat.app.AppCompatActivity;
 import androidx.annotation.NonNull;
+import androidx.core.content.ContextCompat;
 import androidx.fragment.app.Fragment;
 import androidx.fragment.app.FragmentManager;
 import androidx.fragment.app.FragmentStatePagerAdapter;
@@ -41,9 +44,16 @@ import fr.free.nrw.commons.utils.DownloadUtils;
 import fr.free.nrw.commons.utils.ImageUtils;
 import fr.free.nrw.commons.utils.NetworkUtils;
 import fr.free.nrw.commons.utils.ViewUtil;
+import io.reactivex.Observable;
+import io.reactivex.android.schedulers.AndroidSchedulers;
 import io.reactivex.disposables.CompositeDisposable;
+import io.reactivex.schedulers.Schedulers;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLConnection;
 import java.util.ArrayList;
 import java.util.Objects;
+import java.util.concurrent.Callable;
 import javax.inject.Inject;
 import timber.log.Timber;
 
@@ -190,6 +200,7 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple
         }
 
         Media m = provider.getMediaAtPosition(pager.getCurrentItem());
+        MediaDetailFragment mediaDetailFragment = this.adapter.getCurrentMediaDetailFragment();
         switch (item.getItemId()) {
             case R.id.menu_bookmark_current_image:
                 boolean bookmarkExists = bookmarkDao.updateBookmark(bookmark);
@@ -243,6 +254,16 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple
                 return true;
             case R.id.menu_view_report:
                 showReportDialog(m);
+            case R.id.menu_view_set_white_background:
+                if (mediaDetailFragment != null) {
+                    mediaDetailFragment.onImageBackgroundChanged(ContextCompat.getColor(getContext(), R.color.white));
+                }
+                return true;
+            case R.id.menu_view_set_black_background:
+                if (mediaDetailFragment != null) {
+                    mediaDetailFragment.onImageBackgroundChanged(ContextCompat.getColor(getContext(), R.color.black));
+                }
+                return true;
             default:
                 return super.onOptionsItemSelected(item);
         }
@@ -372,6 +393,17 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple
                     if (m.getUser() != null) {
                         menu.findItem(R.id.menu_view_user_page).setEnabled(true).setVisible(true);
                     }
+
+                    try {
+                        URL mediaUrl = new URL(m.getImageUrl());
+                        this.handleBackgroundColorMenuItems(
+                            () -> BitmapFactory.decodeStream(mediaUrl.openConnection().getInputStream()),
+                            menu
+                        );
+                    } catch (Exception e) {
+                        Timber.e("Cant detect media transparency");
+                    }
+                    
                     // Initialize bookmark object
                     bookmark = new Bookmark(
                             m.getFilename(),
@@ -422,6 +454,25 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple
         }
     }
 
+    /**
+     * Decide wether or not we should display the background color menu items
+     * We display them if the image is transparent
+     * @param getBitmap
+     * @param menu
+     */
+    private void handleBackgroundColorMenuItems(Callable<Bitmap> getBitmap, Menu menu) {
+        Observable.fromCallable(
+                getBitmap
+            ).subscribeOn(Schedulers.newThread())
+            .observeOn(AndroidSchedulers.mainThread())
+            .subscribe(image -> {
+                if (image.hasAlpha()) {
+                    menu.findItem(R.id.menu_view_set_white_background).setVisible(true).setEnabled(true);
+                    menu.findItem(R.id.menu_view_set_black_background).setVisible(true).setEnabled(true);
+                }
+            });
+    }
+
     private void updateBookmarkState(MenuItem item) {
         boolean isBookmarked = bookmarkDao.findBookmark(bookmark);
         if(isBookmarked) {
@@ -567,6 +618,18 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple
             return mCurrentFragment;
         }
 
+        /**
+         * If current fragment is of type MediaDetailFragment, return it, otherwise return null.
+         * @return MediaDetailFragment
+         */
+        public MediaDetailFragment getCurrentMediaDetailFragment() {
+            if (mCurrentFragment instanceof MediaDetailFragment) {
+                return (MediaDetailFragment) mCurrentFragment;
+            }
+
+            return null;
+        }
+
         /**
          * 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.

app/src/main/java/fr/free/nrw/commons/media/ZoomableActivity.kt

@@ -13,6 +13,7 @@ import android.widget.Button
 import android.widget.ProgressBar
 import android.widget.TextView
 import android.widget.Toast
+import androidx.core.content.ContextCompat
 import androidx.lifecycle.ViewModelProvider
 import butterknife.BindView
 import butterknife.ButterKnife
@@ -70,6 +71,8 @@ class ZoomableActivity : BaseActivity() {
     @BindView(R.id.zoomable)
     var photo: ZoomableDraweeView? = null
     
+    var photoBackgroundColor: Int? = null
+
     @JvmField
     @BindView(R.id.zoom_progress_bar)
     var spinner: ProgressBar? = null
@@ -180,6 +183,13 @@ class ZoomableActivity : BaseActivity() {
                 ).apply()
             }
         }
+        
+        val backgroundColor = intent.getIntExtra(ZoomableActivityConstants.PHOTO_BACKGROUND_COLOR,
+                MediaDetailFragment.DEFAULT_IMAGE_BACKGROUND_COLOR);
+
+        if (backgroundColor != MediaDetailFragment.DEFAULT_IMAGE_BACKGROUND_COLOR) {
+            photoBackgroundColor = backgroundColor
+        }
     }
 
     /**
@@ -196,7 +206,7 @@ class ZoomableActivity : BaseActivity() {
     /**
      * Handle view model result.
      */
-    private fun handleResult(result: Result){
+    private fun handleResult(result: Result) {
         if(result.status is CallbackStatus.SUCCESS){
             val images = result.images
             if(images.isNotEmpty()) {
@@ -609,6 +619,10 @@ class ZoomableActivity : BaseActivity() {
                 .build()
             photo!!.controller = controller
             
+            if (photoBackgroundColor != null) {
+                photo!!.setBackgroundColor(photoBackgroundColor!!)
+            }
+
             if (!images.isNullOrEmpty()) {
                 val selectedIndex = getImagePosition(selectedImages, images!![position])
                 val isSelected = selectedIndex != -1
@@ -667,5 +681,7 @@ class ZoomableActivity : BaseActivity() {
          * the custom picker.
          */
         const val ORIGIN = "Origin";
+        
+        const val PHOTO_BACKGROUND_COLOR = "photo_background_color"
     }
 }

app/src/main/res/menu/fragment_image_detail.xml

@@ -38,5 +38,25 @@
       android:id="@+id/menu_view_report"
       android:title="@string/menu_view_report"
       app:showAsAction="never" />
+<!-- 
+Setting visible and enabled to false as default because 
+we only want to show it in a very specific case 
+-->
+    <item
+      android:id="@+id/menu_view_set_white_background"
+      android:title="@string/menu_view_set_white_background"
+      android:visible="false"
+      android:enabled="false"
+      app:showAsAction="never" />
+<!-- 
+Setting visible and enabled to false as default because 
+we only want to show it in a very specific case 
+-->
+    <item
+      android:id="@+id/menu_view_set_black_background"
+      android:title="@string/menu_view_set_black_background"
+      android:visible="false"
+      android:enabled="false"
+      app:showAsAction="never" />
 
 </menu>

app/src/main/res/values/strings.xml

@@ -770,6 +770,8 @@ Upload your first media by tapping on the add button.</string>
   <string name="image_selected">Image selected</string>
   <string name="image_marked_as_not_for_upload">Image marked as not for upload</string>
   <string name="menu_view_report">Report</string>
+  <string name="menu_view_set_white_background">Set white background</string>
+  <string name="menu_view_set_black_background">Set black background</string>
   <string name="report_violation">Report violation</string>
   <string name="report_user">Report this user</string>
   <string name="report_content">Report this content</string>

app/src/test/kotlin/fr/free/nrw/commons/media/MediaDetailFragmentUnitTests.kt

@@ -3,6 +3,7 @@ package fr.free.nrw.commons.media
 import android.app.Activity
 import android.content.Context
 import android.content.Intent
+import android.content.SharedPreferences
 import android.content.res.Configuration
 import android.os.Bundle
 import android.os.Looper
@@ -20,6 +21,7 @@ import com.facebook.drawee.generic.GenericDraweeHierarchy
 import com.facebook.drawee.view.SimpleDraweeView
 import com.facebook.soloader.SoLoader
 import com.nhaarman.mockitokotlin2.whenever
+import com.nhaarman.mockitokotlin2.doReturn
 import fr.free.nrw.commons.LocationPicker.LocationPickerActivity
 import fr.free.nrw.commons.Media
 import fr.free.nrw.commons.R
@@ -132,21 +134,26 @@ class MediaDetailFragmentUnitTests {
     @Mock
     private lateinit var listView: ListView
 
-    @Mock
-    private lateinit var searchView: SearchView
-
     @Mock
     private lateinit var intent: Intent
 
     private lateinit var activity: SearchActivity
     
+    @Mock
+    private lateinit var mockContext: Context
+    
+    @Mock
+    private lateinit var mockSharedPreferences: SharedPreferences
+    
+    @Mock
+    private lateinit var mockSharedPreferencesEditor:  SharedPreferences.Editor
+
     @Before
     fun setUp() {
 
         MockitoAnnotations.openMocks(this)
 
         context = ApplicationProvider.getApplicationContext()
-
         AppAdapter.set(TestAppAdapter())
 
         SoLoader.setInTestMode()
@@ -212,6 +219,10 @@ class MediaDetailFragmentUnitTests {
         val map = HashMap<String, String>()
         map[Locale.getDefault().language] = ""
         `when`(media.descriptions).thenReturn(map)
+
+        doReturn(mockSharedPreferences).`when`(mockContext).getSharedPreferences(anyString(), anyInt())
+        doReturn(mockSharedPreferencesEditor).`when`(mockSharedPreferences).edit()
+        doReturn(mockSharedPreferencesEditor).`when`(mockSharedPreferencesEditor).putInt(anyString(), anyInt())
     }
 
     @Test
@@ -805,4 +816,30 @@ class MediaDetailFragmentUnitTests {
         method.isAccessible = true
         method.invoke(fragment, media)
     }
+    
+    @Test
+    fun testOnImageBackgroundChangedWithDifferentColor() {
+        val spyFragment = spy(fragment)
+        val color = 0xffffff
+        doReturn(mockContext).`when`(spyFragment).context
+        doReturn(-1).`when`(mockSharedPreferences).getInt(anyString(), anyInt())
+
+        spyFragment.onImageBackgroundChanged(color)
+
+        verify(simpleDraweeView, times(1)).setBackgroundColor(color) 
+        verify(mockSharedPreferencesEditor, times(1)).putInt(anyString(), anyInt())
+    }
+
+
+    @Test
+    fun testOnImageBackgroundChangedWithSameColor() {
+        val spyFragment = spy(fragment)
+        val color = 0
+        doReturn(mockContext).`when`(spyFragment).context
+        doReturn(color).`when`(mockSharedPreferences).getInt(anyString(), anyInt())
+
+        spyFragment.onImageBackgroundChanged(color)
+        verify(simpleDraweeView, never()).setBackgroundColor(anyInt())
+        verify(mockSharedPreferencesEditor, never()).putInt(anyString(), anyInt())
+    }
 }

app/src/test/kotlin/fr/free/nrw/commons/media/MediaDetailPagerFragmentUnitTests.kt

@@ -1,24 +1,34 @@
 package fr.free.nrw.commons.media
 
 import android.content.Context
+import android.graphics.Bitmap
 import android.os.Bundle
 import android.os.Looper
+import android.view.Menu
+import android.view.MenuItem
 import androidx.fragment.app.FragmentManager
 import androidx.fragment.app.FragmentTransaction
 import androidx.test.core.app.ApplicationProvider
 import com.facebook.drawee.backends.pipeline.Fresco
 import com.facebook.soloader.SoLoader
+import com.nhaarman.mockitokotlin2.any
+import com.nhaarman.mockitokotlin2.doReturn
+import com.nhaarman.mockitokotlin2.never
+import com.nhaarman.mockitokotlin2.times
+import com.nhaarman.mockitokotlin2.verify
 import fr.free.nrw.commons.Media
 import fr.free.nrw.commons.TestAppAdapter
 import fr.free.nrw.commons.TestCommonsApplication
 import fr.free.nrw.commons.auth.SessionManager
 import fr.free.nrw.commons.explore.SearchActivity
-import fr.free.nrw.commons.mwapi.OkHttpJsonApiClient
+import io.reactivex.android.plugins.RxAndroidPlugins
+import io.reactivex.plugins.RxJavaPlugins
+import io.reactivex.schedulers.Schedulers
+import org.junit.After
 import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.mockito.InjectMocks
 import org.mockito.Mock
 import org.mockito.Mockito.`when`
 import org.mockito.MockitoAnnotations
@@ -30,12 +40,12 @@ import org.robolectric.annotation.LooperMode
 import org.wikipedia.AppAdapter
 import java.lang.reflect.Field
 import java.lang.reflect.Method
+import java.util.concurrent.Callable
 
 @RunWith(RobolectricTestRunner::class)
 @Config(sdk = [21], application = TestCommonsApplication::class)
 @LooperMode(LooperMode.Mode.PAUSED)
 class MediaDetailPagerFragmentUnitTests {
-
     private lateinit var fragment: MediaDetailPagerFragment
     private lateinit var context: Context
     private lateinit var fragmentManager: FragmentManager
@@ -49,11 +59,19 @@ class MediaDetailPagerFragmentUnitTests {
     @Mock
     internal var sessionManager: SessionManager? = null
 
-    @InjectMocks
+    @Mock
-    private lateinit var okHttpJsonApiClient: OkHttpJsonApiClient
+    private lateinit var menu: Menu
+
+    @Mock
+    private lateinit var menuItem: MenuItem
+
+    @Mock
+    private lateinit var bitmap: Bitmap
 
     @Before
     fun setUp() {
+        RxAndroidPlugins.setMainThreadSchedulerHandler { Schedulers.trampoline() }
+        RxJavaPlugins.setNewThreadSchedulerHandler { Schedulers.trampoline() }
         
         MockitoAnnotations.openMocks(this)
 
@@ -67,8 +85,8 @@ class MediaDetailPagerFragmentUnitTests {
 
         val activity = Robolectric.buildActivity(SearchActivity::class.java).create().get()
 
-        fragment = MediaDetailPagerFragment.newInstance(false, true);
+        fragment = MediaDetailPagerFragment.newInstance(false, true)
-        fragment = MediaDetailPagerFragment.newInstance(false, false);
+        fragment = MediaDetailPagerFragment.newInstance(false, false)
         fragmentManager = activity.supportFragmentManager
         val fragmentTransaction: FragmentTransaction = fragmentManager.beginTransaction()
         fragmentTransaction.add(fragment, null)
@@ -78,6 +96,16 @@ class MediaDetailPagerFragmentUnitTests {
             SessionManager::class.java.getDeclaredField("context")
         fieldContext.isAccessible = true
         fieldContext.set(sessionManager, context)
+
+        doReturn(menuItem).`when`(menu).findItem(any())
+        doReturn(menuItem).`when`(menuItem).isEnabled = any()
+        doReturn(menuItem).`when`(menuItem).isVisible = any()
+    }
+    
+    @After
+    fun tearDown() {
+        RxAndroidPlugins.reset()
+        RxJavaPlugins.reset()
     }
 
     @Test
@@ -198,4 +226,41 @@ class MediaDetailPagerFragmentUnitTests {
         fragment.onDataSetChanged()
     }
 
+    private fun invokeHandleBackgroundColorMenuItems(getBitmap: Callable<Bitmap>) {
+        val method: Method = MediaDetailPagerFragment::class.java.getDeclaredMethod(
+            "handleBackgroundColorMenuItems",
+            Callable::class.java,
+            Menu::class.java
+        )
+        method.isAccessible = true
+        method.invoke(fragment, getBitmap, menu)
+    }
+
+    @Test
+    fun testShouldDisplayBackgroundColorMenuWithTransparentMedia() {
+        doReturn(true).`when`(bitmap).hasAlpha()
+
+        invokeHandleBackgroundColorMenuItems {
+            bitmap
+        }
+
+        verify(bitmap, times(1)).hasAlpha()
+        verify(menu, times(2)).findItem(any())
+        verify(menuItem, times(2)).isEnabled = true
+        verify(menuItem, times(2)).isVisible = true
+    }
+
+    @Test
+    fun testShouldNotDisplayBackgroundColorMenuWithOpaqueMedia() {
+        doReturn(false).`when`(bitmap).hasAlpha()
+
+        invokeHandleBackgroundColorMenuItems {
+            bitmap
+        }
+
+        verify(bitmap, times(1)).hasAlpha()
+        verify(menu, never()).findItem(any())
+        verify(menuItem, never()).isEnabled = true
+        verify(menuItem, never()).isVisible = true
+    }
 }