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

Improvement: Adjust conversation overview alignments and styling #227

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

Conversation

eylulnc
Copy link
Contributor

@eylulnc eylulnc commented Dec 15, 2024

Problem Description

The current channel structure in the Android app lacks visual clarity and readability. All channels and their titles are aligned in the same way, which makes it difficult for users to differentiate between section headers (e.g., "General," "Lectures," "Direct Messages") and the channels listed under them. This results in a poor user experience, especially when the list of channels grows longer.

Changes

Indentation for Channels: Implemented a visual indentation for channels under their respective section headers to indicate hierarchy clearly.
Header Styling: Section titles (e.g., "General," "Lectures") are now bold and stand out visually, acting as clear headers for the channel groups.
iOS Consistency: Aligned the Android design with the iOS app's visual structure to ensure consistency across platforms.

Steps for testing

  1. Open the Communication tab in the Android app.

  2. Verify the following:
    -- Section titles like "General", "Lectures", and "Direct Messages" are bold and clearly distinguishable.
    -- Channels listed under each section are indented, visually grouping them with their respective headers.
    -- The spacing between channels and headers is visually compact and consistent.

  3. Verify that unread message indicators still appear correctly.

  4. Verify channels with unread messge are bold and have blue indicator

  5. Check that dropdown menus for channel actions (e.g., "Favorite," "Hide," "Mute") are still functional.

Screenshots

  • Android Old
Screenshot 2024-12-07 at 20 53 50
  • Android New
Screenshot 2024-12-22 at 14 43 55
  • iOS
Screenshot 2024-12-15 at 21 37 57
  • Foldable
Screenshot 2024-12-22 at 14 39 43
  • Tablet
Screenshot 2024-12-22 at 14 42 16

@eylulnc eylulnc linked an issue Dec 15, 2024 that may be closed by this pull request
@eylulnc eylulnc self-assigned this Dec 15, 2024
@eylulnc eylulnc added the ready for review This PR can be reviewed label Dec 16, 2024
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.

I just tested it. Didn't experience any issues. Well done! Just two comments:

  1. Can we decrease the padding between the channels, so that they are closer together, like in the iOS app?
  2. See below

import androidx.compose.ui.graphics.Color

object CommunicationColors {
private val ArtemisMainBlue = Color(0xFF3E8ACC)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should use MaterialTheme.colorScheme.primary here to keep it consistent with the other blue elements.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also discussed this with Eylül on Tuesday, I will try to see whether we can get closer to the "original" artemis colors by updating the Material 3 theme with a different seed color and / or different settings. As soon as I know more (hopefully today, but latest tomorrow), I will reach out to @eylulnc and discuss how we can tackle this :)

Copy link
Collaborator

@FelberMartin FelberMartin Dec 19, 2024

Choose a reason for hiding this comment

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

So I checked with the official Material theme builder and using the ArtemisMainBlue color as a seed color (for the current theme used in the app, we used a slightly different seed color). Depending on whether you use the "Color match" option, the results looks like this:

Without Color match

image

With Color match

image

With the color match option enabled, we get quite close to the seed color by using the PrimaryContainer color.

In my opinion, this might be a good compromise between bringing the mobile app appearance closer together, while still complying to the Android Material3 design guidelines and keeping an Android look and feel. What do you guys think about that @eylulnc, @julian-wls ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it look great, colors are more vibrant and it feels closer to the iOS as well. Also as you said we are still match to the Material3.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would be a great solution.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome, then lets use this solution and primaryContainer as the color for this PR, I will create a PR in the meantime to update the Theme accordingly :D

@FelberMartin FelberMartin changed the title Bugfix/communication/change channel alignments Improvement: Adjust conversation overview alignments and styling Dec 19, 2024
Copy link
Collaborator

@FelberMartin FelberMartin left a 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 works like a charm, this is a big improvement for the app :D

I noticed that the little blue dot for a conversation with unread message is just displayed on the channel icons but not for group chats. This would also be a nice addition to the profile pictures of DMs (I try to merge my PR #214 for that hopefully before lunch today, but I think you will get some merge conflicts with that, sorry for that)

@FelberMartin
Copy link
Collaborator

I just thought of this now, but could you also please verify that these changes work on Tablet and Foldable aswell? :D

@eylulnc
Copy link
Contributor Author

eylulnc commented Dec 19, 2024

Just tested it and works like a charm, this is a big improvement for the app :D

I noticed that the little blue dot for a conversation with unread message is just displayed on the channel icons but not for group chats. This would also be a nice addition to the profile pictures of DMs (I try to merge my PR #214 for that hopefully before lunch today, but I think you will get some merge conflicts with that, sorry for that)

No problem at all 😂 I will check it out for group chat as well

@eylulnc
Copy link
Contributor Author

eylulnc commented Dec 19, 2024

I just tested it. Didn't experience any issues. Well done! Just two comments:

  1. Can we decrease the padding between the channels, so that they are closer together, like in the iOS app?
  2. See below

For the padding I tried a lot and could not manage the make it smaller interestingly but I will check it out again in the weekend

…into bugfix/communication/change-channel-alignments

# Conflicts:
#	feature/metis/manage-conversations/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/manageconversations/ui/common/ChannelIcons.kt
#	feature/metis/manage-conversations/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/manageconversations/ui/conversation/overview/ConversationList.kt
…nel-alignments' into bugfix/communication/change-channel-alignments
@eylulnc
Copy link
Contributor Author

eylulnc commented Dec 22, 2024

I just tested it. Didn't experience any issues. Well done! Just two comments:

  1. Can we decrease the padding between the channels, so that they are closer together, like in the iOS app?
  2. See below

For the padding I tried a lot and could not manage the make it smaller interestingly but I will check it out again in the weekend

@julian-wls @FelberMartin I have check it currently height of the row is 56dp which is define by ListItem itself by material I can force it to make it smaller but currently it is not depending to us. Do we want to stay align with material or make it smaller?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR can be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change channel alignments
3 participants