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

AnimationSlide: add a new setting to control slide-scroll animation #2100

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

ia
Copy link
Collaborator

@ia ia commented Mar 6, 2025

  • Please check if the PR fulfills these requirements
  • The changes have been tested locally
  • There are no breaking changes
  • What kind of change does this PR introduce?
    Add a new setting to control side-slide-scroll animation (Settings - User interface - Anim.sliding).

  • What is the current behavior?
    Fancy smooth side-slide-scrolling animation between operating mode screens without any control of this option.

  • What is the new behavior (if this is a feature change)?
    Add AnimationSlide setting with three values:

    • off: static instant transition between screens;
    • S (as in Settings only): side-slide-scroll animation happens only if a user goes to the settings menu, navigates in the settings menu, or exits the settings menu;
    • E (as in Everywhere/Full): side-slide-scroll animation enabled everywhere (current default behavior).
    • NOTE: this option does not intersect in any way with any other animation-related setting, and I would like to keep it in this way.
  • Other information:

Related bug report. I must admit that at first I was kinda skeptical about this setting (just like against any new setting which overbloats the code base and flash storage :) but while I was testing this myself, I have to say that when it's off, I'm fascinated with the rocket-like reactive instant response from the interface. Don't get me wrong, side-slide-scrolling animation looks really cool, smooth and fancy for a such low hardware devices, but I think that some users who prefer the speed of the time to response in the interfaces over visual effects in a daily life, will find this setting to be off/settings only most of the time useful as well.

I did make testing myself with every implemented option value and it seems it works for me as it should.

@discip, I can't wait to get any feedback from you, since it seems that the original user who did request this feature went silent, but you seemed interested in this as well. I know that you're busy these weeks, so don't rush, but let me know once you test it yourself. Thanks a lot in advance.

@Ralim, what do you think about the code? If it's fully ok, you can approve it (without a merge) when you have time, and I will merge it myself later, but only after getting positive confirmation from @discip.

P.S. It's really interesting, how you (re)implemented guiRenderLoop(), guiHandleDraw(), and guiContext struct: this is sooo convenient that we can get not only the type of current mode, but "where we came from" through previousMode field. Nice!

@ia ia added Enhancement New feature or additional function. Usability UX, Usability and/or Design. labels Mar 6, 2025
@ia ia requested review from Ralim and discip March 6, 2025 18:09
@ia ia self-assigned this Mar 6, 2025
@discip
Copy link
Collaborator

discip commented Mar 6, 2025

@ia
Apologies for the delay, I finally have some time to respond.

Initially, I was one of those people who wanted a certain level of animation in the menu navigation. I thought that sideways animations could indicate when you're entering or leaving a main menu, while up/down animations would signal scrolling through sub-menu entries. (for reference #892)

When @Ralim revamped the settings to accommodate different screen sizes, he also added some great animations. (I'm still really grateful for that! 😊)

As I mentioned earlier, there was one animation in particular that felt a bit off, but I've gotten used to it, so no issues there.

What I was referring to was the sideways animation that happens when both the idle screen and soldering screen are set to "detailed." In the past, the temperature would just start rising, and the view remained almost unchanged, which felt more natural to me. However, the sliding animation when switching from idle to soldering now seems like a bit of a disruption.

So the animations at every other place is what I requested some time ago.
The only place it should be left out is the one that was described above. 😊

I hope that now makes sense.

If you ask me, I wouldn't recommend adding this type of setting.

@ia
Copy link
Collaborator Author

ia commented Mar 7, 2025

@ia Apologies for the delay, I finally have some time to respond.

Initially, I was one of those people who wanted a certain level of animation in the menu navigation. I thought that sideways animations could indicate when you're entering or leaving a main menu, while up/down animations would signal scrolling through sub-menu entries. (for reference #892)

Yes, I understand, it looks very useful...

When @Ralim revamped the settings to accommodate different screen sizes, he also added some great animations. (I'm still really grateful for that! 😊)

... and I fully agree, it looks super cool and fancy.

What I was referring to was the sideways animation that happens when both the idle screen and soldering screen are set to "detailed." In the past, the temperature would just start rising, and the view remained almost unchanged, which felt more natural to me. However, the sliding animation when switching from idle to soldering now seems like a bit of a disruption.

I think I finally start to understand, what the original bug is about. Thank you! Finally someone explained this in a proper way. Original report has only one sentence with few words only which doesn't make much of a sense to me. So thank you.

So the animations at every other place is what I requested some time ago. The only place it should be left out is the one that was described above. 😊

Hold my beer, as they say...

I hope that now makes sense.

Yes, finally, thank you!

If you ask me, I wouldn't recommend adding this type of setting.

As of for this PR... we will come to this later, because now I really would like to have this option. I mean, if we have a setting to disable animation for the icons in settings menu, why wouldn't have option to disable transition animation? I would definitely use this myself in some cases, when I need instant feedback from my iron once I click a button. Just try this out of your own curiosity if you will find time, please ;)

@discip
Copy link
Collaborator

discip commented Mar 7, 2025

Just try this out of your own curiosity if you will find time, please ;)

Will do in the not too far future.

@discip
Copy link
Collaborator

discip commented Mar 8, 2025

@ia
I finally tested this, and I have to admit that the none setting brought back some nostalgic feelings.
However, as I mentioned earlier, it's not really essential — it's just one more option to tweak. 😊

I hope this doesn't come across the wrong way – I truly appreciate your work.
My intention in leaving a comment on the original issue was not to ask anyone to take action or create extra work.

Copy link
Owner

@Ralim Ralim left a comment

Choose a reason for hiding this comment

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

This looks good to me.
Make sure to update setting number on sync :)

@ia ia added this to the 2.23 milestone Mar 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or additional function. Usability UX, Usability and/or Design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants