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

Feature: Send message via user dialog #285

Merged

Conversation

FelberMartin
Copy link
Collaborator

Problem Description

As described in #213 the "Send message" button in the user profile dialog has been postponed.

This PR closes #213

Changes

  • Add button to User dialog
  • Adapted navigation to properly handle navigating back from the "Send message" conversation to the previous chat
  • Changes LocalVisibleMetisContextManager to being able to extract current metisContext, to avoid passing down the courseId all the time.

Steps for testing

  • Go to any chat
  • Click on a message
    • From yourself -> see that "Send message" button is not shown
    • From someone else, click on "Send message" -> navigated to DM chat (newly created in case it did not exist yet)
  • Navigate back (via Android back button) -> previous chat is shown
  • Click on a user mention in a post (or create a new one) -> see that navigation also works

@FelberMartin FelberMartin self-assigned this Jan 10, 2025
Copy link
Contributor

@julian-wls julian-wls left a comment

Choose a reason for hiding this comment

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

When clicking on the user profile picture in the chat it works as expected.
The navigation works great, good work!

When clicking on the user profile picture in the ConversationMemberList the app crashes with the following stacktrace:
image

One more comment on the dialog:
I think it would be nice to stick to the new role badges from now on and use this design (see iOS app):

image

@FelberMartin
Copy link
Collaborator Author

When clicking on the user profile picture in the ConversationMemberList the app crashes with the following stacktrace:

Good catch, this is fixed now :D

I think it would be nice to stick to the new role badges from now on and use this design (see iOS app):

I created a follow-up issue for this as this change should then also affect the ConversationMembersList and bloat the scope of this PR: #289

…er-dialog

# Conflicts:
#	feature/metis/conversation/src/test/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/BaseChatUITest.kt
#	feature/metis/conversation/src/test/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/ConversationProfilePictureUiTest.kt
Copy link
Contributor

@eylulnc eylulnc left a comment

Choose a reason for hiding this comment

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

Looking good!
Crash is fixed when clicking the image.
From yourself -> see that "Send message" button is not shown ✅
From someone else, click on "Send message" -> navigated to DM chat ✅
Navigate back -> previous chat is shown ✅
Click on a user mention in a post -> navigated to DM chat ✅

@FelberMartin FelberMartin added the ready for review This PR can be reviewed label Jan 11, 2025
Copy link
Contributor

@julian-wls julian-wls left a comment

Choose a reason for hiding this comment

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

It's looking good now, no more crash!
Code also lgtm.

@FelberMartin FelberMartin added ready to merge This PR can be merged and removed ready for review This PR can be reviewed labels Jan 16, 2025
@FelberMartin FelberMartin merged commit 271b6ea into develop Jan 17, 2025
5 checks passed
@FelberMartin FelberMartin deleted the feature/communication/send-message-via-user-dialog branch January 17, 2025 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge This PR can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Profile picture dialog: send message button
3 participants