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

(feat): Add option to set max FPS #578

Merged
merged 2 commits into from
Jun 6, 2023

Conversation

tomfeigin
Copy link
Contributor

@tomfeigin tomfeigin commented Oct 26, 2022

Solves issue #577

Added the WithMaxFPS WithFPS option to bubbletea program struct, if it is set with a non 0 value it will set the maximum interval at which the view is updated, if it is not set or set to 0, the interval will be the defaultFramerate constant.

@muesli muesli added the enhancement New feature or request label Oct 26, 2022
@tomfeigin
Copy link
Contributor Author

Hey @muesli did you get a chance to look at this PR?
We really want to stop using our fork and use the official repo and this tiny change blocks us

@meowgorithm
Copy link
Member

meowgorithm commented Jan 23, 2023

I think this one makes sense. My only thought is that we probably want to cap the upper value to keep things from accidentally getting too outta control (so maybe 120 or so).

And if we're clamping the values an int probably makes more sense for convenience as it's the Go numeric default.

tea.go Outdated Show resolved Hide resolved
@tomfeigin
Copy link
Contributor Author

@meowgorithm hey, can we merge this, please? Should I change anything?

@meowgorithm
Copy link
Member

@meowgorithm hey, can we merge this, please? Should I change anything?

Hey there! Thanks for the ping and sorry for the delay on this. Will give it a re-review this week.

tomfeigin and others added 2 commits May 10, 2023 20:49
Added the `WithMaxFPS` option to bubbletea program struct, if it is set
with a non 0 value it will set the maximum interval at which the view is
updated, if it is not set or set to 0, the interval will be the
`defaultFramerate` constant.
@meowgorithm
Copy link
Member

Thanks for your patience, @tomfeigin. I've rebased master onto this branch and made a few small edits and I believe this one is ready to go. I'm not able to push my changes to the branch on your repo, however.

Do you mind either ticking the box to allow me to push or resetting to the fps branch on your end? Alternatively, I could just open a new PR as well.

Anyway, let me know.

@tomfeigin
Copy link
Contributor Author

@meowgorithm Thank you! Done :)

@meowgorithm meowgorithm requested a review from muesli May 11, 2023 04:18
@meowgorithm
Copy link
Member

@meowgorithm Thank you! Done :)

Awesome. Would ideally like to get @muesli’s approval as well.

Mues, I played with this a bit and for me it’s working great.

@tomfeigin
Copy link
Contributor Author

@muesli @meowgorithm, any updates on this? We really want to stop maintaining our fork

@meowgorithm
Copy link
Member

@tomfeigin Apologies for the delay. We’re merging this today.

@meowgorithm meowgorithm merged commit f3e1b67 into charmbracelet:master Jun 6, 2023
@meowgorithm
Copy link
Member

Alrighty, you'll see this in the next release. Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants