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

Improved readability for the Clean Theme #62

Merged
merged 2 commits into from
Sep 24, 2023

Conversation

msoos
Copy link
Contributor

@msoos msoos commented Sep 17, 2023

All Submissions:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

This is a slight update to the clean theme. It's now even cleaner. In particular, the "timer" widget has color "SECONDARY1" but normal widgets have color "PRIMARY3". I wish I was making this up, but nope. So I changed all to 0x0. Also, the original clean theme I wrote had DISABLED as light red. I realized that light red didn't quite show that it was indeed red. So now it's much more full-on red. Much easier to see that things are wrong.

@msoos msoos changed the title Improved readability Improved readability for the Clean Theme Sep 17, 2023
@pfeerick
Copy link
Member

I thought I had written a comment yesterday, but it seems to have gone missing 🤷

I'm not against this PR being merged, just wondering if a fix is needed elsewhere.

You mention that the timer widget uses SECONDARY1, but others use PRIMARY3... were there some typos there in the numbers? As when you look at the source for the widgets, that doesn't seem to be the case...

SECONDARY1 is the default for the model info, outputs and text widgets. In the timer, it alternates between SECONDARY1 and SECONDARY2. It also alternates between PRIMARY2 and WARNING.

SECONDARY3 is used by model info and outputs as the default background color.

PRIMARY3 is only used in a new widget for 2.10 (the radio status icons normally on the right side).

Please keep in mind these are the default colors, so will not change if you make changes to the theme colors (i.e. allowing for user override)... you'd have to recreate the widgets in order for the colours to be set to the current themes colors.

@msoos
Copy link
Contributor Author

msoos commented Sep 23, 2023

Please keep in mind these are the default colors, so will not change if you make changes to the theme colors (i.e. allowing for user override)... you'd have to recreate the widgets in order for the colours to be set to the current themes colors.

Oh that's new! A bit surprising actually, but I understand the intent. I guess if unchanged from the default, it should change with the theme, but that's kinda Apple-level attention to detail. I understand that's not necessarily required :)

I think the changes are still needed, though. I observe an improved user experience.

@pfeerick
Copy link
Member

pfeerick commented Sep 24, 2023

No worries, glad was able to clear that up. I think the issue there is we don't keep track of what the "default" is... or more accurately, we just write the colour values at the time of widget creation, rather than go... "if the user didn't actually make a choice, write nothing, and then let the theme dictate the colours" ... 🤔

@pfeerick pfeerick merged commit b799589 into EdgeTX:main Sep 24, 2023
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants