added temporary voice messages #1874
Conversation
DanielStandfest
force-pushed
the
better-voice-message-handling
branch
3 times, most recently
from
c44beda
to
dd3e20b
Compare
|
I am not sure why the localizable change is not passing the test? :/ |
Interesting, seems to be a sorting issue. I’ll take a look. |
So I tried multiple things yesterday and it only happens if you rebase on master (that is what CI does and that's why you can't seem to trigger that change locally in your branch). I have the same issue in #1880. I couldn't fully figure out what's triggering this, even rebasing on old commits did not change that. So for now I'd say we ignore it and take a look later when the PR is merged. We are going to have a closer look to this and your other PR this week :) |
|
|
||
| let referenceId = "temp-\(Date().timeIntervalSince1970 * 1000)" | ||
| temporaryMessage.referenceId = NCUtils.sha1(fromString: referenceId) | ||
| temporaryMessage.internalId = referenceId | ||
| temporaryMessage.isTemporary = true | ||
| temporaryMessage.isOfflineMessage = true |
There was a problem hiding this comment.
isOfflineMessage indicates that this message was sent without an internet connection and should be resend, when connection is available again. So this shouldn't be added here.
There's quite some logic involved (including trying to resend in the background). Check out e.g.
talk-ios/NextcloudTalk/NCChatController.m
Line 685 in dd3e20b
There was a problem hiding this comment.
Thank you for the information, I recognized that the context menu fits better when using the isOfflineMessage = true. That's why I set it to true.
For example the "open in nextcloud" (shouldn't really be an option), "copy" and "create reminder" option dont work out of the box.
I removed it now, should we fix some of the options like "delete" then or hide some of the invalid options?
| "fileLocalPath": messageParameters | ||
| ] | ||
|
|
||
| messageParametersDict[parameterId] = fileParameterDict |
There was a problem hiding this comment.
Why using a UUID here? Should it be the file dictionary? I don't see it used anywhere? (Maybe I missed it, not yet through everything).
There was a problem hiding this comment.
Good question, I guess I had a problem with the file parameter not being set when putting the fileParameters in no dictionary.
What's your suggestion?
|
Hello there, 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! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
related to #1261 Signed-off-by: Daniel Standfest <[email protected]>
DanielStandfest
force-pushed
the
better-voice-message-handling
branch
from
dd3e20b
to
c7ef13c
Compare
added support for temporary voice messages and handling them just like text messages.
related to #1261
Would be happy to get feedback! :)
Cheers,
Daniel