-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add trader chat #2871
Merged
Merged
Add trader chat #2871
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On the way to adding chat for traders this is a first step. Mainly just moving functionality out of TraderDisputeView to Chat class. There are still remaining dispute functionality that needs to be factored away.
The naming of DisputeCommunicationMessage has to stay but they otherwise fit what would be more aptly named ChatCommunicationMessage or something in that spririt.
Add extra button for generically added button in chat
Move session classes to core. Break out DisputeCommunicationMessage handling from DisputeManager and put in ChatMananger prepare for other uses of ChatManager. Renaming of DisputeCommunicationMessage would be nice but it's representing the protobuf messages so the name has to stay.
- Add communication messages to Trade protobuf message to be able to save chat messages per trade - Add Type enum and field to DisputeCommunicationMessage protobuf to be able to dispatch Dispute and Trade chat messages properly - Rename some function as isClient instead of isTrader to make it easier to understand who is who when two traders are communicating with each other
Very basic switch between chat and overview per trade, improvements needed on the UI.
This was referenced Jun 7, 2019
This PR needs some unit tests. @sqrrm do you think there is a way to cherry pick most of it to master without enabling it? |
Yes, I tried to make the first commits only refactoring, then adding the chat functionality to traders, and then adding the UI. |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implementation of bisq-network/proposals#90