show deck card in messages #4327

sowjanyakch · 2024-10-09T19:05:34Z

fix #1025

show deck card in the chat messages when user shared card via "Post to conversation".
Clicking on the deck card chat message will navigate to the deck card on the web.

🚧 TODO

...

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

mahibi · 2024-10-18T13:46:47Z

Playing around how deck cards in more detail for the first time. It feels quite confusing...:

The handling how deck cards are viewed are quite depending on who sent it in which way, if the board is shared to the other user, and also it's different by platforms.

See numbers in the first screenshot of web...

1&2 were received when the board was not yet shared.

was received from other user who sent it from deck via "Post to a conversation"
was received from other user by smartpicker (however if the board is shared beforehand, it displayed via preview(OpenGraph?))
was sent to other user via "Post to a conversation"
was sent to other user by smartpicker (it's a link that has a preview by OpenGraph)

This is how it looks in web:

Android (with this PR):

iPhone:

Todos:

todo for server people?: share board when a card is posted to a chat???
we should decide if cards are shown by own design or by OpenGraph? But mixed it's confusing?
todo for android?: visualize OpenGraph link when it's a Deck Card? Is this how it's done on iOS and web?
todo for android: check if links are opened in Android Nextcloud Deck app / if not whats needs to be done?

pinging some people if anyone want to share opinions..
@nickvergessen @Ivansss @SystemKeeper @juliusknorr

mahibi · 2024-10-18T13:48:21Z

app/src/main/res/values/strings.xml

@@ -415,6 +416,7 @@ How to translate with transifex:
    <string name="nc_sent_poll_you">You sent a poll.</string>
    <string name="nc_sent_location_you">You sent a location.</string>
    <string name="nc_sent_voice_you">You sent a voice message.</string>
+    <string name="nc_sent_deck_card_you">You: %1$s</string>


You sent a deck card.

@mahibi - The string is modified to align with web

The web is always saying You: … but that's not the best. Use what Marcel suggested, to align with the other messages of the same type that exist atm in Android

mahibi · 2024-10-18T13:48:38Z

app/src/main/res/values/strings.xml

@@ -407,6 +407,7 @@ How to translate with transifex:
    <string name="nc_sent_poll" formatted="true">%1$s sent a poll.</string>
    <string name="nc_sent_location" formatted="true">%1$s sent a location.</string>
    <string name="nc_sent_voice" formatted="true">%1$s sent a voice message.</string>
+    <string name="nc_sent_deck_card" formatted="true">%1$s: %2$s</string>


%1$s sent a deck card.

The string is modified to align with web

Same as above

SystemKeeper · 2024-10-18T19:01:53Z

1 was received from other user who sent it from deck via "Post to a conversation"

We try to use the link that included in the richObject to get additional information from the reference provider (not opengraph). If that is not possible, we render the deck card with the information we have in the richObject. Actually the last one is not correctly working, as our check is wrong, you found a bug :) nextcloud/talk-ios#1843. Without the bug, we should render the same as android.

2 was received from other user by smartpicker (however if the board is shared beforehand, it displayed via preview(OpenGraph?))

Since it's just a link, we try to get reference data. If there's one for "deck-card" we use that: https://github.com/nextcloud/talk-ios/blob/25ebf7eca1f1043ab89b7ff4791a4eac34dff075/NextcloudTalk/ReferenceView.swift#L126-L135. Otherwise we fall back to opengraph data, which most of the time is just the link.

3 was sent to other user via "Post to a conversation"

Should be the same as 1. Not totally sure, why it's rendered in this case. Maybe the reference provider behaves differently, but need to check.

4 was sent to other user by smartpicker (it's a link that has a preview by OpenGraph)

Same as 2.

sowjanyakch · 2024-10-22T11:07:16Z

@nickvergessen - Is the current implementation of deck card on android okay for you or do you want to make any changes to the way the deck card is shown?

nickvergessen · 2024-10-23T05:54:54Z

Regarding the questions/comments in #4327 (comment)

was received from other user by smartpicker (however if the board is shared beforehand, it displayed via preview(OpenGraph?))

When the board/card is not publicly accessible, OpenGraph will not be able to return any information, so that's expected.

todo for server people?: share board when a card is posted to a chat???

It was decided that automatic sharing should not happen, as otherwise people might expect that quickly afterwards deleting the message would also undo the share, but we wouldn't know if we just shared or not. In addition we would need to understand and interact with quite a lot 3rdparty app APIs in those cases, so that's a "No" for now.

we should decide if cards are shown by own design or by OpenGraph? But mixed it's confusing?

There is also a significant difference when you share a file to chat or when you post /f/ link or even a public share link. So yes, for now the difference is expected.

todo for android: check if links are opened in Android Nextcloud Deck app / if not whats needs to be done?

While that would be cool, we can also simply raise a follow up issue and keep this PR here scoped.

Signed-off-by: sowjanyakch <[email protected]>

github-actions · 2024-10-23T09:47:31Z

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/4327-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 · 2024-10-23T09:48:11Z

Codacy

Lint

Type	master	PR
Warnings	94	97
Errors	132	132

SpotBugs

Category	Base	New
Bad practice	6	6
Correctness	11	11
Dodgy code	79	79
Internationalization	3	3
Malicious code vulnerability	3	3
Performance	6	6
Security	1	1
Total	109	109

Lint increased!

mahibi · 2024-10-23T10:58:41Z

/backport to stable-20.0

sowjanyakch marked this pull request as draft October 9, 2024 19:06

sowjanyakch force-pushed the deck_card_is_shown_as_object branch from 6b68499 to fb4997f Compare October 11, 2024 11:08

sowjanyakch marked this pull request as ready for review October 11, 2024 11:08

sowjanyakch requested review from mahibi and rapterjet2004 October 11, 2024 11:21

sowjanyakch self-assigned this Oct 11, 2024

sowjanyakch added the 3. to review Waiting for reviews label Oct 11, 2024

sowjanyakch force-pushed the deck_card_is_shown_as_object branch from 83771cc to 3093f0f Compare October 11, 2024 12:27

mahibi reviewed Oct 18, 2024

View reviewed changes

sowjanyakch force-pushed the deck_card_is_shown_as_object branch from 3093f0f to 9b9f5e0 Compare October 21, 2024 11:30

sowjanyakch added 14 commits October 23, 2024 11:35

create deck card xml layout

66d2db2

Signed-off-by: sowjanyakch <[email protected]>

Work on Incoming and Outgoing deck card viewholder

56b0b80

Signed-off-by: sowjanyakch <[email protected]>

Display card deck with parameters

c80050d

Signed-off-by: sowjanyakch <[email protected]>

work on UI of OutcomingDeckCardViewHolder

726fd34

Signed-off-by: sowjanyakch <[email protected]>

work on UI of IncomingDeckCardViewHolder

8b52eb4

Signed-off-by: sowjanyakch <[email protected]>

solve contentDescription warning for Image

f492ebf

Signed-off-by: sowjanyakch <[email protected]>

include deck card name as the lastMessageDisplayText

de420a3

Signed-off-by: sowjanyakch <[email protected]>

Add theming to deck card image

305a66e

Signed-off-by: sowjanyakch <[email protected]>

Rename outcomingDeckCardViewHolder

eb5cdca

Signed-off-by: sowjanyakch <[email protected]>

add replyable message tag

1e82697

Signed-off-by: sowjanyakch <[email protected]>

use correct deck card icon

bf1a426

Signed-off-by: sowjanyakch <[email protected]>

use official deck card icon and some alignment changes

986f19a

Signed-off-by: sowjanyakch <[email protected]>

modify strings

81040e3

Signed-off-by: sowjanyakch <[email protected]>

ktlintFormat

99b410f

Signed-off-by: sowjanyakch <[email protected]>

sowjanyakch force-pushed the deck_card_is_shown_as_object branch from 9b9f5e0 to 99b410f Compare October 23, 2024 09:39

sowjanyakch requested a review from mahibi October 23, 2024 09:42

mahibi approved these changes Oct 23, 2024

View reviewed changes

mahibi merged commit 5bee52d into master Oct 23, 2024
16 of 18 checks passed

mahibi deleted the deck_card_is_shown_as_object branch October 23, 2024 10:58

backportbot bot added the backport-request label Oct 23, 2024

backportbot bot mentioned this pull request Oct 23, 2024

[stable-20.0] show deck card in messages #4370

Closed

2 tasks

backportbot bot removed the backport-request label Oct 23, 2024

AndyScherzinger added this to the 20.1.0 milestone Oct 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

show deck card in messages #4327

show deck card in messages #4327

sowjanyakch commented Oct 9, 2024 •

edited

Loading

mahibi commented Oct 18, 2024

mahibi Oct 18, 2024

sowjanyakch Oct 21, 2024

nickvergessen Oct 23, 2024

mahibi Oct 18, 2024

sowjanyakch Oct 21, 2024

nickvergessen Oct 23, 2024

SystemKeeper commented Oct 18, 2024

sowjanyakch commented Oct 22, 2024

nickvergessen commented Oct 23, 2024

github-actions bot commented Oct 23, 2024

github-actions bot commented Oct 23, 2024

mahibi commented Oct 23, 2024

show deck card in messages #4327

show deck card in messages #4327

Conversation

sowjanyakch commented Oct 9, 2024 • edited Loading

🚧 TODO

🏁 Checklist

mahibi commented Oct 18, 2024

Todos:

mahibi Oct 18, 2024

Choose a reason for hiding this comment

sowjanyakch Oct 21, 2024

Choose a reason for hiding this comment

nickvergessen Oct 23, 2024

Choose a reason for hiding this comment

mahibi Oct 18, 2024

Choose a reason for hiding this comment

sowjanyakch Oct 21, 2024

Choose a reason for hiding this comment

nickvergessen Oct 23, 2024

Choose a reason for hiding this comment

SystemKeeper commented Oct 18, 2024

sowjanyakch commented Oct 22, 2024

nickvergessen commented Oct 23, 2024

github-actions bot commented Oct 23, 2024

github-actions bot commented Oct 23, 2024

Codacy

Lint

SpotBugs

Lint increased!

mahibi commented Oct 23, 2024

sowjanyakch commented Oct 9, 2024 •

edited

Loading