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

🖼️ ISSUE-119 Share Note as image #269

Conversation

ch8n
Copy link
Contributor

@ch8n ch8n commented Oct 16, 2021

Summary

Added share Note as Image feature #119

Description for the changelog

  • added Share Drop Down
  • Modified Share Action behavior
  • Added CaptureBitmap Composable
  • CaptureBitmap returns a callback to get latest composable bitmap
  • Added bitmap sharing feature
  • Updated manifest to write Bitmap to shared space

Preview

WhatsApp.Video.2021-10-16.at.2.27.55.PM.mp4

Checklist

  • Build and linting is passing.
  • This change is not breaking existing flow of a system.
  • I have written test case for this change
    • Manually tested
  • This change is tested from all aspects.
    • Manually tested for short and long note details
  • Implemented any new third-party library (Not applicable)

@ch8n ch8n changed the title Issue 119 note as image 🖼️ ISSUE-119 Share Note as image Oct 16, 2021
@PatilShreyas
Copy link
Owner

Thanks for adding output media @ch8n 😃

Comment on lines +33 to +41
<provider
android:name="androidx.core.content.FileProvider"
android:authorities="com.ch8n.fileprovider"
android:exported="false"
android:grantUriPermissions="true">
<meta-data
android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/provider_paths" />
</provider>
Copy link
Owner

Choose a reason for hiding this comment

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

@ch8n I guess we don't need a provider since we are just sharing images from intent.

Copy link
Owner

Choose a reason for hiding this comment

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

means in simpleapp module, we didn't use this. So can we make it common across both?

Comment on lines +1 to +9
<?xml version="1.0" encoding="UTF-8"?>
<module type="JAVA_MODULE" version="4">
<component name="NewModuleRootManager" inherit-compiler-output="true">
<exclude-output />
<content url="file://$MODULE_DIR$" />
<orderEntry type="inheritedJdk" />
<orderEntry type="sourceFolder" forTests="false" />
</component>
</module>
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 just remove this file change from git?

Comment on lines +145 to +161
suspend fun saveImage(image: Bitmap, context: Context): Uri? =
withContext(Dispatchers.IO) {
val imagesFolder = File(context.cacheDir, "images")
var uri: Uri? = null
try {
imagesFolder.mkdirs()
val file = File(imagesFolder, "shared_image.png")
val stream = FileOutputStream(file)
image.compress(Bitmap.CompressFormat.PNG, 90, stream)
stream.flush()
stream.close()
uri = FileProvider.getUriForFile(context, "com.ch8n.fileprovider", file)
} catch (e: IOException) {
Log.d("Error", "IOException while trying to write file for sharing: " + e.message)
}
uri
}
Copy link
Owner

Choose a reason for hiding this comment

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

@ch8n we have common utility for this
https://github.com/PatilShreyas/NotyKT/blob/master/noty-android/app/src/main/java/dev/shreyaspatil/noty/utils/FileUtils.kt

So we can remove this. Also, we need to handle storage permission


<provider
android:name="androidx.core.content.FileProvider"
android:authorities="com.ch8n.fileprovider"
Copy link
Owner

Choose a reason for hiding this comment

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

Let's make it app-specific:

Suggested change
android:authorities="com.ch8n.fileprovider"
android:authorities="${applicationId}.FileProvider"

@PatilShreyas PatilShreyas added this to In progress in NotyKT via automation Oct 16, 2021
Comment on lines +55 to +58
@Composable
fun CaptureBitmap(
content: @Composable () -> Unit,
): () -> Bitmap {
Copy link
Owner

Choose a reason for hiding this comment

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

Generally composable function (especially when holding UI) should not return a value but this is an exceptional case I guess where we need action based returned value. Nice! 😃

@PatilShreyas PatilShreyas changed the base branch from master to feature/compose/share_image October 16, 2021 13:29
NotyKT automation moved this from In progress to Reviewer approved Oct 16, 2021
Copy link
Owner

@PatilShreyas PatilShreyas left a comment

Choose a reason for hiding this comment

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

LGTM 😃 @ch8n. Thank you so much for this awesome PR. Merging this in feature branch, will do some small improvements and refactoring. Will be targetting this change for upcoming release. Feel free to contribute again 😉

@PatilShreyas PatilShreyas merged commit 7c37d00 into PatilShreyas:feature/compose/share_image Oct 16, 2021
NotyKT automation moved this from Reviewer approved to Done Oct 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants