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

Button.update method? #3603

Closed
TomJGooding opened this issue Oct 27, 2023 · 5 comments · Fixed by #3699
Closed

Button.update method? #3603

TomJGooding opened this issue Oct 27, 2023 · 5 comments · Fixed by #3699
Assignees

Comments

@TomJGooding
Copy link
Contributor

I'm often caught out by Button.update not updating the label (or doing anything!)

I thought about overriding the method to simply assign Button.label, but the argument type for the Static base class is RenderableType whereas the button label is TextType (I'm afraid I don't fully understand the difference!)

Am I the only one who gets caught out by this?

@github-actions
Copy link

Thank you for your issue. Give us a little time to review it.

PS. You might want to check the FAQ if you haven't done so already.

This is an automated reply, generated by FAQtory

@rodrigogiraoserrao rodrigogiraoserrao self-assigned this Nov 17, 2023
@rodrigogiraoserrao
Copy link
Contributor

At the present time, it's nonsensical for Button to inherit from Static, so I'm guessing this inheritance is remnant of a previous Button implementation.

The point of Static is to cache the rendering and use the cache in its method render.
The implementation of Button overrides Static.render, so we're not making use of what the only thing we get by inheriting from Static...

So, we'll drop inheritance of Static and instead you can make use of the reactive attribute label.

@TomJGooding
Copy link
Contributor Author

Thanks @rodrigogiraoserrao - I didn't think to check whether Button actually needed to inherit from Static!

@rodrigogiraoserrao
Copy link
Contributor

I get it, @TomJGooding. I checked. I replaced Static for Widget. I ran the tests. They passed. And I was so suspicious of having broken something without realising it... I spent a lot of time going over that tiny change to make sure I wasn't missing anything 😆

Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

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 a pull request may close this issue.

2 participants