From e36ebfeffe6ced16f364101ca7356908bae015f5 Mon Sep 17 00:00:00 2001 From: Josephine Lim Date: Mon, 7 May 2018 19:27:14 +1000 Subject: [PATCH 1/6] Update pull_request_template.md (#1476) * Update pull_request_template.md * Remove Javadocs mention * Added required/optional notes --- PULL_REQUEST_TEMPLATE.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md index 34078f07e..9d7150008 100644 --- a/PULL_REQUEST_TEMPLATE.md +++ b/PULL_REQUEST_TEMPLATE.md @@ -1,15 +1,15 @@ -## Description +## Description (required) -Fixes #{GitHub issue number} +Fixes #{GitHub issue number and title} {Describe the changes made and why they were made.} -## Tests performed +## Tests performed (required) Tested on {API level & name of device/emulator}, with {build variant, e.g. ProdDebug}. -{Please test your PR at least once before submitting.} - -## Screenshots showing what changed +## Screenshots showing what changed (optional) {Only for user interface changes, otherwise remove this section. See [how to take a screenshot](https://android.stackexchange.com/questions/1759/how-to-take-a-screenshot-with-an-android-device)} + +_Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request._ From bd86dde4442cbe2f60628a1a99ebb5b01df17de4 Mon Sep 17 00:00:00 2001 From: Man Parvesh Singh Randhawa Date: Mon, 7 May 2018 17:32:11 +0800 Subject: [PATCH 2/6] resolves #1464 : MediaDataExtractor is making inefficient (redundant) server calls (#1496) --- app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.java b/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.java index 2d79a6c4f..affb57528 100644 --- a/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.java +++ b/app/src/main/java/fr/free/nrw/commons/MediaDataExtractor.java @@ -61,8 +61,8 @@ public class MediaDataExtractor { } try{ - Timber.d("Nominated for deletion: " + mediaWikiApi.pageExists("Commons:Deletion_requests/"+filename)); - deletionStatus = mediaWikiApi.pageExists("Commons:Deletion_requests/"+filename); + deletionStatus = mediaWikiApi.pageExists("Commons:Deletion_requests/" + filename); + Timber.d("Nominated for deletion: " + deletionStatus); } catch (Exception e){ Timber.d(e.getMessage()); From 4f6b791c936b6559018bd5c35ea7783f6b01c688 Mon Sep 17 00:00:00 2001 From: Tanvi Dadu Date: Mon, 7 May 2018 15:27:59 +0530 Subject: [PATCH 3/6] Open map of place where picture was taken (#1360) * Intent to map added * Merge conflicts resolved * Added the functionality to hide FAB incase of null coordinate * Merge Conflict resolved * Improve pr quality * Improve Quality * Added nested FAB animations * Nested FAB implemented * Improve Quality * Added up arrow * Javadocs Added --- .../nrw/commons/upload/ShareActivity.java | 105 +++++++++++++++++- .../ic_keyboard_arrow_up_black_24dp.xml | 5 + app/src/main/res/layout/activity_share.xml | 27 ++++- app/src/main/res/values/dimens.xml | 2 + app/src/main/res/values/strings.xml | 2 + 5 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 app/src/main/res/drawable/ic_keyboard_arrow_up_black_24dp.xml diff --git a/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java b/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java index 77f76fbed..6a59c8e30 100644 --- a/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java @@ -81,8 +81,7 @@ import fr.free.nrw.commons.mwapi.MediaWikiApi; import fr.free.nrw.commons.utils.ViewUtil; import timber.log.Timber; - - +import android.support.design.widget.FloatingActionButton; import static fr.free.nrw.commons.upload.ExistingFileAsync.Result.DUPLICATE_PROCEED; import static fr.free.nrw.commons.upload.ExistingFileAsync.Result.NO_DUPLICATE; import static java.lang.Long.min; @@ -122,6 +121,7 @@ public class ShareActivity private Uri mediaUri; private Contribution contribution; private SimpleDraweeView backgroundImageView; + private FloatingActionButton maps_fragment; private boolean cacheFound; @@ -145,6 +145,8 @@ public class ShareActivity private long ShortAnimationDuration; private FloatingActionButton zoomInButton; private FloatingActionButton zoomOutButton; + private FloatingActionButton mainFab; + private boolean isFABOpen = false; /** @@ -284,6 +286,24 @@ public class ShareActivity if (mediaUri != null) { backgroundImageView.setImageURI(mediaUri); } + + mainFab = (FloatingActionButton) findViewById(R.id.main_fab); + /* + * called when upper arrow floating button + */ + mainFab.setOnClickListener(new View.OnClickListener() { + @Override + public void onClick(View v) { + if(!isFABOpen){ + showFABMenu(); + }else{ + closeFABMenu(); + } + } + }); + + + zoomInButton = (FloatingActionButton) findViewById(R.id.media_upload_zoom_in); try { zoomInButton.setOnClickListener(new View.OnClickListener() { @@ -358,7 +378,74 @@ public class ShareActivity .commitAllowingStateLoss(); } uploadController.prepareService(); + maps_fragment = (FloatingActionButton) findViewById(R.id.media_map); + maps_fragment.setVisibility(View.VISIBLE); + if( imageObj == null || imageObj.imageCoordsExists != true){ + maps_fragment.setVisibility(View.INVISIBLE); + } + + + maps_fragment.setOnClickListener(new View.OnClickListener() { + @Override + public void onClick(View v) { + if( imageObj != null && imageObj.imageCoordsExists == true) { + Uri gmmIntentUri = Uri.parse("google.streetview:cbll=" + imageObj.getDecLatitude() + "," + imageObj.getDecLongitude()); + Intent mapIntent = new Intent(Intent.ACTION_VIEW, gmmIntentUri); + mapIntent.setPackage("com.google.android.apps.maps"); + startActivity(mapIntent); + } + } + }); } + /* + * Function to display the zoom and map FAB + */ + private void showFABMenu(){ + isFABOpen=true; + + if( imageObj != null && imageObj.imageCoordsExists == true) + maps_fragment.setVisibility(View.VISIBLE); + zoomInButton.setVisibility(View.VISIBLE); + + mainFab.animate().rotationBy(180); + maps_fragment.animate().translationY(-getResources().getDimension(R.dimen.second_fab)); + zoomInButton.animate().translationY(-getResources().getDimension(R.dimen.first_fab)); + } + + /* + * function to close the zoom and map FAB + */ + private void closeFABMenu(){ + isFABOpen=false; + mainFab.animate().rotationBy(-180); + maps_fragment.animate().translationY(0); + zoomInButton.animate().translationY(0).setListener(new Animator.AnimatorListener() { + @Override + public void onAnimationStart(Animator animator) { + + } + + @Override + public void onAnimationEnd(Animator animator) { + if(!isFABOpen){ + maps_fragment.setVisibility(View.GONE); + zoomInButton.setVisibility(View.GONE); + } + + } + + @Override + public void onAnimationCancel(Animator animator) { + + } + + @Override + public void onAnimationRepeat(Animator animator) { + + } + }); + } + @Override public void onRequestPermissionsResult(int requestCode, @@ -461,6 +548,9 @@ public class ShareActivity detectUnwantedPicturesAsync.execute(); } + /* + * to display permission snackbar in share activity + */ private Snackbar requestPermissionUsingSnackBar(String rationale, final String[] perms, final int code) { @@ -693,7 +783,9 @@ public class ShareActivity return super.onOptionsItemSelected(item); } - // Get SHA1 of file from input stream + /* + * Get SHA1 of file from input stream + */ private String getSHA1(InputStream is) { MessageDigest digest; @@ -730,6 +822,9 @@ public class ShareActivity } } + /* + * function to provide pinch zoom + */ private void zoomImageFromThumb(final View thumbView, Uri imageuri ) { // If there's an animation in progress, cancel it // immediately and proceed with this one. @@ -737,6 +832,8 @@ public class ShareActivity CurrentAnimator.cancel(); } ViewUtil.hideKeyboard(ShareActivity.this.findViewById(R.id.titleEdit | R.id.descEdit)); + closeFABMenu(); + mainFab.setVisibility(View.GONE); InputStream input = null; Bitmap scaled = null; try { @@ -866,7 +963,7 @@ public class ShareActivity CurrentAnimator.cancel(); } zoomOutButton.setVisibility(View.GONE); - zoomInButton.setVisibility(View.VISIBLE); + mainFab.setVisibility(View.VISIBLE); // Animate the four positioning/sizing properties in parallel, // back to their original values. diff --git a/app/src/main/res/drawable/ic_keyboard_arrow_up_black_24dp.xml b/app/src/main/res/drawable/ic_keyboard_arrow_up_black_24dp.xml new file mode 100644 index 000000000..bc010396b --- /dev/null +++ b/app/src/main/res/drawable/ic_keyboard_arrow_up_black_24dp.xml @@ -0,0 +1,5 @@ + + + diff --git a/app/src/main/res/layout/activity_share.xml b/app/src/main/res/layout/activity_share.xml index ca8097495..b6e523239 100644 --- a/app/src/main/res/layout/activity_share.xml +++ b/app/src/main/res/layout/activity_share.xml @@ -41,8 +41,6 @@ - - + + + + + + + 20sp 16sp 14sp + 15dp + 25dp diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 3e83fefd1..00bfafdad 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -275,4 +275,6 @@ Error occurred while loading images. Uploaded by: %1$s Share App + Coordinates were not specified during image selection + From 2a98bd21ceff3241f2a4e4515ed56753be784415 Mon Sep 17 00:00:00 2001 From: neslihanturan Date: Mon, 7 May 2018 15:05:20 +0300 Subject: [PATCH 4/6] Add nearby tutorial (#1467) * Add dependency for MaterialShowcase * Add actionview class to get a reference to material showcase * Create a NearbyMaterialShowcaseSequence class * Apply sequence steps * Add first three steps of nearby showcase * Add sequence id constants to make sure they will be displayed only once * Add last step of sequence to explain plus fab * Create an object to prevent customize all sequences every time * Fix typo * Code cleanup * Add strings to strings.xml * Code cleanup * Revert irrelevant change * Revert irrelevant change * Remove showcaseview for recenter button * Use single showcaseView instead of sequence * Add single showcase view insted of sequence to be able to edit text style * Make sure it will be displayed only once * Cleanup * Update strings * Change dismiss text style --- app/build.gradle | 2 + .../nrw/commons/nearby/NearbyActivity.java | 91 ++++++++++++++++++- .../nrw/commons/nearby/NearbyMapFragment.java | 32 ++++++- .../NearbyMaterialShowcaseSequence.java | 18 ++++ .../fr/free/nrw/commons/utils/ViewUtil.java | 4 + app/src/main/res/values/strings.xml | 6 ++ 6 files changed, 150 insertions(+), 3 deletions(-) create mode 100644 app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java diff --git a/app/build.gradle b/app/build.gradle index 9c6f62fd4..5d37f8f54 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -25,6 +25,8 @@ dependencies { transitive=true } + implementation "com.github.deano2390:MaterialShowcaseView:1.2.0" + implementation "com.android.support:support-v4:$SUPPORT_LIB_VERSION" implementation "com.android.support:appcompat-v7:$SUPPORT_LIB_VERSION" implementation "com.android.support:design:$SUPPORT_LIB_VERSION" diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java index 2a423de93..3bf8beacf 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyActivity.java @@ -4,14 +4,18 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.content.SharedPreferences; import android.content.pm.PackageManager; +import android.graphics.Typeface; import android.net.Uri; import android.os.Build; import android.os.Bundle; +import android.os.Handler; import android.support.annotation.NonNull; import android.support.design.widget.BottomSheetBehavior; import android.support.v4.app.FragmentTransaction; import android.support.v7.app.AlertDialog; + import android.view.Menu; import android.view.MenuInflater; import android.view.MenuItem; @@ -25,6 +29,7 @@ import com.google.gson.GsonBuilder; import java.util.List; import javax.inject.Inject; +import javax.inject.Named; import butterknife.BindView; import butterknife.ButterKnife; @@ -41,6 +46,8 @@ import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.disposables.Disposable; import io.reactivex.schedulers.Schedulers; import timber.log.Timber; +import uk.co.deanwild.materialshowcaseview.IShowcaseListener; +import uk.co.deanwild.materialshowcaseview.MaterialShowcaseView; public class NearbyActivity extends NavigationBaseActivity implements LocationUpdateListener { @@ -56,12 +63,15 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp LinearLayout bottomSheetDetails; @BindView(R.id.transparentView) View transparentView; + @BindView(R.id.fab_recenter) + View fabRecenter; @Inject LocationServiceManager locationManager; @Inject NearbyController nearbyController; - + @Inject + @Named("application_preferences") SharedPreferences applicationPrefs; private LatLng curLatLng; private Bundle bundle; private Disposable placesDisposable; @@ -72,11 +82,18 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp private NearbyListFragment nearbyListFragment; private static final String TAG_RETAINED_MAP_FRAGMENT = NearbyMapFragment.class.getSimpleName(); private static final String TAG_RETAINED_LIST_FRAGMENT = NearbyListFragment.class.getSimpleName(); + private View listButton; // Reference to list button to use in tutorial private final String NETWORK_INTENT_ACTION = "android.net.conn.CONNECTIVITY_CHANGE"; private BroadcastReceiver broadcastReceiver; + + private boolean isListShowcaseAdded = false; + private boolean isMapShowCaseAdded = false; + private LatLng lastKnownLocation; + private MaterialShowcaseView secondSingleShowCaseView; + @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -126,6 +143,39 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp MenuInflater inflater = getMenuInflater(); inflater.inflate(R.menu.menu_nearby, menu); + new Handler().post(() -> { + + listButton = findViewById(R.id.action_display_list); + + secondSingleShowCaseView = new MaterialShowcaseView.Builder(this) + .setTarget(listButton) + .setDismissText(getString(R.string.showcase_view_got_it_button)) + .setContentText(getString(R.string.showcase_view_list_icon)) + .setDelay(500) // optional but starting animations immediately in onCreate can make them choppy + .singleUse(ViewUtil.SHOWCASE_VIEW_ID_1) // provide a unique ID used to ensure it is only shown once + .setDismissStyle(Typeface.defaultFromStyle(Typeface.BOLD)) + .setListener(new IShowcaseListener() { + @Override + public void onShowcaseDisplayed(MaterialShowcaseView materialShowcaseView) { + + } + + // If dismissed, we can inform fragment to start showcase sequence there + @Override + public void onShowcaseDismissed(MaterialShowcaseView materialShowcaseView) { + nearbyMapFragment.onNearbyMaterialShowcaseDismissed(); + } + }) + .build(); + + isListShowcaseAdded = true; + + if (isMapShowCaseAdded) { // If map showcase is also ready, start ShowcaseSequence + // Probably this case is not possible. Just added to be careful + setMapViewTutorialShowCase(); + } + }); + return super.onCreateOptionsMenu(menu); } @@ -420,6 +470,45 @@ public class NearbyActivity extends NavigationBaseActivity implements LocationUp updateMapFragment(false); updateListFragment(); } + + isMapShowCaseAdded = true; + } + + public void setMapViewTutorialShowCase() { + /* + *This showcase view will be the first step of our nearbyMaterialShowcaseSequence. The reason we use a + * single item instead of adding another step to nearbyMaterialShowcaseSequence is that we are not able to + * call withoutShape() method on steps. For mapView we need an showcase view without + * any circle on it, it should cover the whole page. + * */ + MaterialShowcaseView firstSingleShowCaseView = new MaterialShowcaseView.Builder(this) + .setTarget(nearbyMapFragment.mapView) + .setDismissText(getString(R.string.showcase_view_got_it_button)) + .setContentText(getString(R.string.showcase_view_whole_nearby_activity)) + .setDelay(500) // optional but starting animations immediately in onCreate can make them choppy + .singleUse(ViewUtil.SHOWCASE_VIEW_ID_2) // provide a unique ID used to ensure it is only shown once + .withoutShape() // no shape on map view since there are no view to focus on + .setDismissStyle(Typeface.defaultFromStyle(Typeface.BOLD)) + .setListener(new IShowcaseListener() { + @Override + public void onShowcaseDisplayed(MaterialShowcaseView materialShowcaseView) { + + } + + @Override + public void onShowcaseDismissed(MaterialShowcaseView materialShowcaseView) { + /* Add other nearbyMaterialShowcaseSequence here, it will make the user feel as they are a + * nearbyMaterialShowcaseSequence whole together. + * */ + secondSingleShowCaseView.show(NearbyActivity.this); + } + }) + .build(); + + if (applicationPrefs.getBoolean("firstRunNearby", true)) { + applicationPrefs.edit().putBoolean("firstRunNearby", false).apply(); + firstSingleShowCaseView.show(this); + } } private void lockNearbyView(boolean lock) { diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java index 431132436..69041d286 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java @@ -7,6 +7,7 @@ import android.content.Intent; import android.content.SharedPreferences; import android.content.pm.PackageManager; import android.graphics.Color; +import android.graphics.Typeface; import android.net.Uri; import android.os.Bundle; import android.support.annotation.NonNull; @@ -58,13 +59,14 @@ import fr.free.nrw.commons.contributions.ContributionController; import fr.free.nrw.commons.utils.UriDeserializer; import fr.free.nrw.commons.utils.ViewUtil; import timber.log.Timber; +import uk.co.deanwild.materialshowcaseview.MaterialShowcaseView; import static android.app.Activity.RESULT_OK; import static android.content.pm.PackageManager.PERMISSION_GRANTED; public class NearbyMapFragment extends DaggerFragment { - private MapView mapView; + public MapView mapView; private List baseMarkerOptions; private fr.free.nrw.commons.location.LatLng curLatLng; public fr.free.nrw.commons.location.LatLng[] boundaryCoordinates; @@ -111,6 +113,10 @@ public class NearbyMapFragment extends DaggerFragment { private final double CAMERA_TARGET_SHIFT_FACTOR_PORTRAIT = 0.06; private final double CAMERA_TARGET_SHIFT_FACTOR_LANDSCAPE = 0.04; + private boolean isSecondMaterialShowcaseDismissed; + private boolean isMapReady; + private MaterialShowcaseView thirdSingleShowCaseView; + private Bundle bundleForUpdtes;// Carry information from activity about changed nearby places and current location @Inject @@ -163,7 +169,6 @@ public class NearbyMapFragment extends DaggerFragment { @Override public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { - Timber.d("onCreateView called"); if (curLatLng != null) { Timber.d("curLatLng found, setting up map view..."); @@ -476,6 +481,7 @@ public class NearbyMapFragment extends DaggerFragment { mapView.getMapAsync(new OnMapReadyCallback() { @Override public void onMapReady(MapboxMap mapboxMap) { + ((NearbyActivity)getActivity()).setMapViewTutorialShowCase(); NearbyMapFragment.this.mapboxMap = mapboxMap; updateMapSignificantly(); } @@ -519,6 +525,7 @@ public class NearbyMapFragment extends DaggerFragment { private void addNearbyMarkerstoMapBoxMap() { mapboxMap.addMarkers(baseMarkerOptions); + mapboxMap.setOnInfoWindowCloseListener(marker -> { if (marker == selected) { bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); @@ -534,6 +541,7 @@ public class NearbyMapFragment extends DaggerFragment { }); mapboxMap.setOnMarkerClickListener(marker -> { + if (marker instanceof NearbyMarker) { this.selected = marker; NearbyMarker nearbyMarker = (NearbyMarker) marker; @@ -541,6 +549,7 @@ public class NearbyMapFragment extends DaggerFragment { passInfoToSheet(place); bottomSheetListBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED); + } return false; }); @@ -634,7 +643,19 @@ public class NearbyMapFragment extends DaggerFragment { addAnchorToSmallFABs(fabGallery, getActivity().findViewById(R.id.empty_view).getId()); addAnchorToSmallFABs(fabCamera, getActivity().findViewById(R.id.empty_view1).getId()); + thirdSingleShowCaseView = new MaterialShowcaseView.Builder(this.getActivity()) + .setTarget(fabPlus) + .setDismissText(getString(R.string.showcase_view_got_it_button)) + .setContentText(getString(R.string.showcase_view_plus_fab)) + .setDelay(500) // optional but starting animations immediately in onCreate can make them choppy + .singleUse(ViewUtil.SHOWCASE_VIEW_ID_3) // provide a unique ID used to ensure it is only shown once + .setDismissStyle(Typeface.defaultFromStyle(Typeface.BOLD)) + .build(); + isMapReady = true; + if (isSecondMaterialShowcaseDismissed) { + thirdSingleShowCaseView.show(getActivity()); + } } @@ -791,6 +812,13 @@ public class NearbyMapFragment extends DaggerFragment { this.bundleForUpdtes = bundleForUpdtes; } + public void onNearbyMaterialShowcaseDismissed() { + isSecondMaterialShowcaseDismissed = true; + if (isMapReady) { + thirdSingleShowCaseView.show(getActivity()); + } + } + @Override public void onStart() { diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java new file mode 100644 index 000000000..c6e46611d --- /dev/null +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java @@ -0,0 +1,18 @@ +package fr.free.nrw.commons.nearby; + +import android.app.Activity; + +import uk.co.deanwild.materialshowcaseview.MaterialShowcaseSequence; +import uk.co.deanwild.materialshowcaseview.ShowcaseConfig; + + +public class NearbyMaterialShowcaseSequence extends MaterialShowcaseSequence { + + public NearbyMaterialShowcaseSequence(Activity activity, String sequenceID) { + super(activity, sequenceID); + ShowcaseConfig config = new ShowcaseConfig(); + config.setDelay(500); // half second between each showcase view + this.setConfig(config); + this.singleUse(sequenceID); // Display tutorial only once + } +} diff --git a/app/src/main/java/fr/free/nrw/commons/utils/ViewUtil.java b/app/src/main/java/fr/free/nrw/commons/utils/ViewUtil.java index 1c45e8178..0c22a40a2 100644 --- a/app/src/main/java/fr/free/nrw/commons/utils/ViewUtil.java +++ b/app/src/main/java/fr/free/nrw/commons/utils/ViewUtil.java @@ -10,6 +10,10 @@ import android.widget.Toast; public class ViewUtil { + public static final String SHOWCASE_VIEW_ID_1 = "SHOWCASE_VIEW_ID_1"; + public static final String SHOWCASE_VIEW_ID_2 = "SHOWCASE_VIEW_ID_2"; + public static final String SHOWCASE_VIEW_ID_3 = "SHOWCASE_VIEW_ID_3"; + public static void showSnackbar(View view, int messageResourceId) { Snackbar.make(view, messageResourceId, Snackbar.LENGTH_SHORT).show(); } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 00bfafdad..dcea51bcc 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -271,9 +271,15 @@ Cancel Retry + Got it! + These are the places near you that need pictures to illustrate their Wikipedia articles + Tapping this button brings up a list of these places + You can upload a picture for any place from your gallery or camera + No images found! Error occurred while loading images. Uploaded by: %1$s + Share App Coordinates were not specified during image selection From cbca264dbcae19a67ea27f7cdac226116f750b06 Mon Sep 17 00:00:00 2001 From: Kaartic Sivaraam Date: Tue, 8 May 2018 06:34:09 +0000 Subject: [PATCH 5/6] CONTRIBUTING: fix formatting of the gist of the guidelines (#1453) * CONTRIBUTING: fix formatting of the gist of the guidelines First level headings for a gist seems to be overkill. So, replace first level headings with an ordered-list which sounds more meaningful. * CONTRIBUTING: specify clearly that 'blame' is a feature of "Git" The contributing file specifies about the ability to know who wrote something without the need of @author javadoc tags but incorrectly attributes the feature to GitHub. Correctly attribute the feature to where it belongs, Git, and specify the name of the feature to help users easily take advantage of it. --- CONTRIBUTING.md | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5a8e7af19..caa02a103 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,23 +5,30 @@ If you're not sure where to start head on to [this wiki page](https://github.com Here's a gist of the guidelines, -# Make separate commits for logically separate changes +1. Make separate commits for logically separate changes -# Describe your changes well in the commit message +1. Describe your changes well in the commit message -The first line of the commit message should be a short description of what has + The first line of the commit message should be a short description of what has changed. It is also good to prefix the first line with "area: " where the "area" is a filename or identifier for the general area of the code being modified. The body should provide a meaningful commit message. -# Write Javadocs +1. Write Javadocs -We require contributors to include Javadocs for all new methods and classes submitted via PRs (after 1 May 2018). This is aimed at making it easier for new contributors to dive into our codebase, especially those who are new to Android development. A few things to note: + We require contributors to include Javadocs for all new methods and classes + submitted via PRs (after 1 May 2018). This is aimed at making it easier for + new contributors to dive into our codebase, especially those who are new to + Android development. A few things to note: -- This should not replace the need for code that is easily-readable in and of itself -- Please make sure that your Javadocs are reasonably descriptive, not just a copy of the method name -- Please do not use `@author` tags - we aim for collective code ownership, and if needed, GitHub allows us to see who wrote something without needing to add these tags + - This should not replace the need for code that is easily-readable in + and of itself + - Please make sure that your Javadocs are reasonably descriptive, not just + a copy of the method name + - Please do not use `@author` tags - we aim for collective code ownership, + and if needed, Git allows us to see who wrote something without needing + to add these tags (`git blame`) -# Write tests for your code (if possible) +1. Write tests for your code (if possible) -# Make sure the Wiki pages don't become stale by updating them (if needed) +1. Make sure the Wiki pages don't become stale by updating them (if needed) From d9600297752f8d7fa6955bd44678811d64ddc8eb Mon Sep 17 00:00:00 2001 From: Ashish Kumar Date: Tue, 8 May 2018 12:21:13 +0530 Subject: [PATCH 6/6] Feature/switch to butterknife (#1494) * Implemented butterknife in MediaDetailFragment [issue #1491] * Implemented butterknife in MediaDetailPagerFragment [[issue #1491]] * post merge upstream master wip [[issue #1491]] --- .../commons/media/MediaDetailFragment.java | 200 ++++++++++-------- .../media/MediaDetailPagerFragment.java | 7 +- 2 files changed, 112 insertions(+), 95 deletions(-) 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 c3a977ab5..9614c4f00 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 @@ -9,6 +9,7 @@ import android.os.AsyncTask; import android.os.Bundle; import android.support.annotation.Nullable; import android.text.Editable; +import android.text.TextUtils; import android.text.TextWatcher; import android.util.TypedValue; import android.view.LayoutInflater; @@ -22,6 +23,9 @@ import android.widget.ScrollView; import android.widget.TextView; import android.widget.Toast; +import butterknife.BindView; +import butterknife.ButterKnife; +import butterknife.OnClick; import java.io.IOException; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -45,7 +49,8 @@ import fr.free.nrw.commons.mwapi.MediaWikiApi; import fr.free.nrw.commons.ui.widget.CompatTextView; import timber.log.Timber; -import static android.view.View.*; +import static android.view.View.GONE; +import static android.view.View.VISIBLE; import static android.widget.Toast.LENGTH_SHORT; public class MediaDetailFragment extends CommonsDaggerSupportFragment { @@ -75,23 +80,37 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { @Inject MediaWikiApi mwApi; - - private MediaWikiImageView image; - private MediaDetailSpacer spacer; private int initialListTop = 0; - private TextView title; - private TextView desc; - private TextView author; - private TextView license; - private TextView coordinates; - private TextView uploadedDate; - private TextView seeMore; - private LinearLayout nominatedforDeletion; - private LinearLayout categoryContainer; - private LinearLayout authorLayout; - private Button delete; - private ScrollView scrollView; + @BindView(R.id.mediaDetailImage) + MediaWikiImageView image; + @BindView(R.id.mediaDetailSpacer) + MediaDetailSpacer spacer; + @BindView(R.id.mediaDetailTitle) + TextView title; + @BindView(R.id.mediaDetailDesc) + TextView desc; + @BindView(R.id.mediaDetailAuthor) + TextView author; + @BindView(R.id.mediaDetailLicense) + TextView license; + @BindView(R.id.mediaDetailCoordinates) + TextView coordinates; + @BindView(R.id.mediaDetailuploadeddate) + TextView uploadedDate; + @BindView(R.id.seeMore) + TextView seeMore; + @BindView(R.id.nominatedDeletionBanner) + LinearLayout nominatedForDeletion; + @BindView(R.id.mediaDetailCategoryContainer) + LinearLayout categoryContainer; + @BindView(R.id.authorLinearLayout) + LinearLayout authorLayout; + @BindView(R.id.nominateDeletion) + Button delete; + @BindView(R.id.mediaDetailScrollView) + ScrollView scrollView; + private ArrayList categoryNames; private boolean categoriesLoaded = false; private boolean categoriesPresent = false; @@ -101,6 +120,9 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { private AsyncTask detailFetchTask; private LicenseList licenseList; + //Had to make this class variable, to implement various onClicks, which access the media, also I fell why make separate variables when one can serve the purpose + private Media media; + @Override public void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); @@ -137,22 +159,7 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { final View view = inflater.inflate(R.layout.fragment_media_detail, container, false); - image = (MediaWikiImageView) view.findViewById(R.id.mediaDetailImage); - scrollView = (ScrollView) view.findViewById(R.id.mediaDetailScrollView); - - // Detail consists of a list view with main pane in header view, plus category list. - spacer = (MediaDetailSpacer) view.findViewById(R.id.mediaDetailSpacer); - title = (TextView) view.findViewById(R.id.mediaDetailTitle); - desc = (TextView) view.findViewById(R.id.mediaDetailDesc); - author = (TextView) view.findViewById(R.id.mediaDetailAuthor); - license = (TextView) view.findViewById(R.id.mediaDetailLicense); - coordinates = (TextView) view.findViewById(R.id.mediaDetailCoordinates); - uploadedDate = (TextView) view.findViewById(R.id.mediaDetailuploadeddate); - seeMore = (TextView) view.findViewById(R.id.seeMore); - nominatedforDeletion = (LinearLayout) view.findViewById(R.id.nominatedDeletionBanner); - delete = (Button) view.findViewById(R.id.nominateDeletion); - categoryContainer = (LinearLayout) view.findViewById(R.id.mediaDetailCategoryContainer); - authorLayout = (LinearLayout) view.findViewById(R.id.authorLinearLayout); + ButterKnife.bind(this,view); if (isFeaturedMedia){ authorLayout.setVisibility(VISIBLE); @@ -196,7 +203,7 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { @Override public void onResume() { super.onResume(); - Media media = detailProvider.getMediaAtPosition(index); + media = detailProvider.getMediaAtPosition(index); if (media == null) { // Ask the detail provider to ping us when we're ready Timber.d("MediaDetailFragment not yet ready to display details; registering observer"); @@ -209,17 +216,18 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { Timber.d("MediaDetailFragment ready to display delayed details!"); detailProvider.unregisterDataSetObserver(dataObserver); dataObserver = null; - displayMediaDetails(detailProvider.getMediaAtPosition(index)); + media=detailProvider.getMediaAtPosition(index); + displayMediaDetails(); } }; detailProvider.registerDataSetObserver(dataObserver); } else { Timber.d("MediaDetailFragment ready to display details"); - displayMediaDetails(media); + displayMediaDetails(); } } - private void displayMediaDetails(final Media media) { + private void displayMediaDetails() { //Always load image from Internet to allow viewing the desc, license, and cats image.setMedia(media); @@ -256,7 +264,6 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { if (success) { extractor.fill(media); setTextFields(media); - setOnClickListeners(media); } else { Timber.d("Failed to load photo details."); } @@ -316,74 +323,81 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { checkDeletion(media); } - private void setOnClickListeners(final Media media) { - if (licenseLink(media) != null) { - license.setOnClickListener(v -> openWebBrowser(licenseLink(media))); + @OnClick(R.id.mediaDetailLicense) + public void onMediaDetailLicenceClicked(){ + if (!TextUtils.isEmpty(licenseLink(media))) { + openWebBrowser(licenseLink(media)); } else { if(isFeaturedMedia) { - Timber.d("Unable to fetch license URL for %s", media.getLicense()); + Timber.d("Unable to fetch license URL for %s", media.getLicense()); } else { Toast toast = Toast.makeText(getContext(), getString(R.string.null_url), Toast.LENGTH_SHORT); toast.show(); } } + } + + @OnClick(R.id.mediaDetailCoordinates) + public void onMediaDetailCoordinatesClicked(){ if (media.getCoordinates() != null) { - coordinates.setOnClickListener(v -> openMap(media.getCoordinates())); + openMap(media.getCoordinates()); } - if (delete.getVisibility() == VISIBLE) { - enableDeleteButton(true); + } - delete.setOnClickListener(v -> { + @OnClick(R.id.nominateDeletion) + public void onDeleteButtonClicked(){ + //Reviewer correct me if i have misunderstood something over here + //But how does this if (delete.getVisibility() == View.VISIBLE) { + // enableDeleteButton(true); makes sense ? + AlertDialog.Builder alert = new AlertDialog.Builder(getActivity()); + alert.setMessage("Why should this file be deleted?"); + final EditText input = new EditText(getActivity()); + alert.setView(input); + input.requestFocus(); + alert.setPositiveButton(R.string.ok, new DialogInterface.OnClickListener() { + public void onClick(DialogInterface dialog, int whichButton) { + String reason = input.getText().toString(); + DeleteTask deleteTask = new DeleteTask(getActivity(), media, reason); + deleteTask.execute(); + enableDeleteButton(false); + } + }); + alert.setNegativeButton(R.string.cancel, new DialogInterface.OnClickListener() { + public void onClick(DialogInterface dialog, int whichButton) { + } + }); + AlertDialog d = alert.create(); + input.addTextChangedListener(new TextWatcher() { + private void handleText() { + final Button okButton = d.getButton(AlertDialog.BUTTON_POSITIVE); + if (input.getText().length() == 0) { + okButton.setEnabled(false); + } else { + okButton.setEnabled(true); + } + } - AlertDialog.Builder alert = new AlertDialog.Builder(getActivity()); - alert.setMessage("Why should this file be deleted?"); - final EditText input = new EditText(getActivity()); - alert.setView(input); - input.requestFocus(); - alert.setPositiveButton(R.string.ok, new DialogInterface.OnClickListener() { - public void onClick(DialogInterface dialog, int whichButton) { - String reason = input.getText().toString(); - DeleteTask deleteTask = new DeleteTask(getActivity(), media, reason); - deleteTask.execute(); - enableDeleteButton(false); - } - }); - alert.setNegativeButton(R.string.cancel, new DialogInterface.OnClickListener() { - public void onClick(DialogInterface dialog, int whichButton) { - } - }); - AlertDialog d = alert.create(); - input.addTextChangedListener(new TextWatcher() { - private void handleText() { - final Button okButton = d.getButton(AlertDialog.BUTTON_POSITIVE); - if (input.getText().length() == 0) { - okButton.setEnabled(false); - } else { - okButton.setEnabled(true); - } - } + @Override + public void afterTextChanged(Editable arg0) { + handleText(); + } - @Override - public void afterTextChanged(Editable arg0) { - handleText(); - } + @Override + public void beforeTextChanged(CharSequence s, int start, int count, int after) { + } - @Override - public void beforeTextChanged(CharSequence s, int start, int count, int after) { - } + @Override + public void onTextChanged(CharSequence s, int start, int before, int count) { + } + }); + d.show(); + d.getButton(AlertDialog.BUTTON_POSITIVE).setEnabled(false); + } - @Override - public void onTextChanged(CharSequence s, int start, int before, int count) { - } - }); - d.show(); - d.getButton(AlertDialog.BUTTON_POSITIVE).setEnabled(false); - }); - } - if (nominatedforDeletion.getVisibility() == VISIBLE){ - seeMore.setOnClickListener(v -> { - openWebBrowser(media.getFilePageTitle().getMobileUri().toString()); - }); + @OnClick(R.id.seeMore) + public void onSeeMoreClicked(){ + if(nominatedForDeletion.getVisibility()== VISIBLE) { + openWebBrowser(media.getFilePageTitle().getMobileUri().toString()); } } @@ -488,11 +502,11 @@ public class MediaDetailFragment extends CommonsDaggerSupportFragment { private void checkDeletion(Media media){ if (media.getRequestedDeletion()){ delete.setVisibility(GONE); - nominatedforDeletion.setVisibility(VISIBLE); + nominatedForDeletion.setVisibility(VISIBLE); } else{ delete.setVisibility(VISIBLE); - nominatedforDeletion.setVisibility(GONE); + nominatedForDeletion.setVisibility(GONE); } } 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 c0564c603..62d1261cf 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 @@ -26,6 +26,8 @@ import android.view.View; import android.view.ViewGroup; import android.widget.Toast; +import butterknife.BindView; +import butterknife.ButterKnife; import javax.inject.Inject; import javax.inject.Named; @@ -53,7 +55,8 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple @Named("default_preferences") SharedPreferences prefs; - private ViewPager pager; + @BindView(R.id.mediaDetailsPager) + ViewPager pager; private Boolean editable; private boolean isFeaturedImage; @@ -72,7 +75,7 @@ public class MediaDetailPagerFragment extends CommonsDaggerSupportFragment imple ViewGroup container, Bundle savedInstanceState) { View view = inflater.inflate(R.layout.fragment_media_detail_pager, container, false); - pager = (ViewPager) view.findViewById(R.id.mediaDetailsPager); + ButterKnife.bind(this,view); pager.addOnPageChangeListener(this); final MediaDetailAdapter adapter = new MediaDetailAdapter(getChildFragmentManager());