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

Improve chat functionality of mediation/arbitration #5207

Merged
merged 5 commits into from
Mar 17, 2021

Conversation

ghost
Copy link

@ghost ghost commented Feb 19, 2021

Fixes #5097
Fixes #3412


Agent view:

image

Client view:

image

Light mode:

image


@leo816
Indicate trade end period
Indicate XMR tx proof (when applicable)

image


@pazza83
Allow trader to close ticket if paid out

image
image


Indicate more clearly the sender (trader side):

image

Indicate more clearly the sender (agent side):

image

@Conza88
Copy link

Conza88 commented Feb 19, 2021

Too late to incorporate #5044 ? hah

@ripcurlx ripcurlx added this to the v1.6.0 milestone Feb 19, 2021
@ripcurlx
Copy link
Contributor

ripcurlx commented Feb 19, 2021

@jmacxx Great that you picked this up!

IMO we should only have the info icon in front and move the other action icons (process and chat) to the end. It just feels weird for me having this action icons in the front (yes, I know we had the CTA buttons there as well in the past). What do you think?

Also it would be great if you could adapt the badge number on the support button so it doesn't reflect the number of open support cases, but rather if there is an action that needs to be done. e.g. if there are new support cases add a new badge to the info icon (add the number of new support cases to the support button) and if it was selected once decrease the badge number and remove the new badge. If there are new chat message for a support case add (+1) to the support button badge and decrease it (-1) if the chat messages were viewed. If a support case was closed add +1 and decrease it if the final message by the mediator/arbitrator was viewed and so on. Do you know what I mean?

I think doing like that prevents that people are ignoring the badge icon as it is always there no matter if there is an action to do or not.

@ghost ghost marked this pull request as draft February 20, 2021 19:46
@ghost ghost closed this Feb 25, 2021
@ghost ghost force-pushed the improve_dispute_chat branch from 2e2ecb4 to d82a61d Compare February 25, 2021 21:40
@ghost ghost reopened this Feb 25, 2021
@ghost ghost mentioned this pull request Feb 28, 2021
Show badge with number of unread chat messages on each ticket
Use icons for chat, info and process ticket functions
Select a row by clicking on it, no clunky select button needed.
Show chat messages in a movable popup window.
More space for ticket list.

Implement requested feature additions:

Indicate if the trade period is over
Indicate more clearly the sender of each chat message
Support badge count
Indicate the XMR tx proof (when applicable)
Allow trader to close own mediation ticket if the trade is paid out

Fixes:

null check for cases when extraData null/not applicable
when upgrading closed disputes, clear chat unread count
@ghost ghost force-pushed the improve_dispute_chat branch from bc76959 to cc56470 Compare March 4, 2021 00:43
@ghost ghost marked this pull request as ready for review March 4, 2021 01:12
@ghost
Copy link
Author

ghost commented Mar 4, 2021

Ready for review!

Dispute object now contains a State field to represent whether it is new, open, reopened, or closed => necessary for the count on the Support badge and displaying a "New" badge on table rows.

@ripcurlx
Copy link
Contributor

@jmacxx Is there a reason why you chose orange for the new badge?

@ghost
Copy link
Author

ghost commented Mar 11, 2021

@jmacxx Is there a reason why you chose orange for the new badge?

You're right, it should be red. Fix pushed.

image

@Conza88
Copy link

Conza88 commented Mar 14, 2021

Awesome work

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NACK - Some naming suggestions and comments I would leave out of this PR.

Also I tested the functionality and found following UX issue:

When a new mediation case is opened it looks like that to the user:
Bildschirmfoto 2021-03-15 um 11 39 47

It is not immediately clear to me what the action I need to do to get rid of the New badge (I need to open the chat). For this initial system message I think the chat icon should have a (1) badge already, so users are clicking on it. I also think it would be great to open the chat window on double click of the dispute item as the default action. WDYT? I'm continuing to test the remaining functionality and will add my comments below. But besides that - great improvements already 👍

@ripcurlx
Copy link
Contributor

Bildschirmfoto 2021-03-15 um 11 45 49

I think if multiple chat messages are sent from one dispute which can be resolved by looking at one chat window the badge count on support support should be (1).

@ripcurlx
Copy link
Contributor

I'm also not sure if the cog icon is the perfect choice for the process. I always think about settings or preferences when I see it. Maybe CLIPBOARD_CHECK
Bildschirmfoto 2021-03-15 um 11 55 17
or FORMAT_LIST_CHECKS could be a better choice. WDYT?
Bildschirmfoto 2021-03-15 um 11 53 54

renamed alertCountProperty to badgeCountProperty
changed the badge counting policy so that the count of chat messages does not contribute to the badge number, but merely the presence of chat messages, and/or the new flag.
show an explanatory message when trader is not allowed to close own dispute ticket
removed some comments and code that were accidentally included from PR5160
renamed variable tpe to tradePeriodEnd
renamed variable cptyName to counterpartyName
clear the new badge on double-click of dispute tablerow
@ghost ghost force-pushed the improve_dispute_chat branch from 3055236 to ad4927a Compare March 15, 2021 22:27
@ghost
Copy link
Author

ghost commented Mar 15, 2021

@ripcurlx Thanks for the detailed review! I've addressed all issues and have an open question with @leo816 and @huey735 about the Process icon.

ad4927a

  • renamed alertCountProperty to badgeCountProperty
  • changed the badge counting policy so that the count of chat messages does not contribute to the badge number, but merely the presence of chat messages, and/or the new flag.
  • show an explanatory message when trader is not allowed to close own dispute ticket
  • removed some comments and code that were accidentally included from PR5160
  • renamed variable tpe to tradePeriodEnd
  • renamed variable cptyName to counterpartyName
  • clear the new badge on double-click of dispute tablerow

@ghost
Copy link
Author

ghost commented Mar 16, 2021

👍 @ripcurlx I implemented latest requested changes.
No response from @huey735 or @leo816 about the process icon, & I prefer the cog.

@ghost ghost requested a review from ripcurlx March 16, 2021 12:45
@leo816
Copy link

leo816 commented Mar 16, 2021

that seems great, sorry I didn't answer earlier, having a lot of conversations at once and missed this. Good Job.

@huey735
Copy link
Member

huey735 commented Mar 16, 2021

That's amazing work @jmacxx
Regarding the Process icon, I share @ripcurlx 's sentiment. The cog makes me think of Settings. Maybe a gavel makes more semantic sense.

gavel002

Or if that's strong, maybe a balance?

balance

@ripcurlx suggestions don't seem thematically relevant. But then again I imagine that with time the chosen icon will gain meaning.

@ghost
Copy link
Author

ghost commented Mar 17, 2021

image

@pazza83
Copy link

pazza83 commented Mar 17, 2021

great work @jmacxx many thanks

@ripcurlx
Copy link
Contributor

Great work @jmacxx 👍

@ripcurlx ripcurlx merged commit 9270398 into bisq-network:master Mar 17, 2021
@ghost ghost mentioned this pull request Apr 2, 2021
@ghost ghost deleted the improve_dispute_chat branch May 29, 2022 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants