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

Show a temporary messages when sending and keep it on error #1259

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented May 17, 2021

Success (bonus weird scrolling) Error case (with copy/paste option)
Peek 2021-05-17 12-00 Peek 2021-05-17 12-01

I have no clue why the scrolling freaks out like this.

Code changes to help testing

Modify your Talk install to make the response slower:

diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php
index f8e0f19a6..e71bd1d81 100644
--- a/lib/Controller/ChatController.php
+++ b/lib/Controller/ChatController.php
@@ -226,6 +226,7 @@ class ChatController extends AEnvironmentAwareController {
         *         found".
         */
        public function sendMessage(string $message, string $actorDisplayName = '', string $referenceId = '', int $replyTo = 0): DataResponse {
+               sleep(1);
                [$actorType, $actorId] = $this->getActorInfo($actorDisplayName);
                if (!$actorId) {
                        return new DataResponse([], Http::STATUS_NOT_FOUND);

And use the follow change to create a slow error

diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php
index f8e0f19a6..e71bd1d81 100644
--- a/lib/Controller/ChatController.php
+++ b/lib/Controller/ChatController.php
@@ -226,6 +226,8 @@ class ChatController extends AEnvironmentAwareController {
         *         found".
         */
        public function sendMessage(string $message, string $actorDisplayName = '', string $referenceId = '', int $replyTo = 0): DataResponse {
+               sleep(1);
+               return new DataResponse([], Http::STATUS_BAD_REQUEST);
                [$actorType, $actorId] = $this->getActorInfo($actorDisplayName);
                if (!$actorId) {
                        return new DataResponse([], Http::STATUS_NOT_FOUND);

Follow up todos

  • Fix scrolling (I have no idea where it goes wild)
  • Messages can have a reference id which should be set and the temporary messages only be removed by the fetching of messages when there is a reference id match.

Fix #838

@mahibi
Copy link
Collaborator

mahibi commented May 17, 2022

wow that's on old one. i now rebased on master but didn't have a closer look what needs to be done here

@AndyScherzinger AndyScherzinger force-pushed the feature/noid/temporary-messages branch from b9434f0 to 25970b2 Compare May 29, 2022 12:24
@timkrueger timkrueger marked this pull request as draft June 1, 2022 12:53
@nextcloud-command nextcloud-command force-pushed the feature/noid/temporary-messages branch from e3942dd to 31fc5df Compare July 6, 2022 20:33
@nextcloud-android-bot
Copy link
Collaborator

Lint

TypemasterPR
Warnings116116
Errors11

SpotBugs (new)

Warning Type Number
Bad practice Warnings 8
Correctness Warnings 35
Experimental Warnings 2
Internationalization Warnings 9
Malicious code vulnerability Warnings 23
Performance Warnings 22
Security Warnings 2
Dodgy code Warnings 65
Total 166

SpotBugs (master)

Warning Type Number
Bad practice Warnings 8
Correctness Warnings 35
Experimental Warnings 2
Internationalization Warnings 9
Malicious code vulnerability Warnings 23
Performance Warnings 22
Security Warnings 2
Dodgy code Warnings 65
Total 166

@nextcloud-command nextcloud-command force-pushed the feature/noid/temporary-messages branch from 31fc5df to 3cd58a8 Compare July 28, 2022 22:46
@AndyScherzinger AndyScherzinger force-pushed the feature/noid/temporary-messages branch from 3cd58a8 to edecb82 Compare December 30, 2022 15:56
@AndyScherzinger
Copy link
Member

/rebase

nickvergessen and others added 2 commits January 8, 2023 20:14
Also fixing some detekt issues.

Signed-off-by: Tim Krüger <[email protected]>
@nextcloud-command nextcloud-command force-pushed the feature/noid/temporary-messages branch from edecb82 to 26c2c00 Compare January 8, 2023 20:14
@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2023

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/1259-talk.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud Talk app.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2023

Codacy

Lint

TypemasterPR
Warnings109109
Errors00

SpotBugs

CategoryBaseNew
Correctness1212
Dodgy code173173
Internationalization55
Malicious code vulnerability33
Performance99
Security22
Total204204

@nickvergessen
Copy link
Member Author

wow that's on old one. i now rebased on master but didn't have a closer look what needs to be done here

Any chance this makes it at some point? :)

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