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

Correctly share image via share menu item #36

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

avitaker
Copy link
Contributor

@avitaker avitaker commented Apr 10, 2017

Solves issue #30. Now added some documentation, removed some generated files and hardcoded strings.

Previously, there were permission problems from trying to share the image to image sharing apps, according to the issue tracker. The following changes were made:
FileProvider class is defined in AndroidManifest, along with the required meta-data and associated files (a string was added to strings.xml for the provider authority). This is apparently required for sharing streams in Android M and above.
Temporary file of the image is written to keep track of exact file location, as well as have a temporary cache of previously shared images. This is done in an AsyncTask that is defined at the bottom of SubjectFragment and executed in the updateShareIntent method.
In devices with Android M or above, explicit permission to write and read images is obtained if not already provided.
The FileProvider class is used to create the URI that will eventually be sent in the intent.
An intent flag is added for purposes of magic (I'm not really sure what it does, but it works).

In the limited use case of trying to share images, this code works. Additional changes might be required to distinguish text from image intents to share, because the code right now will set the share intent to an image type if mUriImageRemote is not null.

murraycu and others added 4 commits March 16, 2017 10:23
Reorganize this so we could have multiple loaders.
Solves issue murraycu#30.

Previously, there were permission problems from trying to share the image to image sharing apps, according to the issue tracker. The following changes were made:
FileProvider class is defined in AndroidManifest, along with the required meta-data and associated files (a string was added to strings.xml for the provider authority). This is apparently required for sharing streams in Android M and above.
Temporary file of the image is written to keep track of exact file location, as well as have a temporary cache of previously shared images. This is done in an AsyncTask that is defined at the bottom of SubjectFragment and executed in the updateShareIntent method.
In devices with Android M or above, explicit permission to write and read images is obtained if not already provided.
The FileProvider class is used to create the URI that will eventually be sent in the intent.
An intent flag is added for purposes of magic (I'm not really sure what it does, but it works).

In the limited use case of trying to share images, this code works. Additional changes might be required to distinguish text from image intents to share, because the code right now will set the share intent to an image type if mUriImageRemote is not null.
Solves issue murraycu#30. Now added some documentation, removed some generated files and hardcoded strings.

Previously, there were permission problems from trying to share the image to image sharing apps, according to the issue tracker. The following changes were made:
FileProvider class is defined in AndroidManifest, along with the required meta-data and associated files (a string was added to strings.xml for the provider authority). This is apparently required for sharing streams in Android M and above.
Temporary file of the image is written to keep track of exact file location, as well as have a temporary cache of previously shared images. This is done in an AsyncTask that is defined at the bottom of SubjectFragment and executed in the updateShareIntent method.
In devices with Android M or above, explicit permission to write and read images is obtained if not already provided.
The FileProvider class is used to create the URI that will eventually be sent in the intent.
An intent flag is added for purposes of magic (I'm not really sure what it does, but it works).

In the limited use case of trying to share images, this code works. Additional changes might be required to distinguish text from image intents to share, because the code right now will set the share intent to an image type if mUriImageRemote is not null.
Solves issue murraycu#30. Now added some documentation, removed some generated files and hardcoded strings.

Previously, there were permission problems from trying to share the image to image sharing apps, according to the issue tracker. The following changes were made:
FileProvider class is defined in AndroidManifest, along with the required meta-data and associated files (a string was added to strings.xml for the provider authority). This is apparently required for sharing streams in Android M and above.
Temporary file of the image is written to keep track of exact file location, as well as have a temporary cache of previously shared images. This is done in an AsyncTask that is defined at the bottom of SubjectFragment and executed in the updateShareIntent method.
In devices with Android M or above, explicit permission to write and read images is obtained if not already provided.
The FileProvider class is used to create the URI that will eventually be sent in the intent.
An intent flag is added for purposes of magic (I'm not really sure what it does, but it works).

In the limited use case of trying to share images, this code works. Additional changes might be required to distinguish text from image intents to share, because the code right now will set the share intent to an image type if mUriImageRemote is not null.
@murraycu
Copy link
Owner

Please squash these into one commit.

@murraycu
Copy link
Owner

Could you do that, please?

@avitaker
Copy link
Contributor Author

Absolutely. Sorry, I've been busy recently but I'll try to squash the changes into one commit within the next 24 hours.

avitaker added 2 commits May 13, 2017 16:19
commit 88c6d4d
Author: Avinash David <[email protected]>
Date:   Sun Apr 9 23:44:16 2017 -0500

    feat: share image directly via share menu item

    Solves issue murraycu#30. Now added some documentation, removed some generated files and hardcoded strings.

    Previously, there were permission problems from trying to share the image to image sharing apps, according to the issue tracker. The following changes were made:
    FileProvider class is defined in AndroidManifest, along with the required meta-data and associated files (a string was added to strings.xml for the provider authority). This is apparently required for sharing streams in Android M and above.
    Temporary file of the image is written to keep track of exact file location, as well as have a temporary cache of previously shared images. This is done in an AsyncTask that is defined at the bottom of SubjectFragment and executed in the updateShareIntent method.
    In devices with Android M or above, explicit permission to write and read images is obtained if not already provided.
    The FileProvider class is used to create the URI that will eventually be sent in the intent.
    An intent flag is added for purposes of magic (I'm not really sure what it does, but it works).

    In the limited use case of trying to share images, this code works. Additional changes might be required to distinguish text from image intents to share, because the code right now will set the share intent to an image type if mUriImageRemote is not null.

commit 8d31c99
Author: Avinash David <[email protected]>
Date:   Fri Apr 7 10:17:09 2017 -0500

    feat: share image directly via share menu item

    Solves issue murraycu#30. Now added some documentation, removed some generated files and hardcoded strings.

    Previously, there were permission problems from trying to share the image to image sharing apps, according to the issue tracker. The following changes were made:
    FileProvider class is defined in AndroidManifest, along with the required meta-data and associated files (a string was added to strings.xml for the provider authority). This is apparently required for sharing streams in Android M and above.
    Temporary file of the image is written to keep track of exact file location, as well as have a temporary cache of previously shared images. This is done in an AsyncTask that is defined at the bottom of SubjectFragment and executed in the updateShareIntent method.
    In devices with Android M or above, explicit permission to write and read images is obtained if not already provided.
    The FileProvider class is used to create the URI that will eventually be sent in the intent.
    An intent flag is added for purposes of magic (I'm not really sure what it does, but it works).

    In the limited use case of trying to share images, this code works. Additional changes might be required to distinguish text from image intents to share, because the code right now will set the share intent to an image type if mUriImageRemote is not null.

commit 6c52efc
Author: Avinash David <[email protected]>
Date:   Thu Apr 6 15:41:45 2017 -0500

    feat: share image directly via share menu item

    Solves issue murraycu#30.

    Previously, there were permission problems from trying to share the image to image sharing apps, according to the issue tracker. The following changes were made:
    FileProvider class is defined in AndroidManifest, along with the required meta-data and associated files (a string was added to strings.xml for the provider authority). This is apparently required for sharing streams in Android M and above.
    Temporary file of the image is written to keep track of exact file location, as well as have a temporary cache of previously shared images. This is done in an AsyncTask that is defined at the bottom of SubjectFragment and executed in the updateShareIntent method.
    In devices with Android M or above, explicit permission to write and read images is obtained if not already provided.
    The FileProvider class is used to create the URI that will eventually be sent in the intent.
    An intent flag is added for purposes of magic (I'm not really sure what it does, but it works).

    In the limited use case of trying to share images, this code works. Additional changes might be required to distinguish text from image intents to share, because the code right now will set the share intent to an image type if mUriImageRemote is not null.
@avitaker
Copy link
Contributor Author

Hey, I've attempted to squash the commits into one, but they're still showing as 6 commits ahead on my branch. I'm not sure if that's what you're looking for.

@murraycu
Copy link
Owner

Thanks, but this is indeed a mess. I will try to pick the useful parts out of this.

@murraycu murraycu changed the title master Correctly share image via share menu item May 19, 2017
@murraycu
Copy link
Owner

OK. I have now pushed a commit with your changes, including some minor cleanup.

To do that, I did this.

  • Checked out your master branch from here: https://github.com/avitaker/android-galaxyzoo/commits/master
  • git reset to before your merge commit.
  • squashed your commits (I think, but maybe that wasn't necessary after removing the merge).
  • Removed the duplicate blocks of text from your commit message.
  • Rewrote your commit message for clarity.
  • Cherry-picked the commit to my master branch, and pushed it.

Thanks.

@murraycu
Copy link
Owner

This is the commit: 863c54e

@avitaker
Copy link
Contributor Author

Alright, thank you for cleaning it up for me, I'm actually not very experienced with git. Glad we could work together to fix that issue, and I'll rebase to your master

@murraycu
Copy link
Owner

If in doubt, I create a new branch and cherry-pick commits into. If even that doesn't work then I "git format-patch HEAD~10" and apply the patches manually.

I "git rebase -i HEAD~10" to squash commits together.

@murraycu
Copy link
Owner

For now, I have reverted this change:
5720d6f
due to this problem:
#30 (comment)

We should push the change again when we have resolved this problem, maybe with an extra commit. Here is the commit message for the original commit, which we could reuse:
863c54e

@avitaker
Copy link
Contributor Author

avitaker commented Jun 27, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants