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.

Sign up for GitHub

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

Jump to bottom

Communication: Show profile picture #111

Open
wants to merge 41 commits into
base: develop
Choose a base branch
from
Open

Communication: Show profile picture #111

wants to merge 41 commits into from

Conversation

FelberMartin
Copy link
Collaborator

@FelberMartin FelberMartin commented Nov 16, 2024 •
edited
Loading

Problem

Currently profile pictures are not visible in the android app (see #63). This is the first of two planned PRs for this feature. This PR solely focusses on the minimal functionality by adding the profile pictures to the chat.

Changes

  • Added authorImageUrl to post entities
  • Created ProfilePictureImageProvider
  • Showing ProfilePicture instead of AuthorRoleIcon
    • Added AuthorRoleIcon in front of the name
    • As a follow up this should be adapted to iOS, I feel these changes would be to big for this PR

Screenshots

image

Followup

Add Profile pictures 2:

  • Create onClick dialog for profile pictures (see iOS)
  • To the conversation header
  • To the Conversation overview
  • To notifications

Adjust the message view to iOS:

  • Show cards around messages
  • Replace authorRole icon with badge above the user name
  • Move post time to the right

@FelberMartin FelberMartin self-assigned this Nov 16, 2024
@FelberMartin FelberMartin marked this pull request as draft November 16, 2024 10:50
@FelberMartin FelberMartin marked this pull request as ready for review November 18, 2024 11:05
@FelberMartin FelberMartin added the ready for review This PR can be reviewed label Nov 18, 2024
julian-wls
julian-wls approved these changes Nov 19, 2024
View reviewed changes
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.

Great feature, the results look really nice.
I didn't experience any issues in connection with these changes.
I found some minor things, which might be worth discussing, see below.
Overall good, approved from my side.

.../informatics/www1/artemis/native_app/feature/metis/conversation/ui/chatlist/MetisChatList.kt Outdated Show resolved Hide resolved
...nformatics/www1/artemis/native_app/feature/metis/shared/ui/profile_picture/ProfilePicture.kt Show resolved Hide resolved
TimOrtel
TimOrtel requested changes Nov 25, 2024
View reviewed changes
...tum/informatics/www1/artemis/native_app/core/ui/remote_images/ProfilePictureImageProvider.kt Outdated Show resolved Hide resolved
...in/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/post/PostItem.kt Outdated Show resolved Hide resolved
...in/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/post/PostItem.kt Show resolved Hide resolved
...in/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/post/PostItem.kt Outdated Show resolved Hide resolved
...in/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/post/PostItem.kt Show resolved Hide resolved
...nformatics/www1/artemis/native_app/feature/metis/shared/ui/profile_picture/ProfilePicture.kt Outdated Show resolved Hide resolved
...nformatics/www1/artemis/native_app/feature/metis/shared/ui/profile_picture/ProfilePicture.kt Outdated Show resolved Hide resolved
...nformatics/www1/artemis/native_app/feature/metis/shared/ui/profile_picture/ProfilePicture.kt Outdated Show resolved Hide resolved
...matics/www1/artemis/native_app/feature/metis/shared/ui/profile_picture/ProfilePictureData.kt Show resolved Hide resolved
...matics/www1/artemis/native_app/feature/metis/shared/ui/profile_picture/ProfilePictureData.kt Show resolved Hide resolved
@FelberMartin FelberMartin removed the ready for review This PR can be reviewed label Nov 25, 2024
@FelberMartin FelberMartin mentioned this pull request Nov 26, 2024
Open
@FelberMartin FelberMartin force-pushed the feature/communication/show-profile-picture branch from 94ef35b to c0473c4 Compare November 26, 2024 19:09
@FelberMartin
Merge remote-tracking branch 'origin/bugfix/communication/attachments…
715d199 
…-rendering' into feature/communication/show-profile-picture

# Conflicts:
#	feature/metis/conversation/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/chatlist/MetisChatList.kt
#	feature/metis/conversation/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ui/thread/MetisThreadUi.kt
#	feature/metis/conversation/src/test/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/ConversationAnswerMessagesUITest.kt
@FelberMartin
Started using CompositionLocal; introduced ArtemisImageProvider
29389c4
@FelberMartin FelberMartin mentioned this pull request Nov 28, 2024
Merged
@FelberMartin FelberMartin mentioned this pull request Nov 28, 2024
Open
@FelberMartin FelberMartin added the ready for review This PR can be reviewed label Nov 29, 2024
@FelberMartin FelberMartin added the database changes This PR includes database changes and should be treated with extra care when merging label Nov 29, 2024
FelberMartin and others added 2 commits November 29, 2024 14:24
@FelberMartin
Merge branch 'develop' into bugfix/communication/attachments-rendering
75e2513
@FelberMartin
Merge remote-tracking branch 'origin/develop' into feature/communicat…
edc6183 
…ion/show-profile-picture

# Conflicts:
#	feature/metis/conversation/src/test/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/conversation/service/storage/impl/MetisStorageServiceImplTestUpgradeLocalAnswerPost.kt
@FelberMartin FelberMartin linked an issue Nov 29, 2024 that may be closed by this pull request
Communication: Profile pricures are not shown in the android app #63
Open
@FelberMartin FelberMartin mentioned this pull request Nov 29, 2024
Open
7 tasks
Base automatically changed from bugfix/communication/attachments-rendering to develop November 30, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julian-wls julian-wls julian-wls approved these changes

@TimOrtel TimOrtel Awaiting requested review from TimOrtel

Requested changes must be addressed to merge this pull request.

Assignees

@FelberMartin FelberMartin

Labels
database changes This PR includes database changes and should be treated with extra care when merging ready for review This PR can be reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Communication: Profile pricures are not shown in the android app
3 participants
@FelberMartin @julian-wls @TimOrtel