Fix #2915 Refractor feature Review (#2916)

* BugFix #2915
* Refractor ReviewActivity and ReviewImageFragment and the related layout files, to properly use the scrollview
* Use ButterKnife for ViewBindings in ReviewImageFragment
* updated resource id names to follow underscore notation in xml
* Use menu item instead of ImageView over toolbar in ReviewActivity
* use tools:replace instead of android:text for dummy texts

* merge nested if's [Codacy review]

* updated string review_category_yes_button_text, use textAllCaps in yes and no button in ReviewFragment

* updated other strings to use non bold letters

app/src/main/java/fr/free/nrw/commons/review/ReviewActivity.java

@@ -4,24 +4,21 @@ import android.annotation.SuppressLint;
 import android.content.Context;
 import android.content.Intent;
 import android.os.Bundle;
+import android.view.Menu;
+import android.view.MenuInflater;
+import android.view.MenuItem;
+import android.view.MotionEvent;
 import android.view.View;
 import android.widget.Button;
-import android.widget.ImageView;
 import android.widget.ProgressBar;
 import android.widget.TextView;
-
-import com.facebook.drawee.view.SimpleDraweeView;
-import com.google.android.material.navigation.NavigationView;
-import com.viewpagerindicator.CirclePageIndicator;
-
-import java.util.ArrayList;
-
-import javax.inject.Inject;
-
 import androidx.appcompat.widget.Toolbar;
 import androidx.drawerlayout.widget.DrawerLayout;
 import butterknife.BindView;
 import butterknife.ButterKnife;
+import com.facebook.drawee.view.SimpleDraweeView;
+import com.google.android.material.navigation.NavigationView;
+import com.viewpagerindicator.CirclePageIndicator;
 import fr.free.nrw.commons.Media;
 import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.auth.AuthenticatedActivity;
@@ -32,10 +29,12 @@ import fr.free.nrw.commons.utils.ViewUtil;
 import io.reactivex.android.schedulers.AndroidSchedulers;
 import io.reactivex.disposables.CompositeDisposable;
 import io.reactivex.schedulers.Schedulers;
+import java.util.ArrayList;
+import javax.inject.Inject;
 
 public class ReviewActivity extends AuthenticatedActivity {
 
-    @BindView(R.id.reviewPagerIndicator)
+    @BindView(R.id.pager_indicator_review)
     public CirclePageIndicator pagerIndicator;
     @BindView(R.id.toolbar)
     Toolbar toolbar;
@@ -43,20 +42,16 @@ public class ReviewActivity extends AuthenticatedActivity {
     NavigationView navigationView;
     @BindView(R.id.drawer_layout)
     DrawerLayout drawerLayout;
-    @BindView(R.id.reviewPager)
+    @BindView(R.id.view_pager_review)
     ReviewViewPager reviewPager;
     @BindView(R.id.skip_image)
-    Button skip_image_button;
-    @BindView(R.id.imageView)
+    Button btnSkipImage;
+    @BindView(R.id.review_image_view)
     SimpleDraweeView simpleDraweeView;
-    @BindView(R.id.progressBar)
+    @BindView(R.id.pb_review_image)
     ProgressBar progressBar;
-    @BindView(R.id.imageCaption)
+    @BindView(R.id.tv_image_caption)
     TextView imageCaption;
-    @BindView(R.id.skip_image_info)
-    ImageView skipImageInfo;
-    @BindView(R.id.review_image_info)
-    ImageView reviewImageInfo;
     public ReviewPagerAdapter reviewPagerAdapter;
     public ReviewController reviewController;
     @Inject
@@ -96,6 +91,7 @@ public class ReviewActivity extends AuthenticatedActivity {
         super.onCreate(savedInstanceState);
         setContentView(R.layout.activity_review);
         ButterKnife.bind(this);
+        setSupportActionBar(toolbar);
         initDrawer();
 
         reviewController = new ReviewController(deleteHelper, this);
@@ -108,9 +104,17 @@ public class ReviewActivity extends AuthenticatedActivity {
 
         runRandomizer(); //Run randomizer whenever everything is ready so that a first random image will be added
 
-        skip_image_button.setOnClickListener(view -> runRandomizer());
-        skipImageInfo.setOnClickListener(view -> showSkipImageInfo());
-        reviewImageInfo.setOnClickListener(view -> showReviewImageInfo());
+        btnSkipImage.setOnClickListener(view -> runRandomizer());
+
+        btnSkipImage.setOnTouchListener((view, event) -> {
+            if (event.getAction() == MotionEvent.ACTION_UP && event.getRawX() >= (
+                    btnSkipImage.getRight() - btnSkipImage
+                            .getCompoundDrawables()[2].getBounds().width())) {
+                showSkipImageInfo();
+                return true;
+            }
+            return false;
+        });
     }
 
     @SuppressLint("CheckResult")
@@ -187,4 +191,22 @@ public class ReviewActivity extends AuthenticatedActivity {
                 null,
                 null);
     }
+
+
+    @Override
+    public boolean onCreateOptionsMenu(Menu menu) {
+        MenuInflater inflater = getMenuInflater();
+        inflater.inflate(R.menu.menu_review_activty, menu);
+        return super.onCreateOptionsMenu(menu);
+    }
+
+    @Override
+    public boolean onOptionsItemSelected(MenuItem item) {
+        switch (item.getItemId()) {
+            case R.id.menu_image_info:
+                showReviewImageInfo();
+                return true;
+        }
+        return super.onOptionsItemSelected(item);
+    }
 }

app/src/main/java/fr/free/nrw/commons/review/ReviewImageFragment.java

@@ -10,11 +10,12 @@ import android.view.ViewGroup;
 import android.widget.Button;
 import android.widget.ProgressBar;
 import android.widget.TextView;
-
-import org.wikipedia.dataclient.mwapi.MwQueryPage;
-
+import butterknife.BindView;
+import butterknife.ButterKnife;
+import butterknife.OnClick;
 import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.di.CommonsDaggerSupportFragment;
+import org.wikipedia.dataclient.mwapi.MwQueryPage;
 
 public class ReviewImageFragment extends CommonsDaggerSupportFragment {
 
@@ -27,16 +28,18 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
     private String fileName;
     private String catString;
 
-    private View textViewQuestionContext;
-    private View textViewQuestion;
-
-    private Button yesButton;
-    private Button noButton;
-
-
     public ProgressBar progressBar;
     private MwQueryPage.Revision revision;
 
+    @BindView(R.id.tv_review_question)
+    TextView textViewQuestion;
+    @BindView(R.id.tv_review_question_context)
+    TextView textViewQuestionContext;
+    @BindView(R.id.button_yes)
+    Button yesButton;
+    @BindView(R.id.button_no)
+    Button noButton;
+
 
     public void update(int position, String fileName) {
         this.position = position;
@@ -50,9 +53,9 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
             if (catString != null && !catString.equals("") && textViewQuestionContext != null) {
                 catString = "<b>" + catString + "</b>";
                 String stringToConvertHtml = String.format(getResources().getString(R.string.review_category_explanation), catString);
-                ((TextView) textViewQuestionContext).setText(Html.fromHtml(stringToConvertHtml));
+                textViewQuestionContext.setText(Html.fromHtml(stringToConvertHtml));
             } else if (textViewQuestionContext != null) {
-                ((TextView) textViewQuestionContext).setText(getResources().getString(R.string.review_no_category));
+                textViewQuestionContext.setText(getResources().getString(R.string.review_no_category));
             }
         }
     }
@@ -68,10 +71,7 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
         position = getArguments().getInt("position");
         View layoutView = inflater.inflate(R.layout.fragment_review_image, container,
                 false);
-        textViewQuestion = layoutView.findViewById(R.id.reviewQuestion);
-        textViewQuestionContext = layoutView.findViewById(R.id.reviewQuestionContext);
-        yesButton = layoutView.findViewById(R.id.yesButton);
-        noButton = layoutView.findViewById(R.id.noButton);
+        ButterKnife.bind(this,layoutView);
 
         String question, explanation, yesButtonText, noButtonText;
         switch (position) {
@@ -118,10 +118,8 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
                 noButtonText = "no";
         }
 
-        noButton.setOnClickListener(view -> getReviewActivity().swipeToNext());
-
-        ((TextView) textViewQuestion).setText(question);
-        ((TextView) textViewQuestionContext).setText(explanation);
+        textViewQuestion.setText(question);
+        textViewQuestionContext.setText(explanation);
         yesButton.setText(yesButtonText);
         noButton.setText(noButtonText);
 
@@ -132,6 +130,11 @@ public class ReviewImageFragment extends CommonsDaggerSupportFragment {
         return layoutView;
     }
 
+    @OnClick(R.id.button_no)
+    public void onNoButtonClicked() {
+        getReviewActivity().swipeToNext();
+    }
+
     private ReviewActivity getReviewActivity() {
         return (ReviewActivity) requireActivity();
     }

app/src/main/res/layout/activity_review.xml

@@ -1,80 +1,56 @@
 <?xml version="1.0" encoding="utf-8"?>
 <androidx.drawerlayout.widget.DrawerLayout xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:app="http://schemas.android.com/apk/res-auto"
-    android:id="@+id/drawer_layout"
-    android:layout_width="match_parent"
-    android:layout_height="match_parent">
+  xmlns:app="http://schemas.android.com/apk/res-auto"
+  xmlns:tools="http://schemas.android.com/tools"
+  android:id="@+id/drawer_layout"
+  android:layout_width="match_parent"
+  android:layout_height="match_parent">
 
-    <androidx.coordinatorlayout.widget.CoordinatorLayout
-        android:id="@+id/coordinator_layout"
-        android:layout_width="match_parent"
-        android:layout_height="match_parent">
 
-        <RelativeLayout
+
+        <LinearLayout
             android:layout_width="match_parent"
-            android:layout_height="match_parent">
+            android:layout_height="match_parent"
+            android:orientation="vertical">
 
             <include layout="@layout/toolbar" />
 
-            <ImageView
-                android:layout_width="22dp"
-                android:layout_height="22dp"
-                android:id="@+id/review_image_info"
-                app:srcCompat="@drawable/ic_info_outline_24dp"
-                android:tint="@color/white"
-                android:layout_alignParentRight="true"
-                android:layout_margin="20dp"/>
 
-            <LinearLayout
-                android:id="@+id/skip_image_row"
-                android:layout_width="match_parent"
-                android:layout_height="25dp"
-                android:layout_below="@+id/toolbar"
-                android:orientation="horizontal"
-                android:gravity="center">
+            <androidx.appcompat.widget.AppCompatButton
+              android:id="@+id/skip_image"
+              style="@style/Widget.AppCompat.Button.Borderless"
+              android:textStyle="bold"
+              android:layout_width="wrap_content"
+              android:layout_height="wrap_content"
+              android:layout_gravity="center_horizontal"
+              android:paddingLeft="12dp"
+              android:paddingRight="12dp"
+              android:drawableEnd="@drawable/ic_info_outline_24dp"
+              android:drawablePadding="12dp"
+              android:drawableTint="@color/button_blue_dark"
+              android:text="@string/skip_image"
+              android:textColor="@color/button_blue_dark"/>
 
-                <Button
-                    android:id="@+id/skip_image"
-                    android:layout_width="130dp"
-                    android:layout_height="match_parent"
-                    android:background="@android:color/transparent"
-                    android:text="@string/skip_image"
-                    android:textColor="@color/button_blue_dark"
-                    android:textStyle="bold" />
+            <RelativeLayout
+              android:layout_width="match_parent"
+              android:layout_height="match_parent">
 
-                <ImageView
-                    android:layout_width="15dp"
-                    android:layout_height="15dp"
-                    android:id="@+id/skip_image_info"
-                    android:layout_marginTop="4dp"
-                    android:layout_marginRight="@dimen/activity_margin_horizontal"
-                    android:layout_marginEnd="@dimen/activity_margin_horizontal"
-                    android:layout_gravity="top"
-                    app:srcCompat="@drawable/ic_info_outline_24dp"
-                    android:tint="@color/button_blue_dark" />
+                <LinearLayout
+                  android:layout_width="match_parent"
+                  android:layout_height="match_parent"
+                  android:orientation="vertical"
+                  android:weightSum="2"
+                  >
 
-            </LinearLayout>
-            <ScrollView
-                android:layout_width="match_parent"
-                android:layout_height="wrap_content"
-                android:layout_below="@+id/skip_image_row"
-                android:layout_above="@+id/reviewPagerIndicator"
-                >
-
-            <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
-                android:layout_width="match_parent"
-                android:layout_height="match_parent"
-                android:orientation="vertical"
-                >
-
-                <RelativeLayout
-                    android:layout_width="match_parent"
-                    android:layout_height="300dp"
-                    android:layout_marginTop="5dp"
-                    >
+                    <RelativeLayout
+                      android:layout_width="match_parent"
+                      android:layout_height="0dp"
+                      android:layout_weight="1"
+                      android:layout_marginTop="5dp"
+                      >
 
                     <com.facebook.drawee.view.SimpleDraweeView
-                        android:id="@+id/imageView"
+                        android:id="@+id/review_image_view"
                         android:layout_width="match_parent"
                         android:layout_height="match_parent"
                         android:layout_alignParentTop="true"
@@ -82,7 +58,7 @@
                         app:srcCompat="@drawable/commons_logo" />
 
                     <RelativeLayout
-                        android:id="@+id/uploadOverlay"
+                        android:id="@+id/rl_container_upload_overlay"
                         android:layout_width="match_parent"
                         android:layout_height="wrap_content"
                         android:layout_alignParentBottom="true"
@@ -92,7 +68,7 @@
                         android:padding="@dimen/tiny_gap">
 
                         <TextView
-                            android:id="@+id/imageCaption"
+                            android:id="@+id/tv_image_caption"
                             style="?android:textAppearanceMedium"
                             android:layout_width="wrap_content"
                             android:layout_height="wrap_content"
@@ -101,44 +77,46 @@
                     </RelativeLayout>
 
                     <ProgressBar
-                        android:id="@+id/progressBar"
+                        android:id="@+id/pb_review_image"
                         android:layout_width="wrap_content"
                         android:layout_height="wrap_content"
                         android:layout_centerInParent="true"
-                        android:visibility="gone" />
+                        android:visibility="gone"
+                        tools:visibility="visible"/>
 
                 </RelativeLayout>
 
-                <fr.free.nrw.commons.review.ReviewViewPager
-                    android:id="@+id/reviewPager"
-                    android:layout_width="match_parent"
-                    android:layout_height="300dp"
-                    android:fadingEdge="none" />
+                    <fr.free.nrw.commons.review.ReviewViewPager
+                      android:id="@+id/view_pager_review"
+                      android:layout_width="match_parent"
+                      android:layout_height="0dp"
+                      android:layout_weight="1"
+                      android:fadingEdge="none"/>
 
             </LinearLayout>
-            </ScrollView>
 
-            <View
-                android:id="@+id/bottomview"
-                android:layout_width="match_parent"
-                android:layout_height="15dp"
-                android:layout_alignParentBottom="true"
-                android:background="?attr/colorPrimaryDark"></View>
+                <RelativeLayout
+                  android:id="@+id/rl_container_bottom_view"
+                  android:layout_width="match_parent"
+                  android:layout_height="wrap_content"
+                  android:padding="12dp"
+                  android:layout_alignParentBottom="true"
+                  android:background="?attr/colorPrimaryDark"
+                  android:elevation="2dp">
+                    <com.viewpagerindicator.CirclePageIndicator
+                      android:id="@+id/pager_indicator_review"
+                      android:layout_width="match_parent"
+                      android:layout_height="wrap_content"
+                      android:layout_gravity="center"
+                      android:background="?attr/colorPrimaryDark"
+                      android:foregroundGravity="center_vertical"
+                      />
+                </RelativeLayout>
 
-            <com.viewpagerindicator.CirclePageIndicator
-                android:id="@+id/reviewPagerIndicator"
-                android:layout_width="match_parent"
-                android:layout_height="10dp"
-                android:layout_gravity="center"
-                android:background="?attr/colorPrimaryDark"
-                android:elevation="1dp"
-                android:foregroundGravity="center_vertical"
-                android:layout_alignParentBottom="true"/>
+            </RelativeLayout>
 
 
-        </RelativeLayout>
-
-    </androidx.coordinatorlayout.widget.CoordinatorLayout>
+        </LinearLayout>
 
     <include layout="@layout/drawer_view" />
 

app/src/main/res/layout/fragment_review_image.xml

@@ -1,70 +1,80 @@
 <?xml version="1.0" encoding="utf-8"?>
 <RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
+  android:layout_width="match_parent"
+  android:layout_height="match_parent"
+  xmlns:tools="http://schemas.android.com/tools"
+  android:paddingBottom="10dp"
+  android:orientation="vertical"
+  >
+
+  <ScrollView
     android:layout_width="match_parent"
     android:layout_height="match_parent"
-    android:orientation="vertical">
+    android:layout_above="@id/ll_container_buttons">
 
+    <LinearLayout
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:paddingTop="6dp"
+      android:orientation="vertical">
 
+      <TextView
+        android:id="@+id/tv_review_question"
+        android:layout_width="match_parent"
+        android:layout_height="80dp"
+        android:gravity="center_vertical"
+        tools:text="testing1"
+        android:textAlignment="center"
+        android:textColor="?attr/reviewHeading"
+        android:textSize="32sp"/>
 
-        <LinearLayout
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content"
-            android:paddingTop="6dp"
-            android:orientation="vertical">
+      <TextView
+        android:id="@+id/tv_review_question_context"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:layout_marginBottom="15dp"
+        android:gravity="center_vertical"
+        tools:text="testing2"
+        android:textAlignment="center"
+        android:textSize="22sp"/>
 
+    </LinearLayout>
+  </ScrollView>
 
-            <TextView
-                android:id="@+id/reviewQuestion"
-                android:layout_width="match_parent"
-                android:layout_height="80dp"
-                android:gravity="center_vertical"
-                android:text="testing1"
-                android:textAlignment="center"
-                android:textColor="?attr/reviewHeading"
-                android:textSize="32sp" />
+  <LinearLayout
+    android:id="@+id/ll_container_buttons"
+    android:layout_width="match_parent"
+    android:layout_height="70dp"
+    android:layout_alignParentBottom="true"
+    android:orientation="horizontal"
+    android:padding="2dp"
+    android:weightSum="2">
 
-            <TextView
-                android:id="@+id/reviewQuestionContext"
-                android:layout_width="match_parent"
-                android:layout_height="wrap_content"
-                android:layout_marginBottom="15dp"
-                android:gravity="center_vertical"
-                android:text="testing2"
-                android:textAlignment="center"
-                android:textSize="22sp" />
+    <Button
+      android:id="@+id/button_yes"
+      android:layout_width="0dp"
+      android:layout_height="48dp"
+      android:layout_weight="1"
+      android:layout_margin="@dimen/activity_margin_horizontal"
+      android:background="@android:color/transparent"
+      android:text="@string/yes"
+      android:textAllCaps="true"
+      android:textAlignment="center"
+      android:textColor="@color/yes_button_color"/>
 
-            <LinearLayout
-                android:layout_width="match_parent"
-                android:layout_height="70dp"
-                android:orientation="horizontal"
-                android:weightSum="2">
+    <Button
+      android:textAllCaps="true"
+      android:id="@+id/button_no"
+      android:layout_width="0dp"
+      android:layout_height="48dp"
+      android:layout_weight="1"
+      android:layout_margin="@dimen/activity_margin_horizontal"
+      android:background="@android:color/transparent"
+      android:text="@string/no"
+      android:textAlignment="center"
+      android:textColor="@color/no_button_color"
+      />
 
-                <Button
-                    android:id="@+id/yesButton"
-                    android:layout_width="0dp"
-                    android:layout_height="wrap_content"
-                    android:layout_margin="@dimen/activity_margin_horizontal"
-                    android:layout_weight="1"
-                    android:background="@android:color/transparent"
-                    android:text="@string/yes"
-                    android:textAlignment="center"
-                    android:textColor="@color/yes_button_color"
-                    android:textSize="18sp" />
-
-                <Button
-                    android:id="@+id/noButton"
-                    android:layout_width="0dp"
-                    android:layout_height="wrap_content"
-                    android:layout_margin="@dimen/activity_margin_horizontal"
-                    android:layout_weight="1"
-                    android:background="@android:color/transparent"
-                    android:text="@string/no"
-                    android:textAlignment="center"
-                    android:textColor="@color/no_button_color"
-                    android:textSize="18sp" />
-
-            </LinearLayout>
-
-        </LinearLayout>
+  </LinearLayout>
 
 </RelativeLayout>

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

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<menu xmlns:android="http://schemas.android.com/apk/res/android"
+  xmlns:app="http://schemas.android.com/apk/res-auto">
+  <item
+    android:id="@+id/menu_image_info"
+    app:showAsAction="always"
+    android:title="@string/image_info"
+    android:icon="@drawable/ic_info_outline_24dp"/>
+</menu>

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

@@ -500,14 +500,14 @@ Upload your first media by tapping on the add button.</string>
   <string name="review_spam_report_problem">spam</string>
   <string name="review_c_violation_report_question">It is copyright violation because it is </string>
   <string name="review_c_violation_report_problem">copyRightViolation</string>
-    <string name="review_category_yes_button_text">NO, MIS-CATEGORIZED</string>
-    <string name="review_category_no_button_text">SEEMS FINE</string>
-    <string name="review_spam_yes_button_text">NO, OUT OF SCOPE</string>
-    <string name="review_spam_no_button_text">SEEMS FINE</string>
-    <string name="review_copyright_yes_button_text">NO, COPYRIGHT VIOLATION</string>
-    <string name="review_copyright_no_button_text">SEEMS FINE</string>
-    <string name="review_thanks_yes_button_text">YES, WHY NOT</string>
-    <string name="review_thanks_no_button_text">NEXT IMAGE</string>
+    <string name="review_category_yes_button_text">No, mis-categorized</string>
+    <string name="review_category_no_button_text">Seems fine</string>
+    <string name="review_spam_yes_button_text">No, out of scope</string>
+    <string name="review_spam_no_button_text">Seems fine</string>
+    <string name="review_copyright_yes_button_text">No, copyright violation</string>
+    <string name="review_copyright_no_button_text">Seems fine</string>
+    <string name="review_thanks_yes_button_text">Yes, why not</string>
+    <string name="review_thanks_no_button_text">Next image</string>
     <string name="skip_image_explanation">Clicking this button will give you another recently uploaded image from Wikimedia Commons</string>
     <string name="review_image_explanation">You can review images and improve the quality of Wikimedia Commoms.\n The four parameters of review are: \n - Is this image in-scope? \n - Does this image follow the rules of copyright? \n - Is this image correctly categorized? \n - If all goes well you can also thank the contributor.</string>
 
@@ -539,4 +539,5 @@ Upload your first media by tapping on the add button.</string>
   <string name="welcome_dont_upload_content_description">Examples of images not to upload</string>
   <string name="skip_image">SKIP THIS IMAGE</string>
   <string name="download_failed_we_cannot_download_the_file_without_storage_permission">Download Failed!!. We cannot download the file without external storage permission.</string>
+  <string name="image_info">Image Info</string>
 </resources>