Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: share image directly via share menu item #35

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 7 additions & 17 deletions app/app.iml
Original file line number Diff line number Diff line change
Expand Up @@ -63,41 +63,31 @@
<sourceFolder url="file://$MODULE_DIR$/src/main/java" isTestSource="false" />
<sourceFolder url="file://$MODULE_DIR$/src/main/rs" isTestSource="false" />
<sourceFolder url="file://$MODULE_DIR$/src/main/shaders" isTestSource="false" />
<sourceFolder url="file://$MODULE_DIR$/src/test/res" type="java-test-resource" />
<sourceFolder url="file://$MODULE_DIR$/src/test/resources" type="java-test-resource" />
<sourceFolder url="file://$MODULE_DIR$/src/test/assets" type="java-test-resource" />
<sourceFolder url="file://$MODULE_DIR$/src/test/aidl" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/test/java" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/test/rs" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/test/shaders" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/androidTest/res" type="java-test-resource" />
<sourceFolder url="file://$MODULE_DIR$/src/androidTest/resources" type="java-test-resource" />
<sourceFolder url="file://$MODULE_DIR$/src/androidTest/assets" type="java-test-resource" />
<sourceFolder url="file://$MODULE_DIR$/src/androidTest/aidl" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/androidTest/java" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/androidTest/rs" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/androidTest/shaders" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/test/res" type="java-test-resource" />
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please remove these .iml file changes from the commit? I do update them every now and then, but I try not to pollute the real changes with these changes to generated files.

<sourceFolder url="file://$MODULE_DIR$/src/test/resources" type="java-test-resource" />
<sourceFolder url="file://$MODULE_DIR$/src/test/assets" type="java-test-resource" />
<sourceFolder url="file://$MODULE_DIR$/src/test/aidl" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/test/java" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/test/rs" isTestSource="true" />
<sourceFolder url="file://$MODULE_DIR$/src/test/shaders" isTestSource="true" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/assets" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/blame" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/classes" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/dependency-cache" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/incremental" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/incremental-classes" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/incremental-runtime-classes" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/incremental-safeguard" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/incremental-verifier" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/instant-run-resources" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/instant-run-support" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/jniLibs" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/manifests" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/pre-dexed" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/proguard-rules" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/reload-dex" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/res" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/restart-dex" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/rs" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/shaders" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/sourceFolderJavaResources" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/symbols" />
<excludeFolder url="file://$MODULE_DIR$/build/intermediates/transforms" />
<excludeFolder url="file://$MODULE_DIR$/build/outputs" />
Expand Down
10 changes: 10 additions & 0 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,16 @@
<meta-data android:name="android.content.SyncAdapter"
android:resource="@xml/syncadapter" />
</service>

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this have a comment saying why it's here, please. For instance, saying that it's here for this particular feature, and what would happen if it wasn't here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

<provider
android:name="android.support.v4.content.FileProvider"
android:authorities="@string/authority_fileprovider"
android:exported="false"
android:grantUriPermissions="true">
<meta-data
android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/provider_paths"/>
</provider>
</application>

</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import android.database.Cursor;
import android.net.Uri;
import android.os.Bundle;
import android.support.annotation.Nullable;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt that this change needs to be in the commit.

import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentManager;
import android.support.v4.app.FragmentTransaction;
Expand Down
67 changes: 67 additions & 0 deletions app/src/main/java/com/murrayc/galaxyzoo/app/SubjectFragment.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,17 @@
import android.app.DownloadManager;
import android.content.Context;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.database.Cursor;
import android.graphics.Bitmap;
import android.net.Uri;
import android.os.AsyncTask;
import android.os.Bundle;
import android.os.Environment;
import android.support.v4.app.ActivityCompat;
import android.support.v4.app.LoaderManager;
import android.support.v4.content.CursorLoader;
import android.support.v4.content.FileProvider;
import android.support.v4.content.Loader;
import android.support.v4.view.MenuItemCompat;
import android.support.v7.widget.ShareActionProvider;
Expand All @@ -45,6 +51,11 @@
import com.squareup.picasso.Callback;
import com.squareup.picasso.Picasso;

import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;

/**
* A fragment representing a single subject.
* This fragment is either contained in a {@link ListActivity}
Expand Down Expand Up @@ -427,7 +438,63 @@ private void updateShareActionIntent() {
//shareIntent.putExtra(Intent.EXTRA_STREAM, mUriImageStandard);
//shareIntent.setType("image/*");

if (mUriStandardRemote!=null) {
GetImageBitmapAsyncTask getImageBitmapAsyncTask = new GetImageBitmapAsyncTask(){
@Override
protected void onPostExecute(Uri uri) {
shareIntent.setType("image/*");
shareIntent.putExtra(Intent.EXTRA_STREAM, uri);
shareIntent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
mShareActionProvider.setShareIntent(shareIntent);
}
};
getImageBitmapAsyncTask.execute(mUriStandardRemote);
}


mShareActionProvider.setShareIntent(shareIntent);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this (now second) setShareIntent()? Maybe the whole instantiation of the shareIntent should move into onPostExecute()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second setShareIntent is set after the AsyncTask has started executing, but before it is completed. When the AsyncTask completes, the share intent is updated with the image type and content URI. I had left it there to provide a temporary intent on slower devices or in edge cases where the AsyncTask doesn't complete before the user decides to share, but it can indeed be removed. I will remove it in the next commit.

I would still leave the instantiation outside onPostExecute, since that leaves less main thread work to be done after the AsyncTask is completed.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would still leave the instantiation outside onPostExecute, since that leaves less main thread work to be done
after the AsyncTask is completed.

OK. That sounds reasonable, but it would be nice to have a comment explaining that choice.

}

private static final int REQUEST_EXTERNAL_STORAGE = 1;
private static String[] PERMISSIONS_STORAGE = {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the constants here instead of hard-coding these strings? This seems to be suitable: https://developer.android.com/reference/android/Manifest.permission.html#WRITE_EXTERNAL_STORAGE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

"android.permission.WRITE_EXTERNAL_STORAGE",
"android.permission.READ_EXTERNAL_STORAGE"
};

public static void verifyStoragePermissions(Activity activity) {
int permission = ActivityCompat.checkSelfPermission(activity, "android.permission.WRITE_EXTERNAL_STORAGE");

if (permission != PackageManager.PERMISSION_GRANTED) {
ActivityCompat.requestPermissions(
activity,
PERMISSIONS_STORAGE,
REQUEST_EXTERNAL_STORAGE
);
}
}

private class GetImageBitmapAsyncTask extends AsyncTask<String,Integer,Uri> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spaces after the commas would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, I have added spaces

@Override
protected Uri doInBackground(String... params) {
try {
Bitmap image = Picasso.with(getContext()).load(params[0]).get();
ByteArrayOutputStream bytes = new ByteArrayOutputStream();
image.compress(Bitmap.CompressFormat.JPEG, 100, bytes);

//TODO: PROVIDE A MORE DESCRIPTIVE FILENAME HERE IF PLAUSIBLE. Filename will be visible when sharing via certain apps like Gmail etc.
String filename = "galaxy_zoo_image.jpg";

String pathname = Environment.getExternalStorageDirectory() + File.separator + filename;
File f = new File(pathname);
f.createNewFile();
FileOutputStream fo = new FileOutputStream(f);
fo.write(bytes.toByteArray());
return FileProvider.getUriForFile(getActivity(), getString(R.string.authority_fileprovider), f);
} catch (IOException e) {
verifyStoragePermissions(getActivity());
e.printStackTrace();
}
return null;
}
}
}
3 changes: 3 additions & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@
//when specifying a transition when starting an activity that contains this SubjectFragment.
<string name="transition_subject_image" translatable="false">transition_subject_image</string>

<!--share image strings-->
<string name="authority_fileprovider">com.murrayc.galaxyzoo.app.fileprovider</string>

<!-- The possible numbers of items that may be downloaded in advance before they are needed. -->
<string-array name="pref_cache_size_entries">
<item>5 subjects</item>
Expand Down