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 a new InfiniteAnimation #5344

Merged
merged 6 commits into from
Jan 2, 2025
Merged

Conversation

dweymouth
Copy link
Contributor

@dweymouth dweymouth commented Dec 28, 2024

Description:

Adds a new type of animation that has no curve or duration, and simply ticks a function every frame until stopped.
This will be useful in 2.6 to schedule an ongoing "update loop" to run on the main app thread, for example in the fyne-io/life repo to replace the goroutine that currently drives the updates.

Note: Since this is implemented entirely using the existing animation type, it's debatable whether it's even needed as it can be fully implemented in user code with the current set of public APIs. However, it's more clean to have a public API specifically for this purpose and a Tick function that takes no arguments, instead of user code having to supply one that simply ignores its argument.

Also, it's possible other names could be more descriptive of its more general use case, like fyne.FrameTicker?

Fixes #4735

Checklist:

  • Tests included. // we don't currently have animation tests
  • Lint and formatter run with no errors.
  • Tests all pass.

Where applicable:

  • Public APIs match existing style and have Since: line.

animation.go Outdated Show resolved Hide resolved
animation.go Outdated
Comment on lines 80 to 89
// Start registers the animation with the application run-loop and starts its execution.
func (i *IndefiniteAnimation) Start() {
i.setupAnimation()
i.animation.Start()
}

// Stop will end this animation and remove it from the run-loop.
func (a *Animation) Stop() {
CurrentApp().Driver().StopAnimation(a)
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a fan of how the methods of different types are interleaved. I'd much rather have all methods for one typ after each other in alphabetical order before the next type.

Copy link
Member

Choose a reason for hiding this comment

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

To clarify: I'd rather have struct, methods, struct, methods and so on. I think that's how Go files generally are structured. Without your latest change it is struct, struct, methods, methods?

@Jacalz
Copy link
Member

Jacalz commented Dec 29, 2024

Whoops. Forgot to add the message again. I like the idea of this but I'm not entirely sure if this is the best API for it (not necessarily against but an animation that just passes random curve and duration seems a bit strange). :)

@dweymouth
Copy link
Contributor Author

Whoops. Forgot to add the message again. I like the idea of this but I'm not entirely sure if this is the best API for it (not necessarily against but an animation that just passes random curve and duration seems a bit strange). :)

That part is just an implementation detail hidden away from the public API. I did it that way to avoid having to do a much larger code change making the animation scheduler aware of a new type. The implementation details could always be changed later, though if you have any other suggestions for how to implement this, I'm all ears.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

I think it makes sense to have a clean way to have such an infinite animation, rather than exposing the details in docs or example.

However the word "indefinite" sounds somewhat confusing - would infinite not be more intuitive (and match the progress widget wording).

@dweymouth dweymouth requested review from Jacalz and andydotxyz January 1, 2025 19:49
@dweymouth dweymouth changed the title Add a new IndefiniteAnimation Add a new InfiniteAnimation Jan 1, 2025
@andydotxyz
Copy link
Member

I'm trying to re-run the Ubuntu tests to see if it is temperamental issue.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Great thanks

@dweymouth dweymouth merged commit 0c59609 into fyne-io:develop Jan 2, 2025
12 checks passed
@dweymouth dweymouth deleted the infinite-animation branch January 2, 2025 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants