search

InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.71k stars 925 forks source link

settings: Add global widget selection #1959

Open vkareh opened 8 months ago

vkareh commented 8 months ago

Instead of each watch face implementing their own settings for which widgets to display, we can have a global selection of widgets. All watch faces can then determine whether it is enabled and so display it in whichever way makes sense for that face.

Current widgets supported are heart rate, step counter, and weather.

Widgets off: InfiniSim_2024-01-10_161937 InfiniSim_2024-01-10_161850 InfiniSim_2024-01-10_161904 InfiniSim_2024-02-05_134208

Widgets on: InfiniSim_2024-01-10_162123 InfiniSim_2024-02-05_134638 InfiniSim_2024-02-05_134518 InfiniSim_2024-02-05_134257

github-actions[bot] commented 8 months ago
Build size and comparison to main: Section Size Difference
text 376044B 1812B
data 948B 0B
bss 63488B 8B
minacode commented 8 months ago

I think you could use the CheckboxList class for the settings menu.

You can find it in src/displayapp/screens/CheckboxList.h.

vkareh commented 8 months ago

@minacode the badly-named CheckboxList class only deals with single-selection settings (radio buttons)

minacode commented 8 months ago

Oh, ok. Maybe we should rename it to something like RadioButtonGroup.

Do you think you could still extract an abstract component from your implementation? I could imagine that an actual checkbox list is common enough.

Maybe it doesn't make sense though. That would be ok. I am just asking.

vkareh commented 8 months ago

Oh, ok. Maybe we should rename it to something like RadioButtonGroup.

No, the name is based on the lvgl library, which uses checkbox as the naming. There is no radio component in lvgl, so the checkboxlist class emulates the radio behaviour by re-theming the checkboxes and overriding the select mechanism.

Do you think you could still extract an abstract component from your implementation? I could imagine that an actual checkbox list is common enough.

I did an initial PoC, but it means that now CheckboxList needs to return arrays of settings, whether it's a checkbox or a radio. This and the additional code required to make that work makes the entire thing to complex/bloated.

Maybe it doesn't make sense though. That would be ok. I am just asking.

I can poke at it some more at another time, but likely out of scope for this PR.

minacode commented 8 months ago

Ok, nice. Thanks for the detailed answer. Given that, I think your PR is fine the way it is now :)

vkareh commented 7 months ago

I've added new commits for configurable widgets on the Analog watch face. Attached screenshots to the description on top.