-
Notifications
You must be signed in to change notification settings - Fork 99
settings_quick_launch: new combo back + up #707
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Federico Bechini <federico.bechini@gmail.com>
|
@asyba Thank you so much for doing this. Having a couple more quick-launch options that can be performed without even needing to look down at the watch would be so useful. I only own a Pebble 2 Duo right now. Have you tried this with a Round Pebble? Obviously, with the rectangular Pebble model this quick-launch pinch combo makes sense, as the buttons are opposite each other. I just wonder how it feels on the round Pebbles, with the back button in the center of the watch, and whether it works as well? I had forgotten that using select-back is the combo for reset, so using that combo for this quick-launch on the Round may not be possible. (It also probably helps to be consistent across models). As far as I am aware, these two system combos are currently used by PebbleOS itself:
Are there any others to be aware of? I concede that a multi-button quick-launch combo is probably not a feature that normal users are likely to ever use, not that it matters if they don't. Pebble watches tend to appeal more to geeky types, and those types of users are likely to find these really useful. I do hope that this gets merged. |
|
Also, how does this implementation adapt for left-handed users, who wear their Pebble on their right wrist, with watch and screen flipped? In this orientation, the combo would probably want to be back+down, (rather than still back+up) so the buttons remain opposite each other. |
|
|
||
| void quick_launch_app_menu_window_push_combo(void) { | ||
| QuickLaunchAppMenuData *data = app_zalloc_check(sizeof(*data)); | ||
| data->is_tap = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting is_tap = true for a hold-combo is confusing. Consider adding a comment explaining why, or restructuring the filter logic to be clearer.
| // Reset recognizer state to prevent stale state | ||
| ClickRecognizer *recognizer = &s_click_manager.recognizers[button_id]; | ||
| recognizer->is_button_down = false; | ||
| recognizer->number_of_clicks_counted = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Directly manipulating ClickRecognizer internal state is fragile. If button up events arrive after this reset, it could cause state corruption.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jplexer I was having problems where, after successfully executing the combo, the quick launches would generally be messed up. This helped me reset them and fix them.
I just try instead call click_manager_reset(&s_click_manager);, I did a quick test and it worked fine.
I also tried removing both and it works fine... maybe none reset is necessary.
Although I do notice the animations are slower without any kind of reset, I don't know if that's just my imagination from so much testing, haha.
Added a new Quick Launch button using a Hold with BACK+UP.