-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Feature: Favorite app or last used app when swiped left on the watch face #708
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
Conversation
|
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 😅. |
tmilburn
left a comment
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.
Overall structure of the code looks good but needs a few fixes.
|
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. |
|
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. |
|
There's an issue inherent to swiping left. The transitions get confusing. VID_20211001_125314.mp4If 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 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. |
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. |
|
@coxtor Tried it and it is awesome, thanks. |
|
Thanks for the feedback so far. |
|
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 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. |
@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 |
|
I agree with @Riksu9000 that #717 does not solve it. It is not a configuration issue of the button but program issue which depends:
So the both cases above are simply solved by For the #717 I'll reply to that thread, since it really doesn't apply to this Favorite app thread. |
|
I see, now I understand what the issue was. The current changes should fix the behavior making #717 obsolete. Still to do:
|
|
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:
This might be really clean solution and we:
What do you think? |
It does allow styling by setting the state to checked at least.
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? |
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. |
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. |
|
@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) |
|
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. |
|
Exactly, should be practically the same code with apps but adds more funcionality and options. |
|
@hubmartin check it out if you like =) |
|
@coxtor Absolutely awesome! Thanks for adding. |
|
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 |
|
@hubmartin thanks for highlighting those bugs! Should be ready to merge now ;) @JF002 ? |
Riksu9000
left a comment
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.
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.
src/displayapp/DisplayApp.cpp
Outdated
| } else { | ||
| if (!currentScreen->OnButtonPushed()) { | ||
| LoadApp(returnToApp, returnDirection); | ||
| if(favoriteAppActive){ |
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.
I'm pretty sure all of these favoriteAppActive things aren't necessary, because they were added before ReturnApp() was properly set ccd3742
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.
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); |
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.
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)
src/displayapp/DisplayApp.cpp
Outdated
| if (currentApp != Apps::Clock && currentApp != Apps::Launcher && currentApp != Apps::QuickSettings){ | ||
| previousApp = currentApp; | ||
| } |
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.
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.
I think it is corrected in the latest update
Fixed
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?
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. |
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.
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. |
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 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:
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. |
|
any way to review this very good pull (@Avamander or @geekbozu )? |
|
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. |
|
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? |
|
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. |
|
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. |
|
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. |
|
Can we reconsider this kind of feature now that we have reached a 3rd page of application ? |
|
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. |

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