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

Implemented media/photo share functionality #3284

Merged
merged 15 commits into from
Sep 5, 2023
Merged

Conversation

Smarshal21
Copy link
Member

@Smarshal21 Smarshal21 commented Aug 28, 2023

Resolves #3276

Successfully Implemented media/photo share functionality on long-pressing a photograph that was shared in a Talk conversation.
This would offer a simpler workflow: no need to expand a photograph full-screen just to share it.
As there is arguably plenty of screen real-estate to accommodate another function in the long-press menu.

🖼️ Screenshots

🏚️ Before
before

🏡 After
after

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

@Smarshal21
Copy link
Member Author

@AndyScherzinger @mahibi Ptal and let me know the changes
will create the branches within this repository from the next PR 😀

@AndyScherzinger
Copy link
Member

@Smarshal21 did you or can you test this for images you opened first by clicking on them (triggers a download of the original image) and for not clicking on the image first but going straight for the long press (only the preview image available) since I expect you'd be sharing a different file then or it might not be working on both scenarios.

@Smarshal21
Copy link
Member Author

Smarshal21 commented Aug 28, 2023

@Smarshal21 did you or can you test this for images you opened first by clicking on them (triggers a download of the original image) and for not clicking on the image first but going straight for the long press (only the preview image available) since I expect you'd be sharing a different file then or it might not be working on both scenarios.

Yeah i tested them for images and it works completely fine

@mahibi
Copy link
Collaborator

mahibi commented Aug 30, 2023

@Smarshal21 did you or can you test this for images you opened first by clicking on them (triggers a download of the original image) and for not clicking on the image first but going straight for the long press (only the preview image available) since I expect you'd be sharing a different file then or it might not be working on both scenarios.

it doesn't work when the file is not downloaded. So it only works for images that were opened (downloaded) before.

To test:

  • send some image to the user who is logged in at the talk android app.
  • DON'T open this file and share it from context menu (e.g. to some other messenger, e.g. whatsapp)
  • whatsapp will complain with "the file type is not supported"
  • back in nextcloud talk, open the file and go back
  • share it from context menu again
  • this time it will work

So there should be checked if the file is already downloaded and if it's not, the download should be triggered. When download is finished, the share method should be triggered.

@mahibi mahibi self-requested a review August 30, 2023 09:39
@Smarshal21
Copy link
Member Author

Sure, will take a look at it

@Smarshal21
Copy link
Member Author

@Smarshal21 did you or can you test this for images you opened first by clicking on them (triggers a download of the original image) and for not clicking on the image first but going straight for the long press (only the preview image available) since I expect you'd be sharing a different file then or it might not be working on both scenarios.

it doesn't work when the file is not downloaded. So it only works for images that were opened (downloaded) before.

To test:

  • send some image to the user who is logged in at the talk android app.
  • DON'T open this file and share it from context menu (e.g. to some other messenger, e.g. whatsapp)
  • whatsapp will complain with "the file type is not supported"
  • back in nextcloud talk, open the file and go back
  • share it from context menu again
  • this time it will work

So there should be checked if the file is already downloaded and if it's not, the download should be triggered. When download is finished, the share method should be triggered.

@mahibi i have changed , could you please let me know if the error is still occurring as i am getting some error in joining a chat from another user (some sort of issue).

@mahibi
Copy link
Collaborator

mahibi commented Sep 1, 2023

the error is still occurring.
There must be logic added to actually download the file first (in the chat, it's only a preview if it was never shown fullscreen). Just have a look how it is done for the playback of voice messages:

        adapter?.registerViewClickListener(
            R.id.playPauseBtn
        ) { _, message ->
            val filename = message.selectedIndividualHashMap!!["name"]
            val file = File(context.cacheDir, filename!!)
            if (file.exists()) {
                if (message.isPlayingVoiceMessage) {
                    pausePlayback(message)
                } else {
                    setUpWaveform(message)
                }
            } else {
                Log.d(TAG, "Downloaded to cache")
                downloadFileToCache(message)
            }
        }

so for sharing the logic should be similar. If the file exists, just continue the the sharing code. If it doesn't exist yet, start the download. When download is finished, come back the share logic.
In the downloadFileToCache method, you will find

        WorkManager.getInstance(context).getWorkInfoByIdLiveData(downloadWorker.id)
            .observeForever { workInfo: WorkInfo ->
                if (workInfo.state == WorkInfo.State.SUCCEEDED) {
                    setUpWaveform(message)
                }
            }

There should be added a condition to check what was downloaded and then continue with the voice playback or the sharing code.

Just let me know if you have more questions.
Or if you don't find the time just let me know if i should take over.

as i am getting some error in joining a chat from another user (some sort of issue)
What is this error? if it's not related to the feature just ping me 1:1 in chat

@Smarshal21
Copy link
Member Author

@mahibi @AndyScherzinger ptal i have updated and it works for me

@mahibi
Copy link
Collaborator

mahibi commented Sep 4, 2023

@mahibi @AndyScherzinger ptal i have updated and it works for me

reviewing.. i will commit some suggested changes soon

When sharing a file by context menu that is not downloaded yet, do not open it after download but just share it. This is done by 'openWhenDownloaded' variable in chatMessage.

Pass a method to downloadFileToCache, so it's more flexible what to do when download finished.

Add some minor changes
@mahibi
Copy link
Collaborator

mahibi commented Sep 4, 2023

@mahibi @AndyScherzinger ptal i have updated and it works for me

i commited some suggested changes, see commit message of abb3389

@Smarshal21
Copy link
Member Author

@mahibi @AndyScherzinger ptal i have updated and it works for me

i commited some suggested changes, see commit message of abb3389

is there anything to be implemented ?

@mahibi mahibi merged commit f172fcc into nextcloud:master Sep 5, 2023
10 of 13 checks passed
@mahibi mahibi added this to the 17.2.0 milestone Sep 5, 2023
Copy link
Contributor

github-actions bot commented Apr 2, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

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

Successfully merging this pull request may close these issues.

media/photo share function - add to long-press menu
3 participants