EdgeTX / edgetx

EdgeTX is the cutting edge open source firmware for your R/C radio
https://edgetx.org
GNU General Public License v2.0
1.58k stars 333 forks source link

Wrong colors in menue Logical Switches #3866

Closed Ralf-W closed 1 year ago

Ralf-W commented 1 year ago

Is there an existing issue for this problem?

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

Activating a Logical Switch does change the color of the line. But the new color is that of the background of the screen and not the color for "Active".

Expected Behavior

Color changes should stay unchanged to V2.8.4: Activating a Logical Switch did change the color of the line to the "Active" color, giving a good contrast to the background of the screen.

Steps To Reproduce

Define any logical condition being normally False/Inactive. Make this condition become True/Active. Watch the coor of the line change to the color of the background of that screen: No contrast.

Version

2.9.0

Transmitter

Radiomaster TX16S / TX16SMK2, Other (Please specify below)

Operating System (OS)

No response

OS Version

No response

Anything else?

No response

pfeerick commented 1 year ago

@JimB40 What say you? Should the line be using ACTIVE instead of SECONDARY3, given there is an outline for the "ENABLED" square to separate it from the rest of the line? This may be relevant to the FM, INPUTS, MIXES, GV, LS and SF pages, as they all use SECONDARY3 to highlight the active/enabled line.

Where does it fit in with the theme structure doc, and does this need any changes made to it, since it hasn't been updated since 2.6? ;) This is not taking into account UIv3 ... that is a totally different topic. https://github.com/EdgeTX/themes/blob/main/structure.md

image

philmoz commented 1 year ago

This will be easy to change after the libopenui refactor.

My concern with the active color is it gets a bit overpowering when there is a lot of it. I also don't think it is as readable. Screenshot 2023-07-28 at 1 10 39 pm

Maybe reduced opacity would help? Screenshot 2023-07-28 at 1 10 17 pm

pfeerick commented 1 year ago

Yeah, I was thinking that also, but I do agree with the OP - we shouldn't be changing from the pre-established theme constants, as that effectively breaks ALL the themes. It is somewhat overpowering with the default theme, but at the same time, it is very clear what is active, and what isn't. That's why this may need to be tweaked for 2.9 to restore prior behaviour, and then done properly as part of the libopenui refactor.

philmoz commented 1 year ago

2.8 introduced this new style of active theming on the inputs and mixes pages.

2.9 extends this theming to GV, LS and SF pages.

Ralf-W commented 1 year ago

Highlighting of active Logical Switches in 2.8.4 2 8 4 default theme - inactive logical switchesjpg 2 8 4 default theme - active logical switches jpg

and now in 2.9.0 2 9 0 default theme -  inactive logical switch jpg 2 9 0 default theme -  active logical switch jpg

Active logical switches are not that highlighted any more.

Ralf-W commented 1 year ago

Sorry, I hit the wrong switch. :-(

Ralf-W commented 1 year ago

I think the Special Functions page looks clear and easy to read in 2.9.0. No color change necessary. But it would be nice, if active Logical Switches would be highlighted using the Active color in the Logical Switches page - as it was done in 2.8.4.

JimB40 commented 1 year ago

Yep now it drifted from initial idea ACTIVE was intended to show that feature is activated (not enabled)

So right usage: External RF is active and Trainer is Active screenshot_tx16s_23-07-28_22-03-57

FM0 is active screenshot_tx16s_23-07-28_22-04-05

LS01 is active (ON) screenshot_tx16s_23-07-28_22-04-40

Even better withe grid as you can clearly see which ones are ON screenshot_tx16s_23-07-28_22-05-17

CRSF Quad model is Active screenshot_tx16s_23-07-28_22-03-36

SF1 is not active SF2 & SF3 are active SF3 has this 'enable' option but this is indicated by checkin checkbox screenshot_tx16s_23-07-28_22-11-46

Wrong usage: Disarm line in Input is Active but it's not highlighted with ACTIVE screenshot_tx16s_23-07-28_22-04-20

If ACTIVE color is too strong (because there are themes that is not) we need to change this color (in this case yellow) to less vivid instead of applying transparency.

pfeerick commented 1 year ago

IMO, the best course of action atm is to correct it so that ACTIVE is used where SECONDARY3 is used in those places. If we want to at the same time, perhaps find a better color than that particular shade of yellow for the default active color. Or not, and leave it up to themes to set that.

MervDale commented 1 year ago

Leave as it was as it makes it more visible in bright light as to what is active and what is not

pfeerick commented 1 year ago

@philmoz Is there any chance you could craft something to restore this specifically for 2.9? With equivalent for 2.10 in the libopenui refactor PR.

In other words, active inputs and mixer lines, active SF, active LS should all be using the ACTIVE theme constant, not SECONDARY3. As the question of wheither it should be yellow or not is separate to whether it should be using the ACTIVE constant to highlight active elements. As per https://github.com/EdgeTX/edgetx/issues/3866#issuecomment-1656286506

philmoz commented 1 year ago

I was looking at this and I'm concerned with the way the active enable checkbox on an active SF would look.

I tried adding an additional border around the checkbox like this; but it's still clunky. Screenshot 2023-08-03 at 4 06 30 pm

I definitely think the yellow needs to be toned down.

philmoz commented 1 year ago

Also what about GVARS: Screenshot 2023-08-03 at 4 10 31 pm

pfeerick commented 1 year ago

Sorry, yes, GVARS also please :) The checkbox is somewhat of a nuisance, but it's a much lesser issue and can be lived with for now. It has it's own defined outline, so you just look inside it to see if it's active or inactive color.

philmoz commented 1 year ago

I've added the equivalent changes to the libopenui refactor PR for 2.10.

JimB40 commented 1 year ago

Guys let me draw those lists. We are drifting now from intial 2.8 design (color-wise) and we now land with diferent look & feel of same UI type on every screen.

Benchmark in 2.9 is MODEL>CURVES Screen. Although list items are vertical every other list should behave in the same manner. Design adn color-wise. List item is divided for "Label area" and "Data area".

There will be variation for list wit items with sub-items (lines) like MODEL>INPUTS but it will go the same design path with exception that user has to see that item is seleceted, and which sub-item is selected.

I will draw it.