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

[SES-1754] Fix message menu icons not visible in light theme #1457

Merged

Conversation

bemusementpark
Copy link

before

Screen Shot 2024-04-13 at 12 11 12 am

after

Screen Shot 2024-04-13 at 12 25 23 am

@bemusementpark bemusementpark changed the title Fix message menu icons not visible in light theme [SES-1754] Fix message menu icons not visible in light theme Apr 15, 2024
@bemusementpark bemusementpark changed the base branch from master to dev April 15, 2024 00:28
Copy link
Collaborator

@AL-Session AL-Session left a comment

Choose a reason for hiding this comment

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

Works for all themes so thumbs up.

However, is the line saying, essentially: "Use the value of 'color', or if that's null then use the looked up color from the attribute"?

If so, when would we expect color to be null? (it's set as item.color?.let { ContextCompat.getColor(context, it) }) - wouldn't it make more sense to just use the lookup from attribute always, and then later it gets reset for the title & subtitle etc. anyways...

Entirely up to you, but just curious about the thinking behind it! :)

@bemusementpark
Copy link
Author

I'm using textColor as the default if no custom color is specified e.g. delete

@bemusementpark bemusementpark merged commit c64868d into oxen-io:dev Apr 16, 2024
1 check passed
@bemusementpark bemusementpark deleted the fix-missing-icon-light-theme branch April 16, 2024 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants