-
Notifications
You must be signed in to change notification settings - Fork 1
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
Communication
: Fix Tagging other Users does not work
#71
Communication
: Fix Tagging other Users does not work
#71
Conversation
…atches the behaviour in iOS and WebApp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tested it and it works as expected. Good work!
The only thing I noticed is, that the preview doesn't get rendered after clicking on the preview button. However, this seems like a topic for another PR in my opinion as it is not really related to the topic of this PR and more related to markdown rendering.
Good to be merged from my side!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a UI test for this tagging behaviour. Also add an integration test that checks that the application can talk to the server properly.
@TimOrtel Could you elaborate, please? I am not sure what you mean by "the application can talk to the server properly". Should this just be a generic test whether the ConversationViewModel can communicate with a MockServer via |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add tests that verify:
- Initially the auto complete hints are not displayed
- when entering text that is not @ then the auto complete hints are not displayed
- when entering an @ the hints are displayed but when removing the @ again the hints are no longer displayed
- auto completing can be used twice in a row
...e/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ReplyTextFieldUiTest.kt
Outdated
Show resolved
Hide resolved
You can see exemplary integration tests in feature/metis/conversation/src/test/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ConversationAnswerMessagesE2eTest.kt |
...tics/www1/artemis/native_app/feature/metis/conversation/ConversationAutoCompletionE2eTest.kt
Outdated
Show resolved
Hide resolved
...tics/www1/artemis/native_app/feature/metis/conversation/ConversationAutoCompletionE2eTest.kt
Outdated
Show resolved
Hide resolved
...ormatics/www1/artemis/native_app/feature/metis/conversation/ui/reply/ReplyTextFieldUiTest.kt
Outdated
Show resolved
Hide resolved
...tics/www1/artemis/native_app/feature/metis/conversation/ConversationAutoCompletionE2eTest.kt
Outdated
Show resolved
Hide resolved
...ormatics/www1/artemis/native_app/feature/metis/conversation/ui/reply/ReplyTextFieldUiTest.kt
Outdated
Show resolved
Hide resolved
...tics/www1/artemis/native_app/feature/metis/conversation/ConversationAutoCompletionE2eTest.kt
Show resolved
Hide resolved
...tics/www1/artemis/native_app/feature/metis/conversation/ConversationAutoCompletionE2eTest.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good to me.
This resolves issue #65
Now using a different API endpoint for searching tag candidates (matching iOS and WebApp behaviour).
Out of Scope
While testing the implementation I found minor bugs / chances to improve the UI/UX, but I will create a separate issue #72 for this. This PR only focusses on the functionality.