fix: scalar animation with percentages
Description
Fixes #2940 by resolving the scalar animation start and destination cell sizes based on the size of the container (parent) rather than the widget.
Related Issue
- #2940
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 using pilot.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) CSS transition feature.
Checklist:
- [ ] Docstrings on all new or modified functions / classes
- [ ] Updated documentation
- [ ] Updated CHANGELOG.md (where appropriate)
Should a check be added here? mypy complains that "DOMNode | None" has no attribute "size", but am I right in thinking that all widgets have a parent which will be either another widget or the app?
https://github.com/Textualize/textual/blob/33d775ab823f4c01d819a29e7581dfe8453c7e9a/src/textual/css/scalar_animation.py#L38
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 Space to start the animation, and you'll the see the width rubber bands as described in the linked issue #2940.
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?
https://github.com/Textualize/textual/blob/0ffa82a13ec28c242ea13e2424863e09d819ed82/src/textual/css/scalar_animation.py#L38-L42
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.