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

settings: Add global widget selection #1959

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vkareh
Copy link
Contributor

@vkareh vkareh commented Jan 10, 2024

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

Copy link

github-actions bot commented Jan 10, 2024

Build checks have not completed. Possible reasons for this are:

  1. The checks need to be approved by a maintainer
  2. The branch has conflicts
  3. The firmware build has failed

@FintasticMan FintasticMan added the enhancement Enhancement to an existing app/feature label Jan 11, 2024
Copy link
Member

@FintasticMan FintasticMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is very useful, thanks!

@vkareh vkareh force-pushed the global-widget-config branch 3 times, most recently from ca33109 to 4eeadff Compare January 16, 2024 21:49
@minacode
Copy link
Contributor

minacode commented Jan 21, 2024

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

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

@vkareh
Copy link
Contributor Author

vkareh commented Jan 22, 2024

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

@minacode
Copy link
Contributor

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 vkareh force-pushed the global-widget-config branch 2 times, most recently from a02aef7 to 039506d Compare January 24, 2024 15:31
@vkareh
Copy link
Contributor Author

vkareh commented Jan 25, 2024

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
Copy link
Contributor

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

@vkareh
Copy link
Contributor Author

vkareh commented Feb 5, 2024

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

@vkareh vkareh force-pushed the global-widget-config branch 3 times, most recently from d54ae36 to 168d9ca Compare February 12, 2024 15:17
@vkareh vkareh force-pushed the global-widget-config branch 2 times, most recently from 49d5ec3 to efdeebf Compare February 19, 2024 01:10
@vkareh vkareh force-pushed the global-widget-config branch from efdeebf to 8f10997 Compare March 18, 2024 12:50
@vkareh vkareh force-pushed the global-widget-config branch from 8f10997 to eb9e02b Compare March 26, 2024 12:15
@vkareh vkareh force-pushed the global-widget-config branch from eb9e02b to a673de8 Compare April 12, 2024 12:59
@vkareh vkareh force-pushed the global-widget-config branch from a673de8 to 8e10a2d Compare May 13, 2024 14:55
@vkareh vkareh force-pushed the global-widget-config branch from 8e10a2d to c70d185 Compare June 10, 2024 14:34
@vkareh vkareh force-pushed the global-widget-config branch from c70d185 to 291920e Compare June 19, 2024 15:30
@vkareh vkareh force-pushed the global-widget-config branch 2 times, most recently from 5f365d9 to 42bc7d6 Compare August 19, 2024 17:07
@vkareh vkareh force-pushed the global-widget-config branch from 42bc7d6 to 3f8361a Compare August 22, 2024 15:52
@vkareh vkareh force-pushed the global-widget-config branch 2 times, most recently from 7e62cde to 3fda05d Compare September 23, 2024 13:05
@vkareh vkareh force-pushed the global-widget-config branch from 3fda05d to 8fff157 Compare October 3, 2024 12:35
@vkareh vkareh force-pushed the global-widget-config branch 2 times, most recently from e9bafa8 to d465d6c Compare October 28, 2024 14:46
@vkareh vkareh force-pushed the global-widget-config branch from d465d6c to 9ecfeca Compare November 20, 2024 14:18
@mark9064 mark9064 added this to the 1.16.0 milestone Nov 21, 2024
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.
@vkareh vkareh force-pushed the global-widget-config branch from 9ecfeca to 370b973 Compare December 4, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing app/feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants