followup for #1935: fix notification subject for reaction

[mahibi]

align text for #1935 to align with ios

https://github.com/nextcloud/talk-android/pull/1936#issuecomment-1106848465

[mahibi]

regarding https://github.com/nextcloud/talk-android/pull/1936#issuecomment-1106848465

Btw the expected fixed version from my POV would be:

Subject as delivered Message that was reacted on

So in your sample

marcel hat mit smiling_face_with_three_hearts auf ihre private Nachricht reagiert Hello

@nickvergessen @AndyScherzinger

As far as i see this is against android's concept of notifications for chat apps. We need to set a person object which name is used to set the contentTitle.

androids documentation: https://developer.android.com/reference/androidx/core/app/NotificationCompat.MessagingStyle where we set it right now: https://github.com/nextcloud/talk-android/blob/30d57fda4f616a4b77b794a977deb651887fd585/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.java#L449

So i guess that's why signal and telegram mix the reaction and the referred message in one text:

So this could be an option for us too, but still would be different to iOS then.. (+ it would require own translation...) Should we do this for 14.0.0 or keep it like it is for now?

PS: From my POV the display of notifications (NotificationsWorker.java) should get more attention after 14.0.0, right now it's a bit complicated..

[mahibi]

comment from @AndyScherzinger :

I'd prefer consistency as a start so the questions would be

• how is this solved throughout other messengers on iOS
• if there are consistent patterns per platform I might still tend to then go with each platform since that matches the user's expectation.

maybe @jancborchardt also has a take on this?

[mahibi]

@Ivansss i guess for iOS the concept is different so it's more flexible to to set the contents? or is it similar to android?

[Ivansss]

I guess it's similar to Android.

At the moment reactions notifications are displayed on iOS like:

Subject as delivered Message that was reacted on

We can think about another format for a next version but I would stick to the format above for 14.0 :)

[jancborchardt]

I'm fine with whatever is feasible for this version. For a future version either how Signal does it, or how Telegram does it (bit shorter, nice) would be good. :)

[nickvergessen]

How long should the text be before it's trimmed in case someone hearts my 4k character messages and not only "zzzZ" Very short? 32/64? Or medium like 128? Or untrimmed?

[starypatyk]

@mahibi

PS: From my POV the display of notifications (NotificationsWorker.java) should get more attention after 14.0.0, right now it's a bit complicated..

Fully agree! :wink:

I plan some refactoring in #1923 (after 14.0.0 is released).

[AndyScherzinger]

@starypatyk I think you are good to go now, last RC4 for 14.0.0 has been tagged today which should then be the stable release next week, at least all changes are in, so there shouldn't be any changes anymore except critical fixes but that shouldn't impact the notification code (too much).

[timkrueger]

Very short? 32/64? Or medium like 128? Or untrimmed? @nickvergessen

I would say untrimmed. Then it can be handled in the client. But then it must be separated so that the client can trim it independently from the reaction part. Or really short with 32 characters.

# Longer
Reacted 😎 to: 'Lorem ipsum dolor sit amet, consectetur ...'

# Shorter
😎 to 'Lorem ipsum dolor sit amet, consectetur ...'

	Compact	Expanded
Longer
Shorter

I faked the screenshots. Would this the way to go?

cc @jancborchardt @Ivansss

I edited the screenshots after a short offside discussion with @nickvergessen. Now the 'Longer' example in the table above is Signal style.

[timkrueger]

Here some examples from Signal:

Short text:

Long text:

[jancborchardt]

Very cool @timkrueger! :) I’d say including the "Reacted" is good as it makes abundantly clear what it is, and usually just seeing the very start of the message (in compacted) is enough.

[timkrueger]

So then this issue can be transferred to the spreed repository? I think that changes must only be done on the server.

@nickvergessen @Ivansss

[nickvergessen]

Aren't you replacing the message already in the client? Because what you display as "message" is the subject of the notification. So you can also just translate it there and add the parameters from the subject and the parsed message into the new string.

We can't really do it on the notification directly as if the subject contains the message we have to heavily cut the content as we can only encrypt around 150 characters.

[timkrueger]

Aren't you replacing the message already in the client? Because what you display as "message" is the subject of the notification. So you can also just translate it there and add the parameters from the subject and the parsed message into the new string.

No. I faked the screenshots.

We use the subject to set the conversation title. In the case of a 1-to-1 conversation it's an empty string: https://github.com/nextcloud/talk-android/blob/387bc3ca4f4c4e33b53ecd1d54283e510a872eb9/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.java#L517

The text which is actually shown is directly set from the encrypted message: https://github.com/nextcloud/talk-android/blob/387bc3ca4f4c4e33b53ecd1d54283e510a872eb9/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.java#L527

Because of that I have German text on an English client:

Tim Zwei hat mit ☺️ auf deine private Nachricht reagiert

We can't really do it on the notification directly as if the subject contains the message we have to heavily cut the content as we can only encrypt around 150 characters.

You're right. The complete problem can not be fixed on the server. But we need somehow only the reaction emoji without the text around.

Or I'm completely wrong?

[nickvergessen]

But we need somehow only the reaction emoji without the text around.

The decrypted push only contains the parsed subject. In case you still get the full notification from the server, you should get the emoji there as a type=highlight parameter with the placeholder reaction https://github.com/nextcloud/spreed/blob/486b89806f4185b01e61009ee486df90cbd5fa22/lib/Notification/Notifier.php#L456-L460