Hide sensorID with button #3150
Conversation
|
My preference would be the toggle under the buttons... IMO would look cleaner that way. Will comment further in a day or two once I've had a chance to play with it. |
pfeerick
added
color
|
Looks good. Travelling at the moment so I can't test. Maybe place the ID # above and to the left of the sensor number instead of above the value. |
|
Added support for NV14, moved sesnorID field to the first column, increased spacing and font for GPS. |
|
I think that the buttons should be above the telemetry sensors... quite annoying to scroll down with so many sensors? |
|
@Eldenroot I think we do not expect this button to be used often. |
|
Maybe like this?
|
|
I guess it's ok at the top... it does look out of place since it adds padding, and we have to watch reducing the height of the button as it makes it harder to touch, as well as add to clutter. Does it work better as a checkbox/toggle, or was that harder to add/didn't fit in? I'm still inclined to agree with JimB40 that it belongs below, out of the way - we have to also avoid cluttering the UI too much by "too much use of" empty space to the point that it's a busy UI. @Eldenroot You need to step back a moment and consider the frequency of use... viewing the telemetry screen data is more common than discovering sensors/adding sensors, hence why you place them lower down. You don't put stuff you don't use often at the top, so you need to scroll past it every time. Plus, it only takes a moment to scroll down, whether with touch or rotary encoder, even with 24+ entries in the list. |
|
Just not enough space for checkBox. Probably possible with some efforts. But I'm also not sure this button belongs to the top. |
|
Might have to watch out for the GPS values getting clipped on NV14/EL18... unless they are truncated, it looks like there can be two numbers after the decimal place... i.e. New York...
|
|
Simulator does not generate such numbers. Does it happen on the TX? |
pfeerick
force-pushed
the
eshifri/sensor_ID
branch
from
a0f488e to
a3d9a85
Compare
|
I removed the top button and moved the show sensor id checkbox down so it's next to the show instances checkbox, as well as putting the stub for translations in place. However, there is one minor bug present still... the alignment for the sensor values is off - it lines up with the id row, not the sensor row - most likely I chopped something out I should have? Also, I don't like the fact that the focus jumps to the top of the sensor table when you toggle either of those checkboxes, rather than stay at the current control (i.e. not such an issue for touch, but annoying with non-touch).
|
|
@philmoz Any chance you can finish this one out? I nearly (and should have) asked if you could incorporate this into #3399 since this already needed a rebase - and if I try now it really will get screwed up. The last comment still represents the current state of this PR, and the remaining issues present. |
It's not my PR. I can take a look if @eshifri is ok with it. |
|
Closed in favor of #3401, but thank you for the initial work and inspiration :) |
This PR implements the request from here: #3070 (comment)
Summary of changes:
@philmoz I hope you do not mind. If you do just let me know. If you have better ideas for implementation I will remove this PR.
@pfeerick do you agree with thee position of the button? I was thinking about having it together with "ignore" or in the very bottom of the screen - I would not expect people to use very often.
Translations are not implemented yet - I will add them if the rest looks OK.