Feature/4378/add temporary messages while sending #4422

mahibi · 2024-11-06T15:38:35Z

resolve #4378

🖼️ Screenshots

When sending, a clock icon is shown:

When being offline, an icon is shown:

When sending failed, a warning icon is shown:

On long click, actions are offered for temporary messages:

"Copy" button is always available.
When sending failed or connection is gone, "Delete" and "Edit" is available.
When sending failed, "Resend" button is available.

🚧 TODO

for followup (maybe not directly related to this PR, but happened during developing):

add logic to check for bad internet connection. Show a warning?
- e.g. to add solution for StreamResetException when pulling chat messages
fix loading spinner overlay with chat messages (not sure if related to this PR)

🏁 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?)

alperozturk96 · 2025-01-07T15:02:03Z

Maybe when the device is offline, the icon could be replaced with a schedule icon instead of the “Wi-Fi off” icon.

nimishavijay · 2025-01-07T15:38:39Z

Super nice! :) Small design feedback:

Instead of a loading spinner, we could use a schedule symbol to make it less distracting (similar to Whatsapp and Telegram, they show a very similar icon)
Instead of the warning icon in the screenshot, we can use the error symbol, also for consistency with other apps (eg. SMS app in Android)
I'm wondering if we could the same error symbol for even the "no connection" state, as it is just another type of error where you can edit/resend etc. The red banner is already very eye catching, and I haven't seen a similar icon in any other apps, so I'm thinking it might confuse users. What do you think? :)

mahibi · 2025-01-07T19:06:02Z

Super nice! :) Small design feedback:

* [ ]  Instead of a loading spinner, we could use a [schedule symbol](https://fonts.google.com/icons?icon.query=time&selected=Material+Symbols+Outlined:schedule:FILL@0;wght@400;GRAD@0;opsz@24&icon.size=24&icon.color=%23222222) to make it less distracting (similar to Whatsapp and Telegram, they show a very similar icon)

We have loading spinners on web, desktop app and iOS, so i think we should have it for android as well..?

* [ ]  Instead of the warning icon in the screenshot, we can use the [error symbol](https://fonts.google.com/icons?icon.query=warning&selected=Material+Symbols+Outlined:error:FILL@0;wght@400;GRAD@0;opsz@24&icon.size=24&icon.color=%23222222), also for consistency with other apps (eg. SMS app in Android)

fine for me

* [ ]  I'm wondering if we could the same error symbol for even the "no connection" state, as it is just another type of error where you can edit/resend etc. The red banner is already very eye catching, and I haven't seen a similar icon in any other apps, so I'm thinking it might confuse users. What do you think? :)

In "no connection" state, the resend button is not available. When using the same icon for both states it would be confusing when the resend button is available or not. So i suggest to have seperate icons.
Not sure if the schedule icon suggested by @alperozturk96 is the best suggestion for it.. We have to keep in mind that at some point we want to have scheduled messages and we also need an icon for it..
The iOS app uses also a loading spinner when being offline, but i think all the movements are too much when having multiple messages pending..

Maybe @nickvergessen has opinions on this?

nimishavijay · 2025-01-08T07:26:55Z

We have loading spinners on web, desktop app and iOS, so i think we should have it for android as well..?

I would say for Android we follow the standard of other apps on Android so it looks as native as possible :) also because of what you mentioned

but i think all the movements are too much when having multiple messages pending

So I'd suggest the schedule icon instead of the loading spinner

We have to keep in mind that at some point we want to have scheduled messages and we also need an icon for it..

Taking a quick look at Signal and Telegram, a calendar icon is used in those, so we could do the same

the resend button is not available. When using the same icon for both states it would be confusing when the resend button is available or not. So i suggest to have seperate icons.

Oh I see! Then it makes sense.

nickvergessen · 2025-01-08T09:25:38Z

Instead of a loading spinner, we could use a schedule symbol to make it less distracting (similar to Whatsapp and Telegram, they show a very similar icon)

We have loading spinners on web, desktop app and iOS, so i think we should have it for android as well..?

I think we shouldn't use the schedule icon as scheduling is still a planned feature and it would be confusing?

Signed-off-by: Marcel Hibbe <[email protected]>

by moving networkMonitor.isOnline to separate check and by setting binding.checkMark.visibility = View.INVISIBLE binding.sendingProgress.visibility = View.GONE before setting the status icons to to handle recyclerview behavior Signed-off-by: Marcel Hibbe <[email protected]>

Signed-off-by: Marcel Hibbe <[email protected]>

during sending: edit and delete should not be shown.. Signed-off-by: Marcel Hibbe <[email protected]>

Signed-off-by: Marcel Hibbe <[email protected]>

mahibi · 2025-01-10T11:41:14Z

/backport to stable-20.1

github-actions · 2025-01-10T11:49:46Z

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

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.

remove shadowed var Signed-off-by: Marcel Hibbe <[email protected]>

Signed-off-by: github-actions <[email protected]>

github-actions · 2025-01-10T12:28:52Z

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

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 · 2025-01-10T12:36:37Z

Codacy

Lint

Type	master	PR
Warnings	106	104
Errors	36	36

SpotBugs

Category	Base	New
Bad practice	6	6
Correctness	222	222
Dodgy code	71	71
Internationalization	3	3
Malicious code vulnerability	3	3
Performance	4	4
Security	1	1
Total	310	310

nickvergessen

🐘

mahibi · 2025-01-10T12:56:02Z

not backported.
will be released with v21.x

mahibi mentioned this pull request Nov 18, 2024

Save push notified messages to the database without entering the app #4463

Closed

6 tasks

mahibi force-pushed the feature/4378/addTemporaryMessagesWhileSending branch from d335f4f to 17995ee Compare November 21, 2024 21:45

mahibi force-pushed the feature/4378/addTemporaryMessagesWhileSending branch 4 times, most recently from 43335b2 to 85aeab0 Compare December 4, 2024 14:47

mahibi force-pushed the feature/4378/addTemporaryMessagesWhileSending branch 4 times, most recently from 3356718 to 14caa73 Compare December 13, 2024 12:51

mahibi force-pushed the feature/4378/addTemporaryMessagesWhileSending branch 2 times, most recently from 4fcc62c to fc05012 Compare December 27, 2024 11:46

mahibi force-pushed the feature/4378/addTemporaryMessagesWhileSending branch 2 times, most recently from bfe6f70 to 55d0880 Compare January 3, 2025 17:28

mahibi marked this pull request as ready for review January 3, 2025 17:28

mahibi self-assigned this Jan 3, 2025

mahibi added the 2. developing Work in progress label Jan 3, 2025

mahibi force-pushed the feature/4378/addTemporaryMessagesWhileSending branch from 55d0880 to c7e8e1f Compare January 6, 2025 11:41

mahibi added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 6, 2025

mahibi requested review from rapterjet2004 and AndyScherzinger January 6, 2025 15:41

mahibi modified the milestones: 20.1.1, 21.0.0 Jan 6, 2025

mahibi added 11 commits January 10, 2025 12:32

resolve warnings

3dca00b

Signed-off-by: Marcel Hibbe <[email protected]>

comment in opHelperFactory

560f955

Signed-off-by: Marcel Hibbe <[email protected]>

resolve detekt warnings

a58607b

Signed-off-by: Marcel Hibbe <[email protected]>

add DB migration

4c79513

Signed-off-by: Marcel Hibbe <[email protected]>

fix parent message for temp messages

3fdaa4b

Signed-off-by: Marcel Hibbe <[email protected]>

save "silent" in chat messages (incl DB)

f665b1c

Signed-off-by: Marcel Hibbe <[email protected]>

add strings for message status icons

1504e51

Signed-off-by: Marcel Hibbe <[email protected]>

change visibility of temp message actions

3094054

during sending: edit and delete should not be shown.. Signed-off-by: Marcel Hibbe <[email protected]>

remove click listener for failed messages (use default longclick)

aa5b4d0

Signed-off-by: Marcel Hibbe <[email protected]>

fix lint warning

68065d7

Signed-off-by: Marcel Hibbe <[email protected]>

mahibi force-pushed the feature/4378/addTemporaryMessagesWhileSending branch from e82585d to 6e24cca Compare January 10, 2025 11:36

backportbot bot added the backport-request label Jan 10, 2025

mahibi and others added 2 commits January 10, 2025 13:18

change icons for message sending and messages sending failed

cd09636

remove shadowed var Signed-off-by: Marcel Hibbe <[email protected]>

Analysis: update lint results to reflect reduced error/warning count

4f4ac5e

Signed-off-by: github-actions <[email protected]>

mahibi force-pushed the feature/4378/addTemporaryMessagesWhileSending branch from ec23fa6 to 4f4ac5e Compare January 10, 2025 12:19

nickvergessen approved these changes Jan 10, 2025

View reviewed changes

mahibi merged commit 88bb5d5 into master Jan 10, 2025
18 checks passed

mahibi deleted the feature/4378/addTemporaryMessagesWhileSending branch January 10, 2025 12:43

backportbot bot mentioned this pull request Jan 10, 2025

[stable-20.1] Feature/4378/add temporary messages while sending #4593

Closed

3 tasks

backportbot bot removed the backport-request label Jan 10, 2025

This was referenced Jan 13, 2025

Worker to send temporary messages #4596

Open

duplicated messages in chat #4605

Closed

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

Closed

mahibi mentioned this pull request Jan 27, 2025

Messages get lost between client and server once sent. No error shown. #3964

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/4378/add temporary messages while sending #4422

Feature/4378/add temporary messages while sending #4422

mahibi commented Nov 6, 2024 •

edited

Loading

alperozturk96 commented Jan 7, 2025 •

edited

Loading

nimishavijay commented Jan 7, 2025

mahibi commented Jan 7, 2025

nimishavijay commented Jan 8, 2025

nickvergessen commented Jan 8, 2025

mahibi commented Jan 10, 2025

github-actions bot commented Jan 10, 2025

github-actions bot commented Jan 10, 2025

github-actions bot commented Jan 10, 2025

nickvergessen left a comment

mahibi commented Jan 10, 2025

Feature/4378/add temporary messages while sending #4422

Feature/4378/add temporary messages while sending #4422

Conversation

mahibi commented Nov 6, 2024 • edited Loading

🖼️ Screenshots

🚧 TODO

🏁 Checklist

alperozturk96 commented Jan 7, 2025 • edited Loading

nimishavijay commented Jan 7, 2025

mahibi commented Jan 7, 2025

nimishavijay commented Jan 8, 2025

nickvergessen commented Jan 8, 2025

mahibi commented Jan 10, 2025

github-actions bot commented Jan 10, 2025

github-actions bot commented Jan 10, 2025

github-actions bot commented Jan 10, 2025

Codacy

Lint

SpotBugs

nickvergessen left a comment

Choose a reason for hiding this comment

mahibi commented Jan 10, 2025

mahibi commented Nov 6, 2024 •

edited

Loading

alperozturk96 commented Jan 7, 2025 •

edited

Loading