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

Change message list to material 3 part 1 #7877

Merged
merged 3 commits into from
May 30, 2024

Conversation

wmontwe
Copy link
Member

@wmontwe wmontwe commented May 28, 2024

This removes customizations that don't work well with Material 3.

@wmontwe wmontwe requested a review from cketti as a code owner May 28, 2024 13:33
@wmontwe wmontwe added the type: UI/UX Visual or UX design issues label May 28, 2024
@cketti
Copy link
Member

cketti commented May 30, 2024

I can see that we need to change the approach to properly support this functionality with Material 3. Removing the functionality now and adding it back using a new approach later runs the risk of simply forgetting about it. That's why I'd much prefer a pull request that replaces the mechanism, not remove it.

I think having different text colors for read and unread messages is an important part of the user experience. Especially in dark mode.

Before After
k9mail__message_list__different_text_colors k9mail__message_list__same_text_color
k9mail__message_list_dark__different_text_colors k9mail__message_list_dark__same_text_colors

Copy link
Member

@cketti cketti left a comment

Choose a reason for hiding this comment

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

From a technical point of view the patch looks good to me.

However, I think it's a UX regression that should be fixed as soon as possible if this is merged as is.

@wmontwe
Copy link
Member Author

wmontwe commented May 30, 2024

The UX will have some regressions as there is no direct replacement for all the customization done in the existing theme.

There are also a couple of XML layouts that either don't use Material components or are even applying AppCompat styles still. Will take some time to remove all of that to get a clear view on all the little things that are not working.

@wmontwe wmontwe force-pushed the change-message-list-to-material-3-part-1 branch from 90ca2f4 to 667487d Compare May 30, 2024 15:06
@wmontwe wmontwe merged commit a8855ed into main May 30, 2024
2 checks passed
@wmontwe wmontwe deleted the change-message-list-to-material-3-part-1 branch May 30, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: UI/UX Visual or UX design issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants