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

Add more context menu options to media gallery screen #350

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mehmedalijaK
Copy link
Collaborator

No description provided.

Copy link
Contributor

@AleksandarIlic AleksandarIlic left a comment

Choose a reason for hiding this comment

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

Looks really good! Just few concerns regarding how we copy the image to clipboard.


val inputStream = connection.getInputStream()
val file = File(
context.getExternalFilesDir(Environment.DIRECTORY_PICTURES),
Copy link
Contributor

Choose a reason for hiding this comment

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

We should put in a cache folder instead. This will make it visible in the gallery app when you copy something.

val inputStream = connection.getInputStream()
val file = File(
context.getExternalFilesDir(Environment.DIRECTORY_PICTURES),
"copied_image.png",
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should not use an extension here since you don't know if it's going to be a png or something else.

@@ -13,3 +24,45 @@ fun copyText(
val clip = ClipData.newPlainText(label, text)
clipboard.setPrimaryClip(clip)
}

suspend fun copyImageToClipboard(context: Context, imageUrl: String) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this is the best way to handle this situation. We already have a loaded image through Coil.

Maybe we can take the already rendered Bitmap and store it in a cache folder of the app. Then use that file to expose Uri with FileProvider.getUriForFile as you did below.

@@ -162,6 +167,16 @@ fun MediaGalleryScreen(
onSaveClick = {
currentImage()?.let { eventPublisher(MediaGalleryContract.UiEvent.SaveMedia(it)) }
},
onMediaUrlCopyClick = {
currentImage()?.url?.let { copyText(context = context, text = it) }
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably copy the original url of the image and not the primal cdn.

To which url is currentImage()?.url pointing to? Is it original or cdn link?

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