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

Languages text not visible on 'Emilia' theme #269

Open
Skyune opened this issue Dec 18, 2022 · 4 comments
Open

Languages text not visible on 'Emilia' theme #269

Skyune opened this issue Dec 18, 2022 · 4 comments

Comments

@Skyune
Copy link

Skyune commented Dec 18, 2022

319870930_1620467885039459_6242354575181723104_n

@anettleship
Copy link
Contributor

anettleship commented Jan 13, 2023

Hello,

I think I can resolve this one.

Emilia is the only theme that implements a light tone for entryBackgroundColor and a dark tone for timelineBackgroundColor.

The colour of the text in the entry input dialogue and the language list is determined by entryBodyColor, but the entry input dialogue background colour is set by entryBackgroundColor and the language list background colour is set by timelineBackgroundColor here, so the text can contrast with the background set either in entryBackgroundColor or timelineBackgroundColor, but because these are set to be opposite tones in the Emilia theme, the text cannot contrast with both.

The simplest fix is to change the colour scheme of Emilia to have a dark entryBackgroundColor with light text.

My preference is for a solution which allows for the granularity to be retained, which I believe was the intention when the six new themes were added here.

One proposal would be to change the settings menu to take its background colour from entryBackgroundColor instead of timelineBackgroundColor here. This would have the effect of changing the background colour of the settings menu popup menus across all themes from one value to the other - they are similar tones and I don't believe it would break functionality, but it isn't perfect.

Another option is that I notice settingsBackgroundColor is specified in the new themes, but I can't see that it's implemented. For example, it is missing in themes.xml.

I'm going to look at whether I can implement this value where it is specified in the theme, to provide the additional granularity of colours.

I'm new to Android and Kotlin, so I'd very much appreciate confirmation that I've not missed anything essential, and also that this is the preferred direction to work in to solve the issue.

@anettleship
Copy link
Contributor

anettleship commented Jan 13, 2023

I've had a go at implementing this here. Will create a pull request when it's built out. Currently puzzling over how to write tests to check text to background contrast. Any guidance anyone can offer on that point would be well received!

@alisonthemonster
Copy link
Owner

Thanks @Skyune for reporting and thanks @anettleship for investigating!

I also recently got a report from a user about the backup dialog in the settings screen for the same theme:

Screenshot_20221016-092507

@anettleship I think making a change to the CustomListPrefDialogFragCompat like you pointed out would be great, they should either both be entry colors or timeline colors, I don't mind either way!

 attrs[0] = R.attr.timelineBackgroundColor
 attrs[1] = R.attr.entryBodyColor

@anettleship
Copy link
Contributor

anettleship commented Jan 23, 2023

Hello, I've created a pull request here. The Import CSV dialogue does not appear to me to be implemented in the same way as the Language and First Day of the Week - I couldn't see an easy fix. I'm going to create a new issue for this and resolve it separately.

alisonthemonster added a commit that referenced this issue Jan 29, 2023
Fix Issue: Languages text not visible on 'Emilia' theme #269
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants