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

Add page_face #321

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

WesleyAC
Copy link
Collaborator

@WesleyAC WesleyAC commented Nov 9, 2023

This allows for organization of a large number of faces, by hiding some faces in a "page"

This seems like a better path forward for what I was trying to do with #314, although it's worth thinking at some point about how this might interact with something like #264 (since it currently wouldn't play nicely with that sort of system).

@WesleyAC WesleyAC mentioned this pull request Nov 18, 2023
@WesleyAC WesleyAC added the tested-on-hw This feature or pull request has been tested on a physical watch label Jan 23, 2024
Copy link
Collaborator

@matheusmoreira matheusmoreira left a comment

Choose a reason for hiding this comment

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

This is a feature I really want too, and this face is a really creative use of the control over scrolling. Looks good to me.

Before I found this PR, I thought about implementing it in a different way in the movement framework itself by adding a layer of indirection: the watch faces array would become an array of pages which is an array of watch faces, short pressing mode scrolls within a page while long pressing mode scrolls the pages. What do you think of this approach?

Seems you had a similar idea with PR #314. Why were you convinced that was not the right approach? It requires less memory than multiple page_face instances. Could be made to work with #264 too.

@chris-v8
Copy link

any chance this could make it to main before movement overhaul? I've been running this PR for few months now on my watch with zero issues. Works really good for hiding settings/preferences.

@matheusmoreira
Copy link
Collaborator

@chris-v8 The feature freeze for the refactoring is due in September. I can't promise it'll make it in but I'll try. I need to review all the code and referenced PRs and see what we can do. It's also something that will significantly change the product's user experience so I cannot merge it without discussing that first.

I can tell you this is something I want personally. Right now movement has two hardcoded pages that we switch between by holding the button. I feel like it should be simple to generalize this mechanism so that we can have any number of pages.

In any case, thanks for testing it on real hardware, it's greatly appreciated and if it does get merged I'll make sure everyone's names are in the commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested-on-hw This feature or pull request has been tested on a physical watch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants