-
Notifications
You must be signed in to change notification settings - Fork 814
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
fix: scalar animation with percentages #3004
fix: scalar animation with percentages #3004
Conversation
Should a check be added here? mypy complains that
|
The app is a DOMnode, which doesn’t have a parent because it’s at the very top. Not sure if that’s applicable here. Haven’t checked the code yet… |
Thanks Will, that's what I thought. I guess someone might try animating the size of the app (though pretty sure that would crash currently anyway)...! |
@TomJGooding Is this one still relevant? |
Yes, it looks like there is still an issue with scalar animation. Here's a slightly modified version of the app from the added test. Press from textual.app import App, ComposeResult
from textual.containers import Container
from textual.widgets import Footer, Static
class ScalarPercentAnimApp(App):
BINDINGS = [("space", "start_animation", "start animation")]
# To simplify percentage calculations, the widget to be animated is placed
# inside a container with a width of 100
CSS = """
#container {
width: 100;
}
#foo {
width: 20%;
height: 3;
background: red;
}
"""
def compose(self) -> ComposeResult:
with Container(id="container"):
yield Static(id="foo")
def action_start_animation(self) -> None:
static = self.query_one("#foo", Static)
# The animation duration is set to 6 seconds, so after every
# second the width should have increased by 10 cells
static.styles.animate("width", "80%", duration=6.0, easing="linear")
if __name__ == "__main__":
app = ScalarPercentAnimApp()
app.run() I think the problem is how the start and destination are 'resolved' here, as shouldn't this be based on the size of the container (parent) rather than the widget? textual/src/textual/css/scalar_animation.py Lines 38 to 42 in 0ffa82a
|
Sorry I realise this PR has been hanging around for a while, but after some tinkering and hopefully now with a better understanding of Textual, I'm tentatively marking this as ready for review. You'll see I've struggled with a creating a test that isn't flaky in CI (only in Windows for some reason), so I'd welcome any suggestions for improving this. |
Closed as stale. |
Description
Fixes #2940 by resolving the scalar animation
start
anddestination
cell sizes based on the size of the container (parent) rather than the widget.Related Issue
How Has This Been Tested?
The added test sets both the initial widget width and the animation value as percentages.
The size is checked at specific intervals during the animation usingpilot.pause
, which might be why the test is a bit flaky?This has also been tested manually, with both the
animate
method and the (currently undocumented) CSStransition
feature.Checklist: