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

Reactions to chat messages #1895

Merged
merged 59 commits into from
Apr 9, 2022
Merged

Reactions to chat messages #1895

merged 59 commits into from
Apr 9, 2022

Conversation

mahibi
Copy link
Collaborator

@mahibi mahibi commented Apr 1, 2022

resolve #1772

Message Reactions menu Reactions listing
grafik grafik grafik

remaining TODOs:

  • send reaction
    • emoji picker
  • delete reaction
  • tab layout
    • add view pager if time permits
  • show reactions for all MessageViewHolders (incoming + outgoing messages)
    • TextMessages
    • VoiceMessages
    • Location
    • Previews
  • set textcolor for reaction for outgoing messages
  • sort emojis in message view descending by amount
  • analyze why some emojis are not shown some emojis are not shown  #1913
    • grafik
  • sort reaction list

known issues:

@mahibi mahibi marked this pull request as draft April 1, 2022 12:40
@mahibi mahibi added this to the 14.0.0 milestone Apr 1, 2022
@AndyScherzinger AndyScherzinger changed the title Feature/1772/reactions Reactions to chat messages Apr 1, 2022
@AndyScherzinger AndyScherzinger force-pushed the feature/1772/reactions branch 5 times, most recently from f933c3b to 88adeb5 Compare April 3, 2022 10:00
@AndyScherzinger AndyScherzinger force-pushed the feature/1772/reactions branch 3 times, most recently from a9e10d2 to 2f827c5 Compare April 4, 2022 22:23
@mahibi mahibi force-pushed the feature/1772/reactions branch 2 times, most recently from 453002c to 20e67c8 Compare April 6, 2022 12:53
AndyScherzinger and others added 9 commits April 8, 2022 09:54
this must have been removed when auto cleaning imports while there was a problem to find the coil lib

Signed-off-by: Marcel Hibbe <[email protected]>
Signed-off-by: Marcel Hibbe <[email protected]>
Signed-off-by: Marcel Hibbe <[email protected]>
Signed-off-by: Marcel Hibbe <[email protected]>
@mahibi mahibi marked this pull request as ready for review April 8, 2022 15:55
@AndyScherzinger
Copy link
Member

@mahibi changinging the sorting for the all tab can be done but shouldn't block this PR - can't say if/when I find the time or you can give it a go.

@XueSheng-GIT
Copy link

I downloaded the apk to test the reactions. Adding a reaction only works if there's no reaction yet (reaction menu pops up). But as soon as a reaction exists (from myself or another person), the reaction listing is shown (which doesn't seem to allow to add additional reactions...). I couldn't find a way to add a reaction in this case.
How to switch from the reaction listing to the reaction menu?

@mahibi
Copy link
Collaborator Author

mahibi commented Apr 9, 2022

I downloaded the apk to test the reactions. Adding a reaction only works if there's no reaction yet (reaction menu pops up). But as soon as a reaction exists (from myself or another person), the reaction listing is shown (which doesn't seem to allow to add additional reactions...). I couldn't find a way to add a reaction in this case. How to switch from the reaction listing to the reaction menu?

a long click on the message will open the Reactions menu where you can set an emoji. to set more emojis just do this another time. This should work regardless if there are already emojis or not!

if reactions exist, you can tap on them inside the message bubble to see details in the Reactions listing. In this view you can't add reactions but just view them (or delete your own reactions when you tap on them.)

Does this answer your question? Or is there some bug?

@XueSheng-GIT
Copy link

XueSheng-GIT commented Apr 9, 2022

a long click on the message will open the Reactions menu where you can set an emoji. to set more emojis just do this another time. This should work regardless if there are already emojis or not!

For me this only works if there's no reaction yet. Long press on an existing reaction does nothing (while pressing). But when releasing the finger, the reaction listing appears.

UPDATE: Just realized that long press works if you exactly hit the text iself. Long press on existing reactions does not open the reactions menu. A bit confusing and not obvious.

@mahibi
Copy link
Collaborator Author

mahibi commented Apr 9, 2022

a long click on the message will open the Reactions menu where you can set an emoji. to set more emojis just do this another time. This should work regardless if there are already emojis or not!

For me this only works if there's no reaction yet. Long press on an existing reaction does nothing (while pressing). But when releasing the finger, the reaction listing appears.

if there are emojis in the message, there's a difference if you tap on the message text or the emojis themselves.
so a long press on the message text will open the Reactions menu.
a press on the emojis will open the Reactions listing view.

does this work for you?

But maybe it should be changed that a long tap on the emojis should also open the Reactions menu....

@AndyScherzinger
Copy link
Member

But maybe it should be changed that a long tap on the emojis should also open the Reactions menu....

for long press, I'd say yes because that would be the expected result I believe.

@mahibi
Copy link
Collaborator Author

mahibi commented Apr 9, 2022

UPDATE: Just realized that long press works if you exactly hit the text iself. Long press on existing reactions does not open the reactions menu. A bit confusing and not obvious.

just changed this. that was a good hint, thanks for testing @XueSheng-GIT 👍

@nextcloud-android-bot
Copy link
Collaborator

Lint

TypemasterPR
Warnings164156
Errors11

SpotBugs (new)

Warning Type Number
Bad practice Warnings 9
Correctness Warnings 91
Experimental Warnings 2
Internationalization Warnings 9
Malicious code vulnerability Warnings 115
Performance Warnings 27
Security Warnings 2
Dodgy code Warnings 158
Total 413

SpotBugs (master)

Warning Type Number
Bad practice Warnings 9
Correctness Warnings 91
Experimental Warnings 2
Internationalization Warnings 9
Malicious code vulnerability Warnings 115
Performance Warnings 27
Security Warnings 2
Dodgy code Warnings 158
Total 413

@github-actions
Copy link
Contributor

github-actions bot commented Apr 9, 2022

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/1895-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.

@mahibi mahibi merged commit d66a6a9 into master Apr 9, 2022
@delete-merged-branch delete-merged-branch bot deleted the feature/1772/reactions branch April 9, 2022 21:21
@mahibi
Copy link
Collaborator Author

mahibi commented Apr 9, 2022

merged without approval to get towards the next RC (tonight or in the next days i guess).

thanks for your help @AndyScherzinger 👍

@AndyScherzinger
Copy link
Member

My pleasure 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress design Related to the design feature: 🗨️ chat
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Reactions to chat messages
4 participants