Skip to content

Conversation

@coxtor
Copy link
Contributor

@coxtor coxtor commented Sep 30, 2021

This is a cleaner reworked version of #487 and adds a favorite option or opens the last used app when swiping left on the watch face.

favorites.mp4
  1. Hold an app to select/deselect it as a favorite app
  2. Swipe left on the watch face to open the favorite app
  3. If no favorite app is defined, the swipe left gesture opens the last used app

@kieranc
Copy link
Contributor

kieranc commented Oct 1, 2021

Thanks very much for recreating this - sadly after some discussion there might be a better approach (Issue #703) however I'd love to help you find out why it doesn't work right as a learning exercise :-) (I don't know either, we can both learn!)

@coxtor
Copy link
Contributor Author

coxtor commented Oct 1, 2021

Thanks very much for recreating this - sadly after some discussion there might be a better approach (Issue #703) however I'd love to help you find out why it doesn't work right as a learning exercise :-) (I don't know either, we can both learn!)

I have been thinking about this too - however I do think that having a favorite quick to access app still is beneficial. I'll try to setup a proper debugging setup on the weekend, right now I am programming blindly 😅.

Copy link

@tmilburn tmilburn left a comment

Choose a reason for hiding this comment

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

Overall structure of the code looks good but needs a few fixes.

@tmilburn
Copy link

tmilburn commented Oct 1, 2021

I like the idea but I feel that from a usability point of view it may be beneficial to use the swipe left to go the last opened app.

@coxtor
Copy link
Contributor Author

coxtor commented Oct 1, 2021

Thank you for looking at the code. Well if the majority feels that the swipe gesture should take you back to the last used app, there is no point on continuing work on this PR? Or would it make sense to make this feature configurable. However, that might clutter the settings long term.

@Riksu9000
Copy link
Contributor

There's an issue inherent to swiping left. The transitions get confusing.

VID_20211001_125314.mp4

If the screen moves right, it should move back left at some point to reach the clock. The same with going up. If the return gesture was a swipe right, then that would make return gestures inconsistent across the system. Maybe the button would be a better shortcut after all? #480

issue

Also replacing the app icon with an asterisk isn't the best way to display the favorite app. Instead the button color could be changed, or the star could be added to a corner, but without replacing the existing icon.

@0x1a8510f2
Copy link

Thank you for looking at the code. Well if the majority feels that the swipe gesture should take you back to the last used app, there is no point on continuing work on this PR? Or would it make sense to make this feature configurable. However, that might clutter the settings long term.

I personally prefer this PR over it being the last app, but I'm also for making this configurable. That could be as simple as including a "last used app" at the top of the favourite app selector.

@geekbozu geekbozu added enhancement Enhancement to an existing app/feature feature request labels Oct 3, 2021
@hubmartin
Copy link
Contributor

@coxtor Tried it and it is awesome, thanks.
Regarding the swiping/screen moving logic. I don't mind if the swiping logic to close app (swipe down) and moving screen (to the left) is not in the same direction. But exiting the app should go to the watchface and not the menu. Could this be somehow improved? 👍

@coxtor
Copy link
Contributor Author

coxtor commented Oct 3, 2021

Thanks for the feedback so far.
@hubmartin I have a private seperate branch, where the function of the pushbutton can be configured implementing exactly what you have asked for.
Right now I am not sure how much time I should continue to invest as the opinions are controversial on how or if this should be implemented.

@hubmartin
Copy link
Contributor

I feel that this PR is perfect and already usable. The only small issue I see is the exiting from the Favorite application which should go back to the watchface when you press the button or swipe down in the apps, that supports swipe to exit.

I played with it for a little bit and It seems that with using ReturnApp you can do it easily to go back to Apps::Clock.

case TouchEvents::SwipeLeft:
  favoriteApp = settingsController.GetFavoriteApp();
  if (favoriteApp == Apps::None)
    LoadApp(Apps::SettingFavoriteApp, DisplayApp::FullRefreshDirections::LeftAnim);
  else
    LoadApp(favoriteApp, DisplayApp::FullRefreshDirections::LeftAnim);
  ReturnApp(Apps::Clock, FullRefreshDirections::RightAnim, TouchEvents::SwipeDown);
  break;     

I would keep this PR simple. Adding long press in app menu to select favorite app or highlightning could be improved later in another PR.

@coxtor
Copy link
Contributor Author

coxtor commented Oct 3, 2021

@coxtor Tried it and it is awesome, thanks. Regarding the swiping/screen moving logic. I don't mind if the swiping logic to close app (swipe down) and moving screen (to the left) is not in the same direction. But exiting the app should go to the watchface and not the menu. Could this be somehow improved? +1

@hubmartin checkout PR #717

@Riksu9000
Copy link
Contributor

@coxtor Tried it and it is awesome, thanks. Regarding the swiping/screen moving logic. I don't mind if the swiping logic to close app (swipe down) and moving screen (to the left) is not in the same direction. But exiting the app should go to the watchface and not the menu. Could this be somehow improved? +1

@hubmartin checkout PR #717

The issue can be seen in the video I posted. When I swipe down, it goes to the app menu instead of the watch face. The issue isn't the button, but ReturnApp().
#708 (comment)

@hubmartin
Copy link
Contributor

hubmartin commented Oct 3, 2021

I agree with @Riksu9000 that #717 does not solve it. It is not a configuration issue of the button but program issue which depends:

  • if you run the app from the menu - then exiting app should go back to the menu
  • if you run app by swipe left shortcu - then exiting the app should go back to the wachface (which is the only place you can do that swipe left gesture and run favorite app)

So the both cases above are simply solved by ReturnApp I mentioned above.

For the #717 I'll reply to that thread, since it really doesn't apply to this Favorite app thread.

@coxtor
Copy link
Contributor Author

coxtor commented Oct 3, 2021

I see, now I understand what the issue was. The current changes should fix the behavior making #717 obsolete.
@Riksu9000 for now the favorite is indicated with icon + * . Unfortunately the button matrix does not allow the styling of individual buttons. An option would be to use individual buttons at the cost of more memory consumption, which is not the best option imho.

Still to do:

  1. remove the settings pane in favor of the "long press to select favorite" option. For this I need to understand why saving the favorite with the buttonid used to launch the app causes a crash - here advise would still be welcome
  2. if the setting page is removed - a nice option to select "last used app" as favorite is needed using the long press selection method. Any inspiration here?

@hubmartin
Copy link
Contributor

hubmartin commented Oct 3, 2021

I'm still not sure that favorite app should have any highlight in the menu. I'm thinking that #480 has also shortcuts like button doubleclick for starting app - would you think this also needs to be somehow highlighted in the menu? What about looong button press in the same PR - now we would have 3 highlighted apps in the menu.

But this brings me the idea - what if you long press on the item in the menu (or in the Settings) and "dialog" appears and will contain this:

Set current app as:

  • Favorite
  • Button Doubleclick
  • Button Loong press
  • (Hide app from the menu for future? Or from the FLASH when we will be able to load them from external FLASH)

This might be really clean solution and we:

  • get rid of separate settings selections UI
  • we wouldn't duplicate code and use more FLASH
  • it could be used for items also in the Settings menu. Which could be awesome since I change Wakeup or Display timeouts quite often and having that on shortcut will be awesome.

What do you think?

@Riksu9000
Copy link
Contributor

Unfortunately the button matrix does not allow the styling of individual buttons

It does allow styling by setting the state to checked at least.
https://docs.lvgl.io/latest/en/html/widgets/btnmatrix.html#simple-button-matrix

if the setting page is removed - a nice option to select "last used app" as favorite is needed using the long press selection method. Any inspiration here?

When nothing is selected, it could go to the last used app.

I like @hubmartin's idea for a new menu on long pressing an app icon. The longer press button action should probably be saved for a power menu, but this makes sense for the double click, and maybe another shortcut for regular long press on the watch face?

@hubmartin
Copy link
Contributor

saving the favorite with the buttonid used to launch the app causes a crash - here advise would still be welcome

Can you point me to the place in the code where I should start investigate that? Whether is it settings or somewhere in the app list? Not familiar with that areas yet.
Do you have a debugger?

@coxtor
Copy link
Contributor Author

coxtor commented Oct 3, 2021

saving the favorite with the buttonid used to launch the app causes a crash - here advise would still be welcome

Can you point me to the place in the code where I should start investigate that? Whether is it settings or somewhere in the app list? Not familiar with that areas yet. Do you have a debugger?

I have a stlink-v2 and the pinetime dev kit, however I haven't managed to set it up yet.

It would be great to fix the crash to have an initial implementation before we start with the modal version with the new button handler.

The crashing line is in src/displayapp/screens/Tile.cpp line 153. From what I can understand without a debugger I am using the exact same expression to store the favorite app as the one used to start the app (apps[buttonId]), the datatype is identical. If I use a static number as index like apps[2], it works without a problem. So something is up with buttonId.

        app->StartApp(apps[buttonId], DisplayApp::FullRefreshDirections::Up);
        running = false;    
      break;
    case LV_EVENT_LONG_PRESSED:
      currentFavoriteApp = settingsController.GetFavoriteApp();
      if(buttonId){
        if(currentFavoriteApp == apps[buttonId]){
          settingsController.SetFavoriteApp(Apps::None);
        }
        else{
          settingsController.SetFavoriteApp(apps[buttonId]); <---------------------------------- This line is the issue, where as apps[2] works fine.
        }   ```

@coxtor
Copy link
Contributor Author

coxtor commented Oct 3, 2021

@Riksu9000 thanks for pointing that out, it looks a lot better highlighting the favorite using the active select state, also now the last app used app is loaded, if no favorite is set.

Finally, if @hubmartin manages to understand what the crashing issue is it should be fine until we refine this to work with the updated button handler. As for that - is there already an implementation that can share data with other screens or would this need to be implemented too ( sending and retrieving information from the dialog window)

@coxtor
Copy link
Contributor Author

coxtor commented Oct 6, 2021

Ah now I understand, so you mean basically adding the hold to set favorite option in the settings menu too? That can be easily be done.

@hubmartin
Copy link
Contributor

Exactly, should be practically the same code with apps but adds more funcionality and options.

@coxtor
Copy link
Contributor Author

coxtor commented Oct 6, 2021

@hubmartin check it out if you like =)

@hubmartin
Copy link
Contributor

@coxtor Absolutely awesome! Thanks for adding.
Noticed just small issue. When I choose one option in the settings, then in becomes green. Then when I choose item on the same screen, then the new selected settings becomes green, but the previous item stays green too. When I scroll to the next settings page and back then only the last item is correctly highlighted. Seems like some refresh is missing? In the App menu there is no such issue.
Thanks for these improvements!

@hubmartin
Copy link
Contributor

hubmartin commented Oct 6, 2021

One small thing. There is no vibration when the item in Settings is selected. I might not be needed because items are big and green selection can be seen perfectly fine, but for consistency I would add it too.

I love how fast I can change faces! :)

pine-swipe-left.mp4

@coxtor
Copy link
Contributor Author

coxtor commented Oct 7, 2021

@hubmartin thanks for highlighting those bugs! Should be ready to merge now ;) @JF002 ?

Copy link
Contributor

@Riksu9000 Riksu9000 left a comment

Choose a reason for hiding this comment

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

A lot of the code should be reformatted with clang-format.

Also long pressing on About crashes the firmware.

I can see how this could be useful, but I think a couple things require attention.

Which apps should be considered for previous app? I'd think only the apps in the app menu, but right now it could open many other screens.

Then I think the issue I mentioned in #708 (comment) should still be considered, because weird transitions will be disorienting. I wonder if it would be bad if the return animation was in a different direction than the swipe. Nevertheless this will create some inconsistency, and using the button instead of a swipe left as the shortcut doesn't have these issues, but might not be quite as convinient? #480.

} else {
if (!currentScreen->OnButtonPushed()) {
LoadApp(returnToApp, returnDirection);
if(favoriteAppActive){
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure all of these favoriteAppActive things aren't necessary, because they were added before ReturnApp() was properly set ccd3742

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The favoriteAppActive is necessary because otherwise I cant set the animation direction correctly. Again I may be missing your point as of #708 (comment)

}
else {
LoadApp(favoriteApp, DisplayApp::FullRefreshDirections::LeftAnim);
ReturnApp(Apps::Clock, FullRefreshDirections::Down, TouchEvents::SwipeDown);
Copy link
Contributor

Choose a reason for hiding this comment

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

This sets the SwipeDown return gesture for all apps, even those that disable it because it causes problems, like Metronome. Also we should discuss if the animation direction should be left or down because of this issue #708 (comment)

Comment on lines 442 to 444
if (currentApp != Apps::Clock && currentApp != Apps::Launcher && currentApp != Apps::QuickSettings){
previousApp = currentApp;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The previous app can also be the settings list, any settings menu technically and flashlight at least. I think the previous app should only be one of the apps in the app menu.

@coxtor
Copy link
Contributor Author

coxtor commented Oct 8, 2021

A lot of the code should be reformatted with clang-format.

I think it is corrected in the latest update

Also long pressing on About crashes the firmware.

Fixed

I can see how this could be useful, but I think a couple things require attention.

Which apps should be considered for previous app? I'd think only the apps in the app menu, but right now it could open many other screens.

The initial implementation was exactly like this, but some users may want to be able to have the flexibility to use any app? My mind is not set on that topic, I guess the majority should decide? We could also decide on a whitelist of some sort?

Then I think the issue I mentioned in #708 (comment) should still be considered, because weird transitions will be disorienting. I wonder if it would be bad if the return animation was in a different direction than the swipe. Nevertheless this will create some inconsistency, and using the button instead of a swipe left as the shortcut doesn't have these issues, but might not be quite as convinient? #480.

I thought I already have considered that comment - the return animation takes you back from the previous swipe. Could you perhaps make more clear what you would like to see. I don't understand the issue here as I think it integrates well with the current integrations.

@Riksu9000
Copy link
Contributor

Which apps should be considered for previous app? I'd think only the apps in the app menu, but right now it could open many other screens.

The initial implementation was exactly like this, but some users may want to be able to have the flexibility to use any app? My mind is not set on that topic, I guess the majority should decide? We could also decide on a whitelist of some sort?

Not favorite app, but previous app. It's was very unexpected for me when it went to the settings list and flashlight, because they don't seem like regular apps from a users perspective.

Then I think the issue I mentioned in #708 (comment) should still be considered, because weird transitions will be disorienting. I wonder if it would be bad if the return animation was in a different direction than the swipe. Nevertheless this will create some inconsistency, and using the button instead of a swipe left as the shortcut doesn't have these issues, but might not be quite as convinient? #480.

I thought I already have considered that comment - the return animation takes you back from the previous swipe. Could you perhaps make more clear what you would like to see. I don't understand the issue here as I think it integrates well with the current integrations.

The issue is still similar to what happens in the video when swiping down, the app menu wasn't the main issue, and the drawing is still relevant #708 (comment). I've suggested using the button instead as the shortcut, which would require #480. The issue is inherent to the current design and can't easily be fixed otherwise.

I just noticed the previous app can also be the notification, which it really shouldn't be.

@coxtor
Copy link
Contributor Author

coxtor commented Oct 8, 2021

I just noticed the previous app can also be the notification, which it really shouldn't be.
Fixed, I created a blacklist for return apps in the latest commit.

The issue is still similar to what happens in the video when swiping down, the app menu wasn't the main issue, and the drawing is still relevant #708 (comment). I've suggested using the button instead as the shortcut, which would require #480. The issue is inherent to the current design and can't easily be fixed otherwise.

I think I understand now. To sum it up: The swipe down gesture "misses" a slot in a animation loop? I don't think using a button is convenient in the users flow.
I don't feel that the case of swiping down in the favorite/last used app section is a deal breaker when navigating it does not feel awkward to me.
Are you suggesting to show the swipe right animation, when swiping down in the favorites view?

@hubmartin
Copy link
Contributor

I understand what @Riksu9000 means, but to me @coxtor solved it the best way he could. When swiping left to open favorite settings/app and then:

  • button does animation right - this does "close" the animation loop
  • swipe down gesture does animation down, this is what I guess Riksu refers to that it breaks "loop". I don't see the issue there. It would be looked much more wrong, if I do swipe down gesture and the movement is left.

I guess that everyone will have a different opinion. But for me I do not count screens and I don't feel even a bit disturbed by the not complete "loop". My expectation is that almost every app closes with gesture swipe down and I expect when I do that motion that the animation will have same down direction as my gesture.

@lman0
Copy link

lman0 commented Oct 29, 2021

any way to review this very good pull (@Avamander or @geekbozu )?
i tested it , and it 's good !
because it feel like that the change requested by @Riksu9000 is more a personal preference than an actual blocker
so it should not block this pull to be merged /approved

@Riksu9000
Copy link
Contributor

The issue is real. Whether we choose to ignore it or not is the question. I don't think it would be very professional of us to simply ignore it.

I'm sure this feature can be useful and I hope we get to test these shortcuts with #782 button doubleclick action.

@Riksu9000 Riksu9000 mentioned this pull request Oct 30, 2021
@kieranc
Copy link
Contributor

kieranc commented Oct 30, 2021

I tested this PR briefly and got confused at first because I didn't read how it was meant to work and it was just launching seemingly random stuff when I swiped left. We have to assume the users will also not read about how it works so I don't think this is ideal behaviour, maybe if no favourite is set, it could display a message explaining how it works?
This is a nice feature, it works, but I can't help but feel maybe just the ability to reorder the app list would solve the problem in a cleaner way. I can't speak about the animation issue, I haven't used it enough, but I would like to get this into a test build and get some user feedback.

@coxtor
Copy link
Contributor Author

coxtor commented Nov 3, 2021

I'm not quite sure where to go from here. I'm glad to continue the discussion on this but I'll continue to work on this feature once we all are clear on what is the best solution.

@KoalaV2
Copy link

KoalaV2 commented Nov 9, 2021

Been running this for a while now and it runs great. Although it does feel like when going into the favorite app that swiping right should bring me back to the clock.

@Riksu9000
Copy link
Contributor

I'll close this as there are still unresolved issues and the usefulness of this feature is questionable. I've never felt that opening an app with two gestures is too much work. A way to remap double-clicking the button could be useful, since it might be even faster than a swipe.

@Riksu9000 Riksu9000 closed this Mar 5, 2023
@leroivi
Copy link

leroivi commented Mar 19, 2025

Can we reconsider this kind of feature now that we have reached a 3rd page of application ?
A few gesture might not feel like much work when you are setting some alarm or any application that you will focus on, but when you just want to skip your current music I feel like it's a bit heavy. So I guess it depends on the target application and it is a good thing to let the user configure which app deserve a shortcut.

@Ronkn
Copy link

Ronkn commented Jan 10, 2026

Really hope this is reconsidered.

I'd almost prefer to have swipe left be the active or last used app (calulator, music player control, etc) for quick access. Right now, with music playing if I want to pause or play from the watch, I have to scroll down and select the music player. If I'm using it a frequently (it's consistently the last used app), would be nice to have that on swipe right menu.

I do like the idea of a screen to have favorite apps stored. I'd rather see the swipe right menu (flashlight, settings, brightness, notifications) customizable and to put favorite apps there. I don't ever use any of the icons. It's just a wasted swipe. Make the current menus on this screen appear in the "apps" screens. So if someone likes the way it is currently, they can make those their favorites again.

That's my 2 cents anyway.

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 feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.